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

Strange asset name when used with webpack v5.0.0-rc.3 #1522

Closed
jeffposnick opened this issue Oct 1, 2020 · 17 comments
Closed

Strange asset name when used with webpack v5.0.0-rc.3 #1522

jeffposnick opened this issue Oct 1, 2020 · 17 comments

Comments

@jeffposnick
Copy link

Expected behaviour

html-webpack-plugin, when used with webpack v5.0.0-rc.3, should emit assets that have proper names, corresponding to the assets' URLs.

Current behaviour

As mentioned at webpack/webpack#11425 (comment), I'm seeing assets with names like __child-HtmlWebpackPlugin_0 when my own webpack plugin inspects the assets created by the html-webpack-plugin child compilation.

Environment

html-webpack-plugin v4.5.0 and webpack v5.0.0-rc.3.

Relevant Links

The test case that exercises this code is https://github.com/GoogleChrome/workbox/blob/webpack-v5/test/workbox-webpack-plugin/node/v5/generate-sw.js#L450-L497

@alexander-akait
Copy link
Collaborator

alexander-akait commented Oct 12, 2020

@jantimon please focus on it, other plugins don't work

@jantimon
Copy link
Owner

Thanks for the report I'll try to find out what causes the issue 👍

Are you already using the html-webpack-plugin hooks to get the correct assets?

@jeffposnick
Copy link
Author

No, in the case of workbox-webpack-plugin I'm not using any html-webpack-plugin hooks. I'm just hooking into the parts of the webpack compilation that the webpack team had recommended, and inspecting any child compilations, with the understanding that my code needs to work with any plugin that creates assets in a child compilation.

If there are html-webpack-plugin-specific hooks that other plugins needs to use in order to get the correct info about HTML assets, could you point me to the documentation?

@jayaddison
Copy link

This behaviour might also exist in environments that haven't yet upgraded to webpack v5 -- I'm seeing it appear during a build that uses:

In the list of filenames that workbox produces for precaching, {'revision':null,'url':'__child-HtmlWebpackPlugin_0'} appears as one of the entries. I'll try to provide a small repro case tomorrow.

@csvan
Copy link

csvan commented Oct 12, 2020

I'm seeing this behavior when used in conjunction with the Webpack compression plugin. The setup worked without issue under Webpack 4. See #1525

@jantimon
Copy link
Owner

jantimon commented Oct 13, 2020

@jeffposnick here are all events: https://github.com/jantimon/html-webpack-plugin#events

I hope that switching to the new asset api will allow us to get rid of {'revision':null,'url':'__child-HtmlWebpackPlugin_0'} even for the webpack hooks but it will take me some more time to implement it.

@csvan
Copy link

csvan commented Oct 13, 2020

@jantimon does this mean that https://github.com/webpack-contrib/compression-webpack-plugin needs to make the same adjustments?

@jantimon
Copy link
Owner

jantimon commented Oct 13, 2020

The main issue is this part:

helperAssetNames.forEach((helperFileName) => {
delete mainCompilation.assets[helperFileName];
});

Although we delete the __child-HtmlWebpackPlugin_0 file it is still there.

Webpack 5 came up with a new asset api.
Hopefully this new api will solve the problem without forcing everyone to write specific html-webpack-plugin code

@jeffposnick
Copy link
Author

@jeffposnick here are all events: https://github.com/jantimon/html-webpack-plugin#events

Oh, right, I remember those from the context of another webpack plugin whose goal was to integrate directly with html-webpack-plugin. I don't think that workbox-webpack-plugin, compression-webpack-plugin, etc. should be using them, though, since we would ideally just treat assets created by html-webpack-plugin the same as assets created by any other part of the webpack compilation.

Webpack 5 came up with a new asset api. Hopefully this new api will solve the problem without forcing everyone to write specific html-webpack-plugin code

FWIW, the new Asset API was introduced in webpack v4.40.0. What I ended up doing with workbox-webpack-plugin when adding support for webpack v5 was to bump the minimum required version of webpack to v4.40.0 (as part of a major semver release). Having two different codepaths for something as fundamental as dealing with assets seemed error prone. I don't know if you have the flexibility of requiring at least webpack v4.40.0+ with html-webpack-plugin, though.

@jayaddison
Copy link

A minimal repro case for this is available in https://github.com/jayaddison/htmlwebpackplugin-1522-repro

@jantimon
Copy link
Owner

Can you please see if this is fixed with [email protected] ?
Please see also #1527

@jayaddison
Copy link

@jantimon This is working well for me; I now see the correct behaviour -- no unusual asset names in the workbox precache list -- when using:

Verified using https://github.com/jayaddison/htmlwebpackplugin-1522-repro/commit/b716aae855a26742b9d6dc37b600f4efbaba12ad

NB: This might not be a concern since you list peerDependencies: webpack@^5.1.2, but I did notice that this change does not appear to be backwards-compatible with [email protected] (verified using https://github.com/jayaddison/htmlwebpackplugin-1522-repro/commit/402160d0405efb9a0bb74d15e6d804b1ee732308).

For Webpack 4.44.2, the following build output appears:

$ npx webpack
Cannot read property 'tap' of undefined
$ echo $?
1

Looks good for Webpack 5 though. Owe you a 🍺

Probably worth waiting for feedback from Jeff as well; his project likely has more stringent requirements and quality control than mine :)

@jantimon
Copy link
Owner

Oh sorry 😨 : - I just saw that there is still one thing missing - so you will have to wait for the next alpha

And yes support for webpack 4 has been removed.

@jantimon
Copy link
Owner

jantimon commented Oct 16, 2020

Now it's part of [email protected]

@jeffposnick
Copy link
Author

jeffposnick commented Oct 16, 2020

I just ran our workbox-webpack-plugin test suite with [email protected] as the dependency, and our tests pass. 👍 I was able to find all the expected assets in the main compilation, and did not need to look in the child compilations

Looks good! Thanks for working on this!

@jantimon
Copy link
Owner

Awesome thank you Jeff for testing it 👍
There are still some tests failing for the current alpha release but I hope I will be able to release a beta next week.

@csvan
Copy link

csvan commented Oct 16, 2020

@jantimon will test this for the compression plugin tonight and close that issue as well if its OK now (which seems very likely). Thanks for your hard work ❤️

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

No branches or pull requests

5 participants