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

Guidance on porting a plugin to webpack v5? #11425

Closed
jeffposnick opened this issue Sep 3, 2020 · 27 comments
Closed

Guidance on porting a plugin to webpack v5? #11425

jeffposnick opened this issue Sep 3, 2020 · 27 comments

Comments

@jeffposnick
Copy link

I maintain workbox-webpack-plugin, and am trying to get a handle on the webpack v5 support story.

The plugin has code that generates one or more new files and then adds them to the compilation.assets once it's done: https://github.com/GoogleChrome/workbox/blob/6d38919ebbc9664327e19ff00302d805c8166170/packages/workbox-webpack-plugin/src/generate-sw.js#L282-L285

This work is done by tapping into compiler.hooks.emit.

When I tried the existing plugin code against webpack@next, I encountered:

[DEP_WEBPACK_COMPILATION_ASSETS] DeprecationWarning: Compilation.assets will be frozen in future, all modifications are deprecated.
BREAKING CHANGE: No more changes should happen to Compilation.assets after sealing the Compilation.
        Do changes to assets earlier, e. g. in Compilation.hooks.processAssets.
        Make sure to select an appropriate stage from Compilation.PROCESS_ASSETS_STAGE_*.

I'm at a loss for how to handle this change, as I can't find anything in https://github.com/webpack/changelog-v5/blob/master/MIGRATION%20GUIDE.md or https://github.com/webpack/changelog-v5 that explains what the equivalent code in v5 would be, or what PROCESS_ASSETS_STAGE_* refers to.

Is there an upgrade guide for plugin authors?

@alexander-akait
Copy link
Member

alexander-akait commented Sep 3, 2020

Yes, we need update this, emit hook should not be used for adding assets, example of usage:

{
  apply(compiler) { 
    if (isWebpack4()) { 
      // Keep compatibility
      compiler.emit('plugin-name', () => {})
    } else { 
     // webpacks export `webpack-sources` to avoid cache problems
     const { sources, Compilation } = require('webpack');

     compiler.hooks.compilation.tap('plugin-name', (compilation) => {
       compilation.hooks.processAssets.tapPromise(
          {
            name: 'plugin-name',
            // https://github.com/webpack/webpack/blob/master/lib/Compilation.js#L3280
            stage: Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL,
          },
          () => { 
            compilation.emitAsset('test.js', new sources.RawSource(data))
          }
        );
     });
   }
  }
}

If you need I can provide example for using webpack cache (persistent cache)

@alexander-akait
Copy link
Member

alexander-akait commented Sep 3, 2020

For supporting webpack@4 and webpack@5 sources you can use this code:

const webpack = require('webpack');

// webpack 5 exposes the sources property to ensure the right version of webpack-sources is used
const { RawSource } =
  // eslint-disable-next-line global-require
  webpack.sources || require('webpack-sources');

@jeffposnick
Copy link
Author

Thanks! I think it's working, at least at its most basic, but I will likely have some more questions 😄

First up, what's your recommended way of implementing isWebpack4() so that I know which of the two different code paths to execute inside of apply()?

@alexander-akait
Copy link
Member

You can use webpack.version[0] === '4' or just use duck typing like typeof compilation.hook.processAssets === "undefined"

@jeffposnick
Copy link
Author

I'd prefer to use duck typing, but I don't have a reference to the current compilation at the time at which I need to branch my logic between v4 and v5, so I can't do compilation.hook.processAssets.

I guess I'll go with webpack.version[0] === '4', but that feels a bit more brittle than the duck typing approach...

@alexander-akait
Copy link
Member

alexander-akait commented Sep 10, 2020

webpack.version[0] === '4' is fine, because webpack@4 will not have new features, only bug fixes, so it is safe for you

@jeffposnick
Copy link
Author

I'm working my way through our test suite, and running into an issue with https://github.com/GoogleChrome/workbox/blob/bb21e8208ec093dd66fb61b5fbcdfd67314eb2bc/test/workbox-webpack-plugin/node/generate-sw.js#L437-L487, which uses html-webpack-plugin along with workbox-webpack-plugin.

After updating to "html-webpack-plugin": "^4.4.1" and "webpack": "^5.0.0-beta.29", and changing our Workbox code to use compilation.hooks.processAssets with stage: webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL, I get the following:

