Skip to content

fix: be compatible with webpack5#48

Open
dogedefi wants to merge 1 commit into
erikdesjardins:masterfrom
dogedefi:master
Open

fix: be compatible with webpack5#48
dogedefi wants to merge 1 commit into
erikdesjardins:masterfrom
dogedefi:master

Conversation

@dogedefi

Copy link
Copy Markdown

Something like index.html or or more will be lost in packaging.

Comment thread index.js
if (isWebpack4 || isWebpack5) {
compiler.hooks.emit.tapAsync(ZipPlugin.name, process);
} else {
compiler.hooks.thisCompilation.tap(ZipPlugin.name, compilation => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We use the processAssets hook in Webpack 5 because adding assets after that hook causes a deprecation warning (#35). This change would bring back that warning.

As far as I can tell, processAssets is the latest (most delayed) hook we can use, since compilation.assets is frozen right after invoking processAssets, and there are no other async hooks in between: https://github.com/webpack/webpack/blob/18c3590b28fcaaa84b63f18968b09602e4a7e259/lib/Compilation.js#L3055-L3070

If the resulting zip is missing files, it likely means that another plugin you're using is emitting assets at an inappropriate time, either after the processAssets hook, or at a later stage than PROCESS_ASSETS_STAGE_OPTIMIZE_TRANSFER. (e.g. using stage: Infinity).

We can't change the stage that this plugin runs at without risking breaking changes for existing users, who may have other plugins at a later stage consuming the zip file generated by this one. (The entire stage/hook ordering system in webpack is incredibly fragile because of this, but there's not much that can be done as an API consumer...)

I might consider adding a config option to this plugin to override the stage, although it sucks to have to configure that as a user...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants