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

Don't overwrite html-webpack-plugin hooks when using more than one instance? #1031

Closed
danburzo opened this issue Aug 23, 2018 · 8 comments
Closed

Comments

@danburzo
Copy link

danburzo commented Aug 23, 2018

Expected behaviour

html-webpack-plugin should probably check if the hooks already exist and not overwrite them?

// setup hooks for webpack 4
if (compiler.hooks) {
compiler.hooks.compilation.tap('HtmlWebpackPluginHooks', compilation => {
const SyncWaterfallHook = require('tapable').SyncWaterfallHook;
const AsyncSeriesWaterfallHook = require('tapable').AsyncSeriesWaterfallHook;
compilation.hooks.htmlWebpackPluginAlterChunks = new SyncWaterfallHook(['chunks', 'objectWithPluginRef']);
compilation.hooks.htmlWebpackPluginBeforeHtmlGeneration = new AsyncSeriesWaterfallHook(['pluginArgs']);
compilation.hooks.htmlWebpackPluginBeforeHtmlProcessing = new AsyncSeriesWaterfallHook(['pluginArgs']);
compilation.hooks.htmlWebpackPluginAlterAssetTags = new AsyncSeriesWaterfallHook(['pluginArgs']);
compilation.hooks.htmlWebpackPluginAfterHtmlProcessing = new AsyncSeriesWaterfallHook(['pluginArgs']);
compilation.hooks.htmlWebpackPluginAfterEmit = new AsyncSeriesWaterfallHook(['pluginArgs']);
});
}

...to better support multiple instances of HtmlWebpackPlugin + any plugins.

(I am not sure if there are other implications to not overwriting the hooks, as it's the first time I deep-dive into the source code of webpack plugins / html-webpack plugins.)

Current behaviour

I have spent a few hours tracking down a problem introduced by the following setup:

webpack.config.js

plugins: [
  HtmlWebpackPlugin(config1),
  HtmlWebpackIncludeAssetsPlugin(config),
  // ...
  HtmlWebpackPlugin(config2),
  // ...
]

Where it only later became obvious that HtmlWebpackIncludeAssetsPlugin is bound to detached instances of the hooks that were later overwritten by the second HtmlWebpackPlugin.

Why this was so hard to deduce is that normally if you include a plugin before HtmlWebpackPlugin, it will throw an error to let you know, but when the plugin is sandwiched between two HtmlWebpackPlugin instances, it will silently not do anything.

Environment

Node.js v10.7.0
darwin 17.7.0
6.4.0
webpack@4.16.5 
html-webpack-plugin@3.2.0
@jantimon
Copy link
Owner

We will change the hook system to avoid the behavior - you can get a first impression if you take a look at the current alpha version

@danburzo
Copy link
Author

Thanks, sounds good!

@jantimon
Copy link
Owner

Should be fixed once #1032 is released

@jantimon
Copy link
Owner

jantimon commented Sep 4, 2018

Released in the current html-webpack-plugin@4.0.0-alpha.2 release

@j-5-s
Copy link

j-5-s commented Oct 25, 2018

Currently using html-webpack-plugin@4.0.0-alpha.2 and the hooks are not getting called:

compiler.hooks.compilation.tap(PLUGIN, (compilation) => {
   HtmlWebpackPlugin.getHooks(compilation).beforeAssetTagGeneration.tapAsync(PLUGIN, writeConfig);
   HtmlWebpackPlugin.getHooks(compilation).afterTemplateExecution.tapAsync(PLUGIN, writeJsObj);
});

writeConfig and writeJsObj do not get called. If pass in HtmlWebpackPlugin as a dependency when I invoke the plugin from the consuming app, it works. I assume it has something to do with this issue but maybe im just doing it wrong?

@jantimon
Copy link
Owner

@jamescharlesworth do you have multiple html-webpack-plugin versions in your node_modules folder?

@j-5-s
Copy link

j-5-s commented Oct 31, 2018

@jantimon yes, that looks to be the issue. My plugin had a demo folder with html-webpack-plugin in it's node_modules folder so it was in both

/node_modules/html-webpack-plugin
/demo/node_modules/html-webpack-plugin

Making it a peer dependency in my demo folder and removing it seems to have fixed the issue. Thank you!

@jantimon
Copy link
Owner

Cool 👍

@lock lock bot locked as resolved and limited conversation to collaborators Nov 30, 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

No branches or pull requests

3 participants