Provide the same API for config.optimization.minimizer as config.plugins #84
Conversation
|
@edmorley this appears to be a breaking change, am I right? |
|
Quite possibly? I guess we could add some more test cases to confirm? |
|
The API for // current:
minimizer(value) // returns Optimization instance
// proposed:
minimizer(name, value) // returns Plugin instanceSo yeah, breaking change. |
|
@guycreate could you also update the README parts where this is changed? Thanks! |
guybarnard
force-pushed
the
patch-optimization
branch
from
df3ddca to
5bed05c
Compare
|
Thanks for feedback. Implemented changes as best requested, by changing .gitignore to match coding standards, a call of .toConfig from config.js (and associated test), and readme to reflect the updated API. Agreed this is a breaking change. |
There was a problem hiding this comment.
@eliperelman this looks good to me with the final tweaks made - can you spot any other changes required? It would be good to get this merged/released soon, so I can start on neutrinojs/neutrino#1146. However given this change is breaking I was thinking we should perhaps get #107 merged/released as a semver patch before this PR is then released as a new major?
|
@guycreate meant to say thank you for the changes - they look great, just a couple of small tweaks and this should be good to merge. If you'd prefer me to make those changes (given a few rounds of review already, so I don't want it to be too much of a pain!) just let me know :-) |
guybarnard
force-pushed
the
patch-optimization
branch
from
9334616 to
188e829
Compare
|
Thanks for continued feedback. I think this reflects the required changes, but feel free to edit directly if that's quicker/easier for additional edits. |
There was a problem hiding this comment.
This looks great - thank you for the changes!
@eliperelman I'll likely merge/publish this as 5.0.0 tomorrow unless I hear back? Any other changes you'd like to see here, or else other breaking changes you'd like to see in v5?
edmorley
changed the title
|
Published as v5.0.0. Thank you again for the PR! :-) |
| .tap(args => newArgs) | ||
|
|
||
| // Example | ||
| config |
There was a problem hiding this comment.
Hi !
I think there is a small mistake here, as it should be config.optimization too :)
There was a problem hiding this comment.
@Gramatiik, ah you're right. Would you be up for opening a PR? :-)
In #84 (released as part of webpack-chain 5.0.0) the syntax for using `optimization.minimizer` was altered, so that it matched that for `plugin`, `resolve.plugin` and `resolveLoader.plugin`. Syntax in webpack-chain 4: ``` config.optimization.minimizer([ new WebpackPluginFoo(), new WebpackPluginBar(arg1, arg2), ]); ``` Syntax in webpack-chain 5+: ``` config.optimization.minimizer('foo').use(WebpackPluginFoo); config.optimization.minimizer('bar').use(WebpackPluginBar, [arg1, arg2]); ``` Currently if someone uses the old syntax with newer webpack-chain, then the `minimizer()` call succeeds, but later when `.toConfig()` is called, the following error is generated: `TypeError: Cannot read property '__expression' of undefined` This PR adds an explicit check to `optimization.minimizer()` which ensures a clearer error message is shown at point of use, so that the stack trace is more relevant, and root cause clearer. Refs: #204 (comment)
In #84 (released as part of webpack-chain 5.0.0) the syntax for using `optimization.minimizer` was altered, so that it matched that for `plugin`, `resolve.plugin` and `resolveLoader.plugin`. Syntax in webpack-chain 4: ``` config.optimization.minimizer([ new WebpackPluginFoo(), new WebpackPluginBar(arg1, arg2), ]); ``` Syntax in webpack-chain 5+: ``` config.optimization.minimizer('foo').use(WebpackPluginFoo); config.optimization.minimizer('bar').use(WebpackPluginBar, [arg1, arg2]); ``` Currently if someone uses the old syntax with newer webpack-chain, then the `minimizer()` call succeeds, but later when `.toConfig()` is called, the following error is generated: `TypeError: Cannot read property '__expression' of undefined` This PR adds an explicit check to `optimization.minimizer()` which ensures a clearer error message is shown at point of use, so that the stack trace is more relevant, and root cause clearer. Refs: #204 (comment)
The
configsection has special plugin logic that allows.plugin(name).use(pluginclass)and merge functionality.This pull request adds equivalent logic for optimization plugins (known as minimizers). This allows the configuration of such plugins to be changed as part of a
neutrinoenvironment (with arguments overriden before the plugin is actually initialized).Fixes #95.