Fix loader context

[jescalan]

This resolves a bug in which the webpack loader context was not being passed correctly to the options parsing function.

[ai]

1. Use PostCSS common config with ctx.webpack
2. I will accept this PR only tomorrow (it is time to go to bed for me :) )

[jescalan]

Not sure what you mean with #1 there, but thanks so much! Would be great to see a patch for this, it's breaking some of my software 😞

[ai]

Loader options are deprecated (because of webpack bug).

Config: https://github.com/postcss/postcss-loader#usage

Webpack instance: https://github.com/postcss/postcss-loader#dynamic-config

[jescalan]

Not entirely sure I understand why this would be the case. Either way, the ctx coming in was the node global previously, not the webpack loader context, which I assume wasn't the desired result!

[ai]

webpack loader context is in ctx.webpack. ctx is not node global context. It is a argument for PostCSS common config:

module.exports = function (ctx) {

[ai]

Here is more docs about ctx in PostCSS common config https://github.com/michael-ciniawsky/postcss-load-config#context

[jescalan]

So I am not sure what postcss-load-config is, but I am not interested is using it at all here. I just need to get the webpack loader context available in a config function passed directly to the loader. Right now, that context is not available at all, you can trace it as such:

At this point, in the direct context of the exported function called by webpack, this is the webpack loader context. You save it off as a variable called loader. Now we move through the code, and here is where the parseOptions function is called without setting the context to anything specifically, so it retains its own context for this, and we have lost the webpack loader context.

Now, when we're inside parseOptions is when the config function is called, if one was passed. It gets called with this, which is set to the global context since there is no this bound to the function when its called -- the function is called without context, so it is automatically the global context. So inside of the config function passed in, it's the global context. This is a this/context bug in this specific codebase, it doesn't have anything to do with webpack at all, and is completely and entirely fixed by this PR alone.

The bug was introduced in this commit, when the options function call was extracted into a different file which didn't have the correct loader context.

Hope this makes sense!

[michael-ciniawsky]

@ai like @jescalan said this inside parseOptions is not bound to loader, likely that is the source of a few weird bugs, we recently experienced :).

@jescalan 👍

[michael-ciniawsky]

@ai updated CHANGELOG.md and package.json locally, can you approve patch release on npm please ? :)

[ai]

I told that I am sleeping 😉

I anyway planned to merge it and release tomorrow.

[michael-ciniawsky]

I told that I am sleeping 😉

You're sleeping as it seems 😛 , but sry I didn't see it, not to make any stress here, can I push CHANGELOG and pkg version bump with 'Release 1.1.1 version' or better you do it tomorrow :)?

[ai]

I will do it tomorrow

[michael-ciniawsky]

But can't make npm patch release anyways. Sleep well

@ai kk discared

[ai]

@jescalan released as 1.1.1.

But note, that loader options was deprecated ;).

[jescalan]

@ai @michael-ciniawsky thanks so much guys for the quick response and release! 🎉

[ai]

Thank you for fix :)