Skip to content

The way I should specify minification options is "unpleasant" #321

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

Closed

Conversation

termosa
Copy link

@termosa termosa commented Aug 23, 2016

I will disfigure my configuration file if I try to specify many options for cssnano. Why not provide an option to parse the file with options?

@codecov-io
Copy link

codecov-io commented Aug 23, 2016

Current coverage is 97.72% (diff: 66.66%)

Merging #321 into master will decrease coverage by 0.62%

@@             master       #321   diff @@
==========================================
  Files             9          9          
  Lines           303        308     +5   
  Methods          41         41          
  Messages          0          0          
  Branches         66         67     +1   
==========================================
+ Hits            298        301     +3   
- Misses            5          7     +2   
  Partials          0          0          

Powered by Codecov. Last update 068cec3...ef71fa7

@andreypopp
Copy link
Contributor

andreypopp commented Aug 23, 2016

This is also possible to configure using query key in loader config:

module.exports = {
  ...,
  module: {
    loaders: [
      { test: /\.css$/, loader: "css", query: {minimize: true} }
    ]
  }
};

@termosa
Copy link
Author

termosa commented Aug 23, 2016

@andreypopp wow, now I know a bit more about webpack.
But still, it requires me to load configurations by my own and to use object-assign…
Are you sure you don't want to provide this option? Or at least support of cssnano options in webpack config like eslint did?

@andreypopp
Copy link
Contributor

@termosa idk, not a maintainer, but my opinion is that it's better to keep the loader implementation simpler. Also I think eslint-loader shouldn't support reading from .eslintrc.

@sokra
Copy link
Member

sokra commented Nov 17, 2016

I agree with @andreypopp. options in the configuration should be with only source of loader settings.

@sokra sokra closed this Nov 17, 2016
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.

4 participants