Conflict: Multiple assets emit different content to the same filename service-worker.js
Conflict: Multiple assets emit different content to the same filename workbox-632a65e2.js

I can see that the code in workbox-webpack-plugin that creates assets (a lightly modified version of what I linked to in the OP) is being run twice, presumably first by the child compilation that html-webpack-plugin runs, and then again by the main, parent compilation.

(I'm not sure if this code was run twice by webpack v4 as well, and it just couldn't detect creating multiple assets with the same name.)

My goal is to ensure that my workbox-webpack-plugin code is run after the child compilation run by html-webpack-plugin is complete, and only run once. Would using a different stage value or hook help? Or is there any way to have my plugin opt-out of being used in the child compilation that html-webpack-plugin runs?

@alexander-akait
Copy link
Member

alexander-akait commented Sep 10, 2020

@jeffposnick Looks you adding assets, you need to use compiler.hooks.thisCompilation:

compiler.hooks.thisCompilation.tap(pluginName, (compilation) => {
  compilation.hooks.processAssets.tapPromise(
    { name: pluginName, stage: Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL}, 
    () => {
      // Code
    }
 )
})

But in same rare cases if you need to adding something for the child compilation you need to use compiler.hooks.compilation.tap, but it is not your case

@sokra
Copy link
Member

sokra commented Sep 11, 2020

webpack.version[0] === '4'

This might be problematic once we release 40.0.0...

@jeffposnick
Copy link
Author

jeffposnick commented Sep 15, 2020

@jeffposnick Looks you adding assets, you need to use compiler.hooks.thisCompilation:

