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

Store custom hook in WeakMap for Webpack 5 compatability #191

Merged
merged 9 commits into from Oct 2, 2019

Conversation

@ian-craig
Copy link
Contributor

commented Sep 13, 2019

From Webpack 5, it's no longer possible to just assign new custom hooks onto the compiler.hooks object. The recommendation is to store these in a WeakMap keyed off the compiler instance.

https://github.com/webpack/changelog-v5/blob/master/README.md#hook-object-frozen

Running plugin v2.0.4 in Webpack v5.0.0-alpha.26 gives the following error:

TypeError: Cannot read property 'call' of undefined
    at ManifestPlugin.<anonymous> (...\node_modules\webpack-manifest-plugin\lib\plugin.js:191:53)
    at Hook.eval [as callAsync] (eval at create (...\node_modules\webpack\node_modules\tapable\lib\HookCodeFa
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (...\node_modules\webpack\node_modules\tapable\lib\Hook.js:18
    at Compiler.emitAssets (...\node_modules\webpack\lib\Compiler.js:507:19)
    at process.nextTick (...\node_modules\webpack\lib\Compiler.js:302:10)
    at process._tickCallback (internal/process/next_tick.js:61:11)

This change just moves the storage of webpackManifestPluginAfterEmit into a WeakMap.

Tested in Webpack v4.35.3 and v5.0.0-alpha.26.

Fixes #186

Update:
I did a second pass over this and created a static method to expose custom hooks to other plugins, following the recommended approach and a similar implementation to fork-ts-checker-webpack-plugin.

I'd expect other plugins to use hooks as follows:

ManifestPlugin.getCompilerHooks(compiler).afterEmit.tap('ManifestPlugin', (manifest) => {
   //...
});

This new behavior is supported for Webpack 4+, but I've also preserved compiler.hooks.webpackManifestPluginAfterEmit in Webpack 4

@mastilver

This comment has been minimized.

Copy link
Collaborator

commented Sep 24, 2019

Hi @ian-craig

Could you update .travis.yml to test webpack@5 ?

@codecov-io

This comment has been minimized.

Copy link

commented Sep 29, 2019

Codecov Report

Merging #191 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #191   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      4    +1     
  Lines         115    129   +14     
  Branches       22     26    +4     
=====================================
+ Hits          115    129   +14
Impacted Files Coverage Δ
lib/plugin.js 100% <100%> (ø) ⬆️
spec/helpers/webpack-version-helpers.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 421fa29...8648e73. Read the comment docs.

spec/plugin.spec.js Outdated Show resolved Hide resolved
spec/plugin.spec.js Outdated Show resolved Hide resolved
@mastilver mastilver merged commit bba9e24 into danethurber:master Oct 2, 2019
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to 421fa29
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.