Html-webpack-plugin: [v4-beta.2] Issue with HtmlWebpackPlugin.getHooks()

[jeffposnick]

I'm trying to upgrade preload-webpack-plugin to work with the new html-webpack-plugin v4 API. I'm running into some issues. You can see the WIP code at https://github.com/GoogleChromeLabs/preload-webpack-plugin/tree/html-plugin-updates

One issue is with using HtmlWebpackPlugin.getHooks() to get the hook to tap into. I've found that this does not work for me if I do the following inside my plugin:

const HtmlWebpackPlugin = require('html-webpack-plugin');
const hook = HtmlWebpackPlugin.getHooks(compilation).beforeEmit;

When running that code, hook is a valid instance of AsyncSeriesWaterfallHook, but tapping into it is a no-op, and the callback is never run.

Instead, the only way I've gotten this working is by passing in the exact same HtmlWebpackPlugin module reference that I use when configuring webpack, and getting HtmlWebpackPlugin.getHooks(compilation).beforeEmit from that. That returns a AsyncSeriesWaterfallHook that I can tap into and which will run my callback.

The problem with that is that I can't ask developers to pass in a reference to the HtmlWebpackPlugin module when configuring the preload-webpack-plugin.

(You can see the code in question at https://github.com/GoogleChromeLabs/preload-webpack-plugin/blob/c57b0b3521ca31091e0a77784bdb56a8ae8ba98d/src/index.js#L130-L141)

Part of the problem might be because I'm specifying "html-webpack-plugin": "^4.0.0-beta.2" as a devDependency in my plugin's package.json, and that's causing my plugin to load html-webpack-plugin from a different location then what's used in my webpack.config.js. But they're the same versions, and I was under the impression that HtmlWebpackPlugin.getHooks() was a static method that theoretically shouldn't rely on any state tied to a particular module reference.

What's the best practice here?

[jantimon]

You are right the problem occurs if the html-webpack-plugin is installed multiple times.

Do you have any idea how we might fix this?

[jeffposnick]

What kind of state information is being used by html-webpack-plugin's static getHooks() function?

It seems like if you're exposing a static function that's safe to be called from any arbitrary piece of code (including code run inside of a different plugin), then it should be pure, and its behavior should depend entirely on the inputs passed in, and not any other state information.

[jeffposnick]

It looks like the internal WeakMap used here is the state info that means tapping into to html-webpack-plugin v4's hooks will only work if the require('html-webpack-plugin') used inside of the custom plugin resolves to the same instance used inside of the webpack configuration.

https://github.com/jantimon/html-webpack-plugin/blob/65879ebe5061d821c9cbc9639915d855a612851b/lib/hooks.js#L79

I've worked around this where I was bumping up against it (in the preload-webpack-plugin unit tests) by explicitly making sure that the two places that require() is being done both end up pointing to the equivalent module id:

https://github.com/GoogleChromeLabs/preload-webpack-plugin/blob/a7f97bca6e9b4ab000e9986e7da7f6ded56228f3/test/webpack4-htmlplugin4/index.js#L19-L22

It seems slightly magical to have what appears to be a static method hanging off of your module, but whose behavior depends on some internal state, but... I don't have any specific suggestions as to how to change your implementation to resolve that. Hopefully in the real world, developers won't end up in scenarios where require('html-webpack-plugin) ends up resolving to two different module instances.

[jantimon]

We could try to check which html-webpack-plugin is used in the compiler.options.plugins. (Maybe also as part of the html-webpack-plugin getHooks code.

[jeffposnick]

We could try to check which html-webpack-plugin is used in the compiler.options.plugins. (Maybe also as part of the html-webpack-plugin getHooks code.

Hmmm, that's interesting. So the following does work around the issues I was seeing, but it's a bit less than ideal:

if (!hook) {
  const [HtmlWebpackPlugin] = compiler.options.plugins.filter(
      (plugin) => plugin.constructor.name === 'HtmlWebpackPlugin');
  assert(HtmlWebpackPlugin, 'Unable to find an instance of ' +
      'HtmlWebpackPlugin in the current compilation.');
  hook = HtmlWebpackPlugin.constructor.getHooks(compilation).beforeEmit;
}

(This might not work properly if there were multiple HtmlWebpackPlugin instances in compiler.plugins, among other issues.)

A system in which the state info that's needed could be derived purely from the compiler + compilation objects instead of a system in which require('html-webpack-plugin') always resolves to the same module seems 👍 .

[jantimon]

Yes - maybe the webpack team could provide a api for that e.g.

compiler.getPlugins().HtmlWebpackPlugin

or we could also allow to pass plugins to the HtmlWebpackPlugin e.g.

new HtmlWebpackPlugin({
   plugins: [...]
})

However I would definitely prefer option 1

[jeffposnick]

Just a quick ping about your last comment—is there an open issue with the webpack team about option 1? I know that, e.g., @TheLarkInn has been responsive in the past about these sorts of issues.

Failing that, option 2 at least adds a possibility of coupling between HtmlWebpackPlugin instances and plugins that need to rely on HtmlWebpackPlugin state. Though I'm not exactly sure how the HtmlWebpackPlugin instance would be passed through to the plugins in that array?

[jantimon]

Maybe @sokra or @TheLarkInn might help out here because they came up with the idea of this new event system

[jeffposnick]

FWIW, I've given up hope that this might be addressed on the webpack level, and I'm just going to ship preload-webpack-plugin with the hack described in https://github.com/jantimon/html-webpack-plugin/issues/1091#issuecomment-434708455

[jantimon]

@brunocodutra used the same hack for the favicons webpack plugin:
https://github.com/jantimon/favicons-webpack-plugin/blob/f221480b4bb322a8d895583c87efdf2b65d865a3/src/index.js#L65-L68

[colbyfayock]

not sure if this would help you out, but in case it helps someone else out stumbling upon this issue, for my package i moved html-webpack-plugin as a peerDependency and devDependency to avoid the 2nd installation of this in the installee's application, which was causing the conflicts for me

[soulofmischief]

@jantimon Will the same work for html-webpack-harddisk-plugin?

[jantimon]

Right now the html-webpack-plugin is offering the following method:

https://github.com/jantimon/html-webpack-plugin/blob/655cbcdb9a81db774dd1a66fada085dd6c20dee0/lib/hooks.js#L71-L85

Maybe we should add the code from @jeffposnick here too?

const HtmlWebpackPlugin = compilation.compiler.options.plugins.find(
      (plugin) => plugin.constructor.name === 'HtmlWebpackPlugin'
)

e.g.:

function getHtmlWebpackPluginHooks (compilation) { 
   let hooks = htmlWebpackPluginHooksMap.get(compilation); 
   // Setup the hooks only once 
   if (hooks === undefined) { 
    const HtmlWebpackPlugins = compilation.compiler.options.plugins.filter(
       (plugin) => plugin.constructor.name === 'HtmlWebpackPlugin'
    );
    // Try to find out if the current package is used
    const isCurrentVersionUsed = HtmlWebpackPlugins.some((plugin) => plugin.getHtmlWebpackPluginHooks === getHtmlWebpackPluginHooks);
    // Try to get the hooks from the actual used plugin version
    if (isCurrentVersionUsed || HtmlWebpackPlugins.length === 0) {
      hooks = createHtmlWebpackPluginHooks(); 
    } else {
      // Return the hooks of the first used html webpack plugin
      hooks = HtmlWebpackPlugins[0].getHtmlWebpackPluginHooks(compilation);
    }
    htmlWebpackPluginHooksMap.set(compilation, hooks); 
   } 
   return hooks; 
}

[stale[bot]]

This issue had no activity for at least half a year. It's subject to automatic issue closing if there is no activity in the next 15 days.

[jahed]

Just encountered this issue while migrating one of my plugins. Wanted to mention another issue caused by using a module reference. Linked dependencies (e.g. npm link) that have HtmlWebpackPlugin in their devDependency (as it's a peerDependency) will also end up using the wrong reference.

I use npm link myself for some manual testing before publishing a new version of my plugin.

Here's my solution. It's not pretty but it works for anyone wanting to migrate first before improving on design:

import type { Compiler } from 'webpack'
import type { default as HtmlWebpackPluginInstance } from 'html-webpack-plugin'

const extractHtmlWebpackPluginModule = (compiler: Compiler): typeof HtmlWebpackPluginInstance | null=> {
  const htmlWebpackPlugin = (compiler.options.plugins || []).find(
    (plugin) => plugin.constructor.name === 'HtmlWebpackPlugin'
  ) as typeof HtmlWebpackPluginInstance | undefined
  if (!htmlWebpackPlugin) {
    return null
  }
  const HtmlWebpackPlugin = htmlWebpackPlugin.constructor
  if (!HtmlWebpackPlugin || !('getHooks' in HtmlWebpackPlugin)) {
    return null
  }
  return HtmlWebpackPlugin as typeof HtmlWebpackPluginInstance
}

For my usecase the cleanest solution would be to pass HWP into other plugins and have instance-level hooks instead of global ones. Global state always becomes a mess when dealing with dependencies. :sweat:

[jantimon]

@jahed passing the webpack into the plugin is a good idea, actually that's exactly how the facebook team is doing it:

https://github.com/facebook/create-react-app/blob/a4fa63fcc1fb97fa50778b7c1a73a01da3a3e022/packages/react-scripts/config/webpack.config.js#L606-L612

The only downside is that is not usable from the cli e.g.:

webpack --plugin html-webpack-plugnin --plugin fancy-plugin

[AntonioRedondo]

Same thing here. As of "html-webpack-plugin": "^4.3.0" and "webpack": "^4.43.0" my beforeEmit callback isn't called. Above @jeffposnick https://github.com/jantimon/html-webpack-plugin/issues/1091#issuecomment-434708455 solution works though.

[sokra]

html-webpack-plugin must be a peerDependency.
npm link won't work for these scenario, it's broken for peerDependencies. Try file: dependency or portal:.

[stale[bot]]

This issue had no activity for at least half a year. It's subject to automatic issue closing if there is no activity in the next 15 days.