workbox-webpack-plugin needs to execute its code after all assets are finalized from the main compilation and any child compilations, and it needs access to the final names (i.e. URLs) and hashes for each asset. Specifically, when run alongside html-webpack-plugin, it needs to know about all the HTML assets that are created. (But this applies to when it's run alongside any other plugin that performs a child compilation as well.)

I tried tapping into thisCompilation as suggested, but (not surprisingly) I don't see the html-webpack-plugin's assets in compilation.assets when I do that. I do see the html-webpack-plugin's child compilation in the compilation.children array, but when I try to get the HTML asset information from there, the asset name is something like __child-HtmlWebpackPlugin_0, not index.html.

As mentioned above, this is with html-webpack-plugin v4.4.1 and webpack v5.0.0-beta.29.

@alexander-akait
Copy link
Member

/cc @jantimon Why html-webpack-plugin create ugly assets names which cannot be analyzed?

@jeffposnick I think you need using this stage https://github.com/webpack/webpack/blob/master/lib/Compilation.js#L3381

Maybe you can create simple example or provide link on the problem, I will investigate this

@jeffposnick
Copy link
Author

I am generating an asset manifest for a service worker, so that stage does seem relevant 😄

webpack/lib/Compilation.js

Lines 3376 to 3381 in 9230acb

/**
* Summarize the list of existing assets.
* When creating new assets from this they should be fully optimized.
* e. g. creating an assets manifest of Service Workers.
*/
Compilation.PROCESS_ASSETS_STAGE_SUMMARIZE = 1000;

However, even when I set the stage to PROCESS_ASSETS_STAGE_SUMMARIZE, I'm not clear how to get the asset names/URLs I need from the html-webpack-plugin compilation.

The latest copy of my work-in-progress code is at https://github.com/GoogleChrome/workbox/blob/webpack-v5/packages/workbox-webpack-plugin/src/generate-sw.js (i.e. in the webpack-v5 branch).

Running it might be a bit annoying; try this after checking it out and switching to the right branch:

$ npm install
$ npx gulp build
$ npx mocha --grep='•' --timeout=60000 test/workbox-webpack-plugin/node/v5/generate-sw.js

That should run just the failing test of the html-webpack-plugin use case.

@alexander-akait
Copy link
Member

However, even when I set the stage to PROCESS_ASSETS_STAGE_SUMMARIZE, I'm not clear how to get the asset names/URLs I need from the html-webpack-plugin compilation.

You can use compilation.getAssets() to get assets, or do you need only html-webpack-plugin assets?

@jantimon
Copy link
Contributor

I am in Italy right now but I am planning to upgrade to setAssets soon - what is needed to make it easier for you?

The ugly name is only generated from the child generator and will be removed again - it's only used internally

@jeffposnick
Copy link
Author

jeffposnick commented Sep 15, 2020

You can use compilation.getAssets() to get assets, or do you need only html-webpack-plugin assets?

When using webpack v4, we rely on the code in get-manifest-entries-from-compilation.js to obtain and filter out the assets based on the user's configuration. In webpack v4, this code gives us access to all assets, including those created in child compilations.

Under the hood, the code calls:

compilation.getStats().toJson({
  assets: true,
  chunkGroups: true,
});

to get both the full set of assets as well as figure out each asset's associated chunkGroups (to allow for filtering based on chunk membership).

Will compilation.getAssets() give that same information, including chunkGroup associations?

Is there any way to get that information about all assets, whether they were created in a parent or a child compilation, in a single call (as was possible in webpack v4), or do I have to iterate over the child compilations?

It sounds like @jantimon is still in the progress of finalizing their work on html-webpack-plugin for webpack v5 compatibility, so I don't want to rush that along. But yes, having some way to get the final asset name for the HTML documents, rather than the internal ugly name, is important to our use case.

@alexander-akait
Copy link
Member

Is there any way to get that information about all assets, whether they were created in a parent or a child compilation, in a single call (as was possible in webpack v4), or do I have to iterate over the child compilations?

Hm, we can do it, we can add assets info - { childCompilation: true }, but I found it useless, because you don't know what specific compilation the files came from

@alexander-akait
Copy link
Member

alexander-akait commented Sep 17, 2020

Will compilation.getAssets() give that same information, including chunkGroup associations?

No, assets are more than chunks, because developers can add assets using copy-webpack-plugin and other plugins, but each chunk exists in assets

@sokra
Copy link
Member

sokra commented Sep 17, 2020

In webpack 5 you can still do getStats().toJson similar to webpack 4. I would add all: false to omit all other properties.

@jeffposnick
Copy link
Author

jeffposnick commented Sep 23, 2020

Hm, we can do it, we can add assets info - { childCompilation: true }, but I found it useless, because you don't know what specific compilation the files came from

For my use case, I need access to all of the assets that were created by a given invocation of webpack, regardless of whether they came from the main or a child compilation. I don't actually need to know which specific child compilation the assets originated from. What exactly is the syntax for enabling that—are you referring to something like compilation.getStats().toJson({childCompilation: true, ...})?

In webpack 5 you can still do getStats().toJson similar to webpack 4. I would add all: false to omit all other properties.

When I add all: false, my stats.assets array is empty, and stats.filteredAssets is set to 1. So I don't think I want to use all: false.

One additional breaking change that I've noticed with the getStats() output is that

compilation.getStats().toJson({
  assets: true,
  chunkGroups: true,
});

code when run in webpack v5 gives assets in the stats.assets array that have size: 0. (The items in stats.chunks do have what appear to be valid size properties.)

Is there a reason why assets obtained from getStats() wouldn't have valid size properties assigned to them at the time at which my plugin is running? I've switched to PROCESS_ASSETS_STAGE_SUMMARIZE. My plugin allows developers to filter out final assets that exceed a certain maximum size, so having access to that info is important.

I've updated the code in the webpack-v5 branch to the include the changes mentioned above, and the instructions to run just the test that's failing (which makes use of the assets[].size information from getStats()) is the same as before:

The latest copy of my work-in-progress code is at https://github.com/GoogleChrome/workbox/blob/webpack-v5/packages/workbox-webpack-plugin/src/generate-sw.js (i.e. in the webpack-v5 branch).

Running it might be a bit annoying; try this after checking it out and switching to the right branch:

$ npm install
$ npx gulp build
$ npx mocha --grep='•' --timeout=60000 test/workbox-webpack-plugin/node/v5/generate-sw.js

@jeffposnick jeffposnick changed the title Guidance on DEP_WEBPACK_COMPILATION_ASSETS in webpack v5? Guidance on porting a plugin to webpack v5? Sep 24, 2020
@sokra
Copy link
Member

sokra commented Sep 25, 2020

code when run in webpack v5 gives assets in the stats.assets array that have size: 0. (The items in stats.chunks do have what appear to be valid size properties.)

Is there a reason why assets obtained from getStats() wouldn't have valid size properties assigned to them at the time at which my plugin is running? I've switched to PROCESS_ASSETS_STAGE_SUMMARIZE. My plugin allows developers to filter out final assets that exceed a certain maximum size, so having access to that info is important.

size (and content) is determined during emitting. Determining the size requires to convert that asset to a buffer and that can be expensive. You can determine the size from compilation.getAsset(name).source.size() if you want to know it at this time. Maybe we should better set it to size: undefined so one can see that it's not determined yet.

When I add all: false, my stats.assets array is empty, and stats.filteredAssets is set to 1. So I don't think I want to use all: false.

I wonder why? That should only happen when there is not enough space for the assets to display. So only when assetsSpace is too small. But it should default to Infinity for toJson...

@sokra
Copy link
Member

sokra commented Sep 25, 2020

I wonder why you want to use the Stats anyway. Usually one doesn't use them before the compilation finished. You can access all info from the Compilation directly:

  • compilation.getAssets() will give all assets
    • Asset.source is the Source object, with size() method
    • Asset.name is the filename
    • Asset.info is a meta info object
      • Asset.info.development is true for dev-only assets
      • Asset.info.immutable is true when the asset has a hash (but be careful as not all things set this correctly)
  • compilation.chunkGroups will give all chunkGroups
    • ChunkGroup.options.name might be interesting
    • ChunkGroup.chunks too
      • Chunk.files lists all files directly related to this chunk (like .js, .css)
      • Chunk.auxiliaryFiles lists all files indirectly related to this chunk (like .png, .wasm)
    • ChunkGroup.childrenIterable lists all children of this chunk group (on demand loaded chunk groups)
      • Maybe you want to follow this to get the full graph reachable
  • You can also start here: compilation.entrypoints which lists all Entrypoints (extends ChunkGroup) by name.
  • Or here: compilation.namedChunkGroups which lists only chunk groups with names.
  • Also check webpack/types.d.ts for more info about the types.

Note that while webpack puts all assets somewhere in the graph, not all plugins do so. e. g. when using the copy-webpack-plugin there are some assets in the assets list which are only reachable from the chunk graph. More plugins do that. You probably also don't put the service-worker asset into the chunk graph.

@jeffposnick
Copy link
Author

I wonder why you want to use the Stats anyway. Usually one doesn't use them before the compilation finished. You can access all info from the Compilation directly:

All of those compilation.*Asset() methods are documented as Available since webpack 4.40.0. workbox-webpack-plugin currently has a minimum required version of webpack v4.0.0.

Around the time of the webpack v4.0.0 release, when I was working on the changes needed to move from webpack v3.x to webpack v4.0.0, using Stats to get the information I need about the final assets for a given compilation seemed like the best approach. (If there was a better approach, it was not possible for me to find it in the webpack documentation.)

I really don't want to have support two fundamentally different codepaths for obtaining and working with a list of assets within the same plugin, based on whether the user is running it in webpack v4 (prior to v4.40.0) or webpack v5.

It sounds like the recommended course of action for the next workbox-webpack-plugin release would be to ditch compatibility with anything earlier than webpack v4.40.0, and base all the logic on compilation.*Asset() instead of Stats. Does that sound about right?

@sokra
Copy link
Member

sokra commented Sep 25, 2020

anything earlier than webpack v4.40.0

For me this sounds like a good approach. Upgrading from 4.0 -> 4.40 should be straightforward for the user and I guess they already did so anyway. Anyway it wouldn't be a huge breaking change. Probably better than supporting two code paths, even while this isn't too bad. Many official plugins also support webpack 5 with two code paths... One could delete the old path in the next major version and there is no risk adding webpack 5 support when the old code path stays mostly untouched. Anyway you choice.

@jeffposnick
Copy link
Author

Just a quick update: things appear to be mostly working based on our test suite. Thanks for everyone's help! We have an alpha of the next major Workbox release coming up in the near future, and I think we'll be good for getting this code out there and encourage developers to test with webpack v5 (and webpack v4.40+).

Two open questions that I am left with:

  • I am not entirely sure how to account for the default publicPath: 'auto' change. I've updated our code to just ignore 'auto' when attempting to combine a publicPath and asset name to produce the correct URL that will end up in the list of URLs for the service worker to cache. There might be some nuisance I'm missing here, though.

  • I ended up tapping into thisCompilation as recommended, and iterated through the compilation.getAssets() from the top-level compilation as well as anything in compilation.children that exists. This gives me access to assets created by html-webpack-plugin, but the strange HTML asset names mentioned in Guidance on porting a plugin to webpack v5? #11425 (comment) makes me less than 100% sure that I'm doing the right thing. Hopefully the next html-webpack-plugin release will resolve that issue and I can confirm that they work well together.

@sokra
Copy link
Member

sokra commented Oct 2, 2020

  • I am not entirely sure how to account for the default publicPath: 'auto' change. I've updated our code to just ignore 'auto' when attempting to combine a publicPath and asset name to produce the correct URL that will end up in the list of URLs for the service worker to cache. There might be some nuisance I'm missing here, though.

With publicPath: "auto" the publicPath is not known at compile-time. Only at runtime. At runtime you can read the publicPath from __webpack_plugin_path__ (in the main thread). You can pass this value to the Service Worker and append it to the (relative) assets path.

Or you could calculate the publicPath yourself in the Service Worker from self.location assuming Service Worker is deployed at the same location as the other assets.

Or you can require that the user have to set the publicPath and throw an error with publicPath: "auto".

@jeffposnick
Copy link
Author

Thanks for the additional context around publicPath: 'auto'.

I don't think passing through __webpack_plugin_path__ from the main thread is going to be recommendation we make to all developers, but it's good to know how to do it if needed. Something like navigator.serviceWorker.register('sw.js?publicPath=' + __webpack_plugin_path__) might be the easiest way to have developers do that.

The workbox-webpack-plugin offers a configuration option for adding in a path prefix to the URLs it uses when fetching assets, so that's another escape hatch if needed.

@jeffposnick
Copy link
Author

workbox-webpack-plugin added webpack v5 compatibility in our v6.0.0-alpha.3 release.

I'm going to close this issue since I think all of my open questions have been resolved. Thanks for everyone's help.

I'll open follow-up issues with specific questions if anything unexpected comes up when users start testing with webpack v5.

bz2 added a commit to bz2/license-checker-webpack-plugin that referenced this issue Nov 14, 2020
Fixes microsoft#27

Changes based on dialogue in webpack/webpack#11425 between webpack and
workbook-webpack-plugin devs.

Split tap and asset logic in `LicenseCheckerWebpackPlugin.apply()` by
version of webpack. Use PROCESS_ASSETS_STAGE_ADDITIONAL for webpack 5.

Update peerDependencies to `^4.4.0 || ^5.4.0`.
bz2 added a commit to bz2/license-checker-webpack-plugin that referenced this issue Nov 15, 2020
Fixes microsoft#27

Changes based on dialogue in webpack/webpack#11425 between webpack and
workbook-webpack-plugin devs.

Split tap and asset logic in `LicenseCheckerWebpackPlugin.apply()` by
version of webpack. Use PROCESS_ASSETS_STAGE_ADDITIONAL for webpack 5.
Keep existing logic for webpack 4 largely unchanged.

Update peerDependencies to `^4.4.0 || ^5.4.0`.
unindented pushed a commit to microsoft/license-checker-webpack-plugin that referenced this issue Nov 15, 2020
Fixes #27

Changes based on dialogue in webpack/webpack#11425 between webpack and
workbook-webpack-plugin devs.

Split tap and asset logic in `LicenseCheckerWebpackPlugin.apply()` by
version of webpack. Use PROCESS_ASSETS_STAGE_ADDITIONAL for webpack 5.
Keep existing logic for webpack 4 largely unchanged.

Update peerDependencies to `^4.4.0 || ^5.4.0`.
domq pushed a commit to epfl-si/colorable that referenced this issue Jul 5, 2021
- Free-form extract `AdditionalAssetsPlugin` from static-site-generator-webpack-plugin, abstracting and modernizing the code as we go
- Let client code specify the desired additional assets as a callback (rather than having knowledge about the URL mapping ahead of time)
- Bestow “second-stage rocket” capabilities to the plugin, i.e. make it so that said client code can call a method-ish that retrieves already webpack'd JS
- Use Webpack API to emit the things that the aforementioned client code returns, as figured out from webpack/webpack#11425
- In order to avoid Webpack whining about `DeprecationWarning:  Compilation.assets will be frozen in future`, use an [`additionalPass` hook](https://webpack.js.org/api/compiler-hooks/#additionalpass), rather than an emit hook
- Document my journey
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

4 participants