Skip to content

Conversation

jantimon
Copy link
Owner

@jantimon jantimon commented Jul 6, 2018

Export all hooks static and typed similar to webpack/webpack#7672
cc @ooflorent

Copy link

@ooflorent ooflorent left a comment

Choose a reason for hiding this comment

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

Looks fine! But I don't like getHook function. I prefer static code.

…pack-plugin hooks

BREAKING CHANGE: The html-webpack-plugin doesn't add its hooks to the compilation object anymore
@jantimon jantimon force-pushed the webpack-4-static-hooks branch from 59c8e8f to 4a96a9c Compare July 6, 2018 12:47
@jantimon
Copy link
Owner Author

jantimon commented Jul 6, 2018

Okay changed it to getHooks - will rename it again to hooks depending on:

webpack/webpack#7673 (comment)

@emilio-martinez
Copy link

This feels like a great change, in my opinion. The access to hooks feels much cleaner.

@jantimon jantimon merged commit 82b34a1 into webpack-4 Jul 8, 2018
hooks.htmlWebpackPluginAfterEmit = new AsyncSeriesWaterfallHook(['pluginArgs']);
function createHtmlWebpackPluginHooks () {
return {
htmlWebpackPluginBeforeHtmlGeneration: new AsyncSeriesWaterfallHook(['pluginArgs']),
Copy link
Contributor

Choose a reason for hiding this comment

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

You should consider removing the htmlWebpackPlugin prefix, as it's now already namespace via getHooks.

Copy link
Owner Author

Choose a reason for hiding this comment

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

👍 I'll take a look

@jantimon jantimon deleted the webpack-4-static-hooks branch July 10, 2018 06:47
@lock
Copy link

lock bot commented Aug 9, 2018

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants