Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for complex config options #348

Closed
wants to merge 3 commits into from
Closed

Add support for complex config options #348

wants to merge 3 commits into from

Conversation

mdreizin
Copy link

@mdreizin mdreizin commented Sep 28, 2016

This PR adds ability to use use custom loader options:

{
    module: {
        loaders: [{
            test: /\.css$/,
            loaders: [
                'style',
                'css?config=customCssConfig'
            ]
        }]
    },
    customCssConfig: {}
}

Instead of #327, #315 I prefer to use recommended way to retrieve the loader config loader-utils#getloaderconfig.

@sokra Could you please review this PR?

@codecov-io
Copy link

codecov-io commented Sep 28, 2016

Current coverage is 98.37% (diff: 100%)

Merging #348 into master will increase coverage by 0.02%

@@             master       #348   diff @@
==========================================
  Files             9         10     +1   
  Lines           303        307     +4   
  Methods          41         42     +1   
  Messages          0          0          
  Branches         66         67     +1   
==========================================
+ Hits            298        302     +4   
  Misses            5          5          
  Partials          0          0          

Powered by Codecov. Last update 22f6621...be53c18

@mdreizin mdreizin changed the title added ability to use custom loader options via "config" loader query Add support for complex config options Sep 28, 2016
@mdreizin
Copy link
Author

mdreizin commented Oct 2, 2016

cc @jhnns @bebraw

@bebraw
Copy link
Contributor

bebraw commented Oct 2, 2016

What's the benefit over options?

@mdreizin
Copy link
Author

mdreizin commented Oct 2, 2016

@bebraw Here is some of benefits:

  • it gives you ability to have shared config for group of xxx-loader
  • it gives you ability override default options without modifying query/options of xxx-loader
  • it gives you ability to have custom configs for some user cases
  • it gives you ability to use recommended way to retrieve config of xxx-loader
  • etc

Please see:

https://github.com/webpack/less-loader#less-plugins
https://github.com/jtangelder/sass-loader#sass-options
https://github.com/webpack/html-loader#advanced-options
webpack/loader-utils@7e86a18

@bebraw
Copy link
Contributor

bebraw commented Oct 2, 2016

Gotcha. Looks like a good idea then so 👍 from me. I think @sokra can comment on the code once he has some time available.

@mdreizin
Copy link
Author

mdreizin commented Oct 2, 2016

@bebraw Thanks a lot.
It would be nice to have this feature as early as possible.
I'm looking forward to get feedback from @sokra.

@jhnns
Copy link
Member

jhnns commented Oct 4, 2016

I thought we agreed that polluting the webpack config object is not a good idea? We also recently added config object validation to webpack@2, so this will not work with webpack 2, right?

@bebraw
Copy link
Contributor

bebraw commented Oct 4, 2016

@jhnns Ok, I guess in that case we might to remove the feature from those other loaders to keep this consistent and document how to achieve the same effect without.

@sokra
Copy link
Member

sokra commented Nov 17, 2016

No longer allowed in webpack 2 as @jhnns said

it gives you ability to have shared config for group of xxx-loader

const sharedOptions = { ... }

it gives you ability override default options without modifying query/options of xxx-loader

options: {
  ...sharedOptions,
  key: value
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants