Add capability to pass other options as function

[jescalan]

In its current implementation, parser, syntax, and stringifier can only be passed as string which are required internally. This patch allows users to pass in any of these options as a function instead of a string, which is important to be able to do if the option is passed from elsewhere and not required directly as is done internally.

This patch is made in a manner that is non-breaking, although honestly making a breaking change to the way options are handled would provide a much cleaner syntax and easier user experience. If you're open to breaking changes, I'd be happy to modify this -- my proposal would be to just have the postcss webpack option always ask for an object or function that returns an object. This way we have a flexible data structure that's able to accommodate whatever the user needs to pass, it's expandable for future options, and all the config is in one place instead of spread across querystrings and webpack options. For example:

module.exports = {
  loaders: [...],
  postcss: {
    plugins: [...],
    syntax: someModule
    // and any number of other options
  }
}

[ai]

Yeap, we should update API. I agree that we should support only one syntax.

Let’s change syntax in 2 steps:

1. Add support for two syntaxes. And print a warning for old one.
2. After few months remove old syntax.

[ai]

Because this changes is very important and we should be cautious, I will accept this PR few hours later (I am in the hangover right now :D).

[jescalan]

Awesome! Let me know if there's anything else I can do to help.

There is one drawback here I realized that I feel like should be brought up at least. If we swap over to accepting all syntax options only through the webpack postcss option, using webpack's require decorator will no longer be as effective without the querystring options. I don't ever use this feature personally since my settings are always inside the config file, I'm not sure if anyone does honestly, but it's possible that some others do so it might be worth asking around.

[ai]

@Jenius require() query string should overrides config options

[jescalan]

Ah yeah I see what you're saying here, disregard the last comment then. Agreed.

[ai]

Sorry, I need to spend all my today and tomorrow time for work. I will accept it on weekend.

[ai]

I thought about this issue and there is a one problem. All current loaders uses query string for options. Right now we move plugins to separate webpack.config.js only because plugin options can be passed in query string.

But why we need to move all options here?

[jescalan]

Sorry I'm not entirely sure I understand, but I tried to make it clear what my use case was at least in the initial description:

This patch allows users to pass in any of these options as a function instead of a string, which is important to be able to do if the option is passed from elsewhere and not required directly as is done internally.

Sorry if I misunderstood!

[jescalan]

???

[ai]

@Jenius GET parameters is a webpack API. Why we should add extra non-standard way?

[jescalan]

So some of the options come in as strings are are require-ed internally. You can see this here. The user in this case has no control over where the modules can be require-ed from. This also makes it impossible to take a library that the user has loaded up and initialized manually and use it along with postcss.

Passing a string and requiring internally limits the loading of any other options to only libraries that can be found directly in node's require pathway and gives the user no options when it comes to customizing the behavior of the loaded module.

I made this PR because I have a use case where this is limiting enough to require a fork -- this is a real issue that affected me, not some craziness I made up.

If you don't want to accept it, that's ok but I want to make sure that you do understand the reasons behind it and the fact that this is an issue which will require myself and other users who need this functionality to rely on a fork indefinitely.

[ai]

@Jenius maybe we need to fix require instead of changing a options?

How did other webpack loaders fix this issue?

Can you show me some examples of your problem to understand it better?

[jescalan]

Well, webpack has an established way of handling this use case, which is documented here. There are a number of other loaders that use this method to solve the issue.

This is exactly the pattern I submitted in this PR, so I think it should be the right way to handle it.

[ai]

And what is your problem exactly?

Sorry for asking so many questions, but API is very important thing. Especially, when you have so bad API basement as in webpack :D.

[jescalan]

I'm sorry, I have tried my best to explain why this is necessary a number of times in this issue, and I'm not sure how I can make it more clear. This was my last attempt to explain it. Was there anything that I did a bad job of explaining there that I can try to re-word?

[ai]

Sorry, I am not native speaker. Can you just show me a code? Abstract things is always hard for me, I think better with real use case and real code example.

[MoOx]

+1 for this PR, but I would keep support for current syntax, and not deprecate it (because it's clearly enough for 99% of users (since nobody raised this issue before).

[jescalan]

Yeah, this particular PR does keep the current syntax, just adds the secondary syntax as well.

[jescalan]

So for a code example, usage would look like this in a webpack config file:

var autoprefixer = require('autoprefixer')
var customParser = require('./custom_parser')

module.exports = {
  module: {
    loaders: [{ test: /\.css$/, loader: 'postcss' }]
  },
  postcss: { plugins: [autoprefixer], parser: customParser }
}

Hope this helps!

[kylemac]

the code in this PR's readme.md commit is also a real world code example, from a month ago

[ai]

Thanks for clarification.

I well will accept this PR and release new version, when I will find some stable place (I am in travel). I think on this week.

[ai]

I removed && !opts.syntax to allow override syntax in query (somebody may need it)

[ai]

Released in 0.9

[jescalan]

🎉

[ai]

Sorry for long delay 😞