-
-
Notifications
You must be signed in to change notification settings - Fork 178
refactor(index): upgrade compiler.plugin to the new plugin system (tapable)
#198
Conversation
compiler.plugin to the new plugin system (tapable)
| "ci:coverage": "npm run test:coverage -- --runInBand", | ||
| "defaults": "webpack-defaults" | ||
| "defaults": "webpack-defaults", | ||
| "webpack-defaults": "webpack-defaults" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "webpack-defaults": "webpack-defaults"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm how the hell did that end up in there? lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a left over from the webpack-defaults v1.6.0...2.0.0 upgrade one needs to be aware of, unrelated to this PR entirely. I just saw/catched it here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I need to make a change for this? Or is PR good as is?
| const requestShortener = new RequestShortener(compiler.context); | ||
|
|
||
| compiler.plugin('compilation', (compilation) => { | ||
| compiler.hooks.compilation.tap('UglifyJsWebpackPlugin', (compilation) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is UglifyJsWebpackPlugin accessible as a hook or is this just the plugin name for e.g interception ? UglifyJsPlugin ? What's the naming convention here ? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michael-ciniawsky - Check the tapable changes for 4.x. This is something you are going to see a lot of on the contrib side for Webpack 4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's why I'm asking :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the plugin name! :-D
|
If there aren't any objections, this should be ready to land on |
|
This PR updates
uglifyjs-webpack-pluginto use.tap()and.tapAsync()isntaed of.plugin()and use the new Tapable API inwebpack@webpack/webpack#next.Looks like prettier did some auto updating etc.