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 a plugin option to support passing options cssnano v4 #73

Merged
merged 3 commits into from Aug 29, 2018

Conversation

tommytroylin
Copy link
Contributor

@tommytroylin tommytroylin commented Aug 3, 2018

for issue #69 #71

add a cssProcessorPluginOptions as a third arg passed to cssProcessor

@@ -23,6 +23,9 @@ class OptimizeCssAssetsWebpackPlugin extends LastCallWebpackPlugin {
this.options.cssProcessorOptions = !options || options.cssProcessorOptions === undefined ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, postcss needs the same options in old versions
{ from, to }

As a convenience, plugins also expose a process method so that you can use them as standalone tools.


cleaner.process(css, processOpts, pluginOpts)
// This is equivalent to:
postcss([ cleaner(pluginOpts) ]).process(css, processOpts)

from postcss api

@mirucon
Copy link

mirucon commented Aug 9, 2018

Been having the issue with the option, and this PR completely solved it! +1 for merging this.

@NMFR
Copy link
Owner

NMFR commented Aug 27, 2018

Was thinking if this should be implemented with a cssProcessorOptionsArgs option.

If cssProcessorOptionsArgs was not passed use:
cssProcessor.process(css, cssProcessorOptions)

If cssProcessorOptionsArgs was passed, ignore the value of cssProcessorOptions and use cssProcessorOptionsArgs for the rest of the arguments after the css:
cssProcessor.process(css, ...cssProcessorOptionsArgs)

But the ignoring cssProcessorOptions part fells bad.

What do you think?

@tommytroylin
Copy link
Contributor Author

@NMFR
I also fell bad when ignoring it.
I'm thinking if we can make sure that cssProcessorOptions should be an object. maybe we can reuse this arg name;

There are 3 solutions now:

  1. Keep old cssProcessorOptions. Add cssProcessorPluginOptions for pluginsOptions
  2. Keep old cssProcessorOptions. Add cssProcessorOptionsArgs Array, when it was passed, ignore cssProcessorOptions and spread cssProcessorOptionsArgs to that plugin call
  3. if cssProcessorOptions is object, it should work as same as now. But if it is an array, spread it to plugin call

Idk which one would you guys prefer.

@NMFR
Copy link
Owner

NMFR commented Aug 29, 2018

I don't think solution 3 is a good ideia, it would make it weird if the first parameter was an array.

Going with solution 1 since its more retro compatible.

@NMFR NMFR merged commit 5c141d8 into NMFR:master Aug 29, 2018
@NMFR
Copy link
Owner

NMFR commented Aug 29, 2018

Many thx for the help

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

4 participants