Dependency Extraction Webpack Plugin: Make the plugin work when using `optimizations.runtimeChunk = 'single'` #26214

stefanfisk · 2020-10-16T18:29:17Z

Description

A quick proof of concept for fixing #24352, as requested by @sirreal.

How has this been tested?

I'm currently using this patch daily, and it works for me. Beyond that I don't know.

Types of changes

The asset filename logic has been modified, and the assets are now pushed to the entrypoint chunks instead of the runtime chunk.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

sirreal · 2020-11-13T16:13:22Z

Thanks for this @stefanfisk 👍

What do you think of adding a test that fails like in your example repository? You can add a directory here with a minimal example project: https://github.com/WordPress/gutenberg/tree/master/packages/dependency-extraction-webpack-plugin/test/fixtures

stefanfisk · 2020-11-17T07:49:08Z

@sirreal I've added a test, but it is not failing since the PR already contained a fix.

I also added a small change to which chunk's hash is used in the asset file.

stefanfisk · 2020-11-17T07:50:01Z

Oh, and I changed the snapshot format to contain the asset filenames, as that was part of the issue.

packages/dependency-extraction-webpack-plugin/lib/index.js

stefanfisk · 2020-12-30T10:13:45Z

I just cleaned the code up, and added use of the Webpack 5 Entrypoint.getEntrypointChunk() function. For Webpack 4 it still uses the dodgy way of finding the entrypoint chunk.

I also added an option for specifying the asset output filename separately, so that one can use hashes in the js output filenames and still have predictable asset filenames.

gziolo · 2021-02-01T06:26:35Z

@stefanfisk, many thanks for opening this PR. It seems like a complex issue to address so I appreciate all the time invested on it since it works for you already 😃

I have one general question to better understand the context. What’s the purpose of runtimeChunk set to single?

stefanfisk · 2021-02-01T06:49:12Z

runtimechunk='single' results in all loaded entrypoints sharing instances of modules, rather than Webpack initialising every module separately for each entrypoint if multiple entrypoints are loaded at the same time. So runtime in this context refers to the Webpack runtime.

This for example allows you to use an entrypoint for admin that is always loaded for admin pages, a block-editor entrypoint that is only loaded by the block editor, all while they share instances of stateful modules.

gziolo

@slightlyfaulty left a comment on the #24352 confirming that this PR fixes the issue. I went through the proposed changes and they look good. All the newly added unit tests bring a lot of value. I think we can proceed with merging those changes as soon as the PR gets rebased with trunk and all conflicts are resolved. @stefanfisk, thank you for keeping this PR opened for so long.

gziolo · 2021-11-24T08:43:49Z

packages/dependency-extraction-webpack-plugin/lib/index.js

+				let assetFilename;
+
+				if ( outputFilename ) {
+					assetFilename = compilation.getPath( outputFilename, {


Should we enforce a matching outputFormat based on the extension used in the file? At least when it ends with .php or .json. We could also leave it as is - the developer using the plugin still would have to pass two options to set everything correctly when using non-default output format.

gziolo · 2021-11-24T08:44:08Z

packages/dependency-extraction-webpack-plugin/test/build.js

 					expect(
 						fs.readFileSync( assetFile, 'utf-8' )
-					).toMatchSnapshot( 'Asset file should match snapshot' );
+					).toMatchSnapshot(
+						`Asset file '${ assetBasename }' should match snapshot`


Looks like a good idea in general 👍🏻

gziolo · 2021-11-24T08:48:55Z

packages/dependency-extraction-webpack-plugin/README.md

@@ -116,6 +116,13 @@ module.exports = {

 The output format for the generated asset file. There are two options available: 'php' or 'json'.

+##### `outputFilename`


With this new option proposed combinedOutputFile plays a nearly identical role. At some point, we might want to deprecate combinedOutputFile and consolidate the handling in outputFilename.

This results in the asset files being named after the entry points, instead of the runtime chunk when using `runtimeChunk = 'single'`. Fixes WordPress#24352

Allows specifying the asset filename format separately from the output filename format. This is useful when the output filenames include a hash.

gziolo · 2021-11-24T10:01:43Z

@stefanfisk, thas was quick! CI checks are still green which means we are good to go 🎉

github-actions · 2021-11-24T10:12:42Z

Congratulations on your first merged pull request, @stefanfisk! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

stefanfisk · 2021-11-24T10:15:53Z

@gziolo maintaining a local copy of the plugin in my site repos has been a thorn in my side for over a year now (npm keeps installing webpack 4 and 5 at the same time when I least expect it).

At this point I am not missing any chance of getting this merged 😸

stefanfisk · 2021-11-24T10:16:54Z

I'm not clear on the release schedule for the packages, do you have any idea of when this PR will be available?

gziolo · 2021-11-24T10:33:31Z

It's hard to tell because the WordPress 5.9 release was moved to January 25th: https://make.wordpress.org/core/2021/11/22/wordpress-5-9-revised-release-schedule/. We are now in feature freeze mode and the @noisysocks (the editor tech lead for WordPress 5.9 release) manually cherry-picks only bug fixes to the branch that is used for npm releases. It all depends on when he decides to create a wp/5.9 branch so we could get back to the regular cadences of releasing changes every two weeks. I plan to propose a revised approach for the next major WordPress releases starting from 6.0, so we can continue doing more regular npm publishing from trunk.

sirreal · 2021-11-24T14:50:35Z

Thanks for the work on this @stefanfisk and especially thank you for your patience and perseverance ❤️

noisysocks · 2021-11-25T00:55:39Z

It all depends on when he decides to create a wp/5.9 branch so we could get back to the regular cadences of releasing changes every two weeks. I plan to propose a revised approach for the next major WordPress releases starting from 6.0, so we can continue doing more regular npm publishing from trunk.

I'll stick to the documented process which is to branch wp/trunk → wp/5.9 at the time of WP 5.9 RC 1 (January 4). I think best not to tinker with our processes mid-release. Agree that we ought to revise it for future releases though.

jsnajdr · 2022-05-09T12:34:42Z

Hi @stefanfisk 👋 I noticed this PR now, long after it was merged, when working on something else in the dependency extraction plugin, and I wanted to ask: how does a practical project that runs into this issue look like?

Suppose there is a project that produces two entrypoint scripts, a.js and b.js, and also an entrypoint script, runtime.js, and the dependency extraction plugin produces two asset files, a.asset.php and b.asset.php. Who consumes such a project and how? How are the scripts delivered and executed? If I use the documented way to enqueue the scripts in WordPress, then only the a.js and b.js scripts will ever be enqueued, but PHP doesn't know about the runtime.js, because no .asset.php file mentions it. The code will likely fail, because part of it is missing.

So, if it's not WordPress enqueuing the scripts in a standard way, who else cares about the .asset.php files?

A similar issue will arise if there are two entrypoints that have any shared chunk in common, not just runtime. To load the entrypoint, the consumer is supposed to load 2+ JS files, but any .assets.php currently mentions only one.

gziolo · 2022-05-09T14:38:16Z

@jsnajdr, the interesting part is that I was exploring using optimization.runtimeChunk set to single in the context of React Fast Refresh which needs it to work correctly with multiple entry points. See my comment for more details #25188 (comment). I included how my build folder looked like during testing and indeed it didn't care to create an asset file for runtime.js nor mark it as a dependency:

stefanfisk · 2022-05-09T16:23:06Z

@jsnajdr I already had my own script loader that parsed a JSON manifest generated by webpack-bundle-analyzer, so I was only using the *.asset.php files for getting the WordPress dependencies. I gave it a go, but I couldn't figure out any way of getting the equivalent functionality without using dependency-extraction-webpack-plugin.

The reason for using the custom script loader was that I was splitting the npm deps from the main bundle to minimize the impact of doing deploys that modified the site's JS code (which I was doing almost daily at the time).

jeffpaul · 2022-05-10T19:21:53Z

@stefanfisk checking back to see if you can confirm your WordPress.org username so we can ensure you are properly credited in the WordPress 6.0 release coming out on May 24th. You can use the links above or share information directly with me and I'll ensure it's included correctly within the release. And thanks again for your contribution, it's greatly appreciated!

stefanfisk · 2022-05-11T04:24:46Z

@jeffpaul done!

jsnajdr · 2022-05-11T11:47:12Z

Thanks @stefanfisk, that helps me a lot. In other words, you were looking at the asset.php files only to learn what are an entrypoint's external dependencies, and that info is indeed reliable. But if the entrypoint has two or more chunks, like it always has with a shared runtime chunk, the asset.php file won't tell you that you need to load the runtime script, too. But you are getting the information from another source. That explains everything 👍

stefanfisk · 2022-05-11T11:59:54Z

@jsnajdr I'm no longer using this code, so I'm not sure if it works, but here's a gist describing exactly how it all worked: https://gist.github.com/stefanfisk/bb4e045f3d15829a377916dc5fee9acc

Feel free to use it as you wish.

jsnajdr · 2022-05-11T12:54:59Z

here's a gist describing exactly how it all worked

Thanks, that gist shows very well the shortcomings of the current asset.php files. The gist has 300 lines of code, while ideally it would be 7 lines like this:

$assets = require( "$dir/assets.php" );
foreach ( $assets['scripts'] as $script ) {
  wp_enqueue_script( $script['handle'], $script['src'], $script['deps'], $script['ver'] );
}
foreach ( $assets['styles'] as $style ) {
  wp_enqueue_style( $style['handle'], $style['src'], $style['deps'], $style['ver'] );
}

gziolo · 2022-05-11T13:28:00Z

here's a gist describing exactly how it all worked

Thanks, that gist shows very well the shortcomings of the current asset.php files. The gist has 300 lines of code, while ideally it would be 7 lines like this:
$assets = require( "$dir/assets.php" );
foreach ( $assets['scripts'] as $script ) {
  wp_enqueue_script( $script['handle'], $script['src'], $script['deps'], $script['ver'] );
}
foreach ( $assets['styles'] as $style ) {
  wp_enqueue_style( $style['handle'], $style['src'], $style['deps'], $style['ver'] );
}

That's a simplified version because you don't want always to enqueue all those assets, but I agree on a principle that it would be great to have a way to register (wp_register_script or wp_register_style) all of them this way.

jsnajdr · 2022-05-11T13:57:59Z

That's a simplified version because you don't want always to enqueue all those assets

Not sure what you mean, because in general I need to enqueue all the scripts that webpack produced? There are some additional assets like sourcemaps or .LICENCE.txt files, but if something is a script or style, it needs to be there.

gziolo · 2022-05-11T14:09:11Z

That's a simplified version because you don't want always to enqueue all those assets

Not sure what you mean, because in general I need to enqueue all the scripts that webpack produced? There are some additional assets like sourcemaps or .LICENCE.txt files, but if something is a script or style, it needs to be there.

It might be correct for your use case. In general, when you have multiple entry points you might want to consume only a subset of them on different pages. That's why I suggested that you register all assets but enqueue only the entry points when necessary and WP would pick all dependencies out of the box.

stefanfisk · 2022-05-11T14:23:38Z

A basic example is splitting editor assets from frontend assets.

…

On Wed, May 11, 2022 at 4:09 PM Greg Ziółkowski ***@***.***> wrote: That's a simplified version because you don't want always to enqueue all those assets Not sure what you mean, because in general I need to enqueue all the scripts that webpack produced? There are some additional assets like sourcemaps or .LICENCE.txt files, but if something is a script or style, it needs to be there. It might be correct for your use case. In general, when you have multiple entry points you might want to consume only a subset of them on different pages. That's why I suggested that you register all assets but enqueue only the entry points when necessary and WP would pick all dependencies out of the box. — Reply to this email directly, view it on GitHub <#26214 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGKC7I2Y6TJJILQBWND5BTVJO5RFANCNFSM4STVNTBQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

jsnajdr · 2022-05-11T14:27:19Z

That's why I suggested that you register all assets but enqueue only the entry points when necessary and WP would pick all dependencies out of the box.

I see, there can be a big project with multiple entrypoints and it always registers all its assets, but when requesting "please enqueue stuff for entrypoint a," it will enqueue only a subset.

jsnajdr · 2022-05-11T14:27:57Z

I created #41002 where I'm trying to solve some issues discussed here.

stefanfisk requested a review from gziolo as a code owner October 16, 2020 18:29

gziolo added [Tool] Dependency Extraction Webpack Plugin /packages/dependency-extraction-webpack-plugin [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible labels Oct 24, 2020

stefanfisk force-pushed the fix/24352-dependency-extraction-runtimechunk-single branch from 3b3d69a to ce1e442 Compare November 17, 2020 07:47

stefanfisk commented Nov 19, 2020

View reviewed changes

packages/dependency-extraction-webpack-plugin/lib/index.js Outdated Show resolved Hide resolved

stefanfisk force-pushed the fix/24352-dependency-extraction-runtimechunk-single branch from ce1e442 to d5b5042 Compare December 30, 2020 10:10

Base automatically changed from master to trunk March 1, 2021 15:44

kjellr mentioned this pull request Nov 23, 2021

Consider a root-level site padding solution that still lets some items go full-width #35607

Closed

slightlyfaulty mentioned this pull request Nov 23, 2021

dependency-extraction-webpack-plugin breaks when using optimizations.runtimeChunk = 'single' #24352

Closed

gziolo reviewed Nov 24, 2021

View reviewed changes

gziolo approved these changes Nov 24, 2021

View reviewed changes

gziolo added [Type] Bug An existing feature does not function as intended and removed [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible labels Nov 24, 2021

gziolo changed the title ~~Fix #24352 - dependency-extraction-webpack-plugin breaks when using optimizations.runtimeChunk = 'single'~~ Dependency Extraction Webpack Plugin: Make the plugin work when using optimizations.runtimeChunk = 'single' Nov 24, 2021

stefanfisk added 3 commits November 24, 2021 10:47

Include asset filenames in snapshots

051f187

Use entrypoint chunk instead of runtime chunk

03149f9

This results in the asset files being named after the entry points, instead of the runtime chunk when using `runtimeChunk = 'single'`. Fixes WordPress#24352

Add outputFilename option

a6f01f2

Allows specifying the asset filename format separately from the output filename format. This is useful when the output filenames include a hash.

stefanfisk force-pushed the fix/24352-dependency-extraction-runtimechunk-single branch from d5b5042 to a6f01f2 Compare November 24, 2021 09:49

gziolo merged commit d2d673d into WordPress:trunk Nov 24, 2021

github-actions bot added this to the Gutenberg 12.1 milestone Nov 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dependency Extraction Webpack Plugin: Make the plugin work when using `optimizations.runtimeChunk = 'single'` #26214

Dependency Extraction Webpack Plugin: Make the plugin work when using `optimizations.runtimeChunk = 'single'` #26214

stefanfisk commented Oct 16, 2020 •

edited by gziolo

Loading

sirreal commented Nov 13, 2020

stefanfisk commented Nov 17, 2020

stefanfisk commented Nov 17, 2020

stefanfisk commented Dec 30, 2020

gziolo commented Feb 1, 2021

stefanfisk commented Feb 1, 2021

gziolo left a comment

gziolo Nov 24, 2021

gziolo Nov 24, 2021

gziolo Nov 24, 2021

gziolo commented Nov 24, 2021

github-actions bot commented Nov 24, 2021

stefanfisk commented Nov 24, 2021

stefanfisk commented Nov 24, 2021

gziolo commented Nov 24, 2021 •

edited

Loading

sirreal commented Nov 24, 2021

noisysocks commented Nov 25, 2021 •

edited

Loading

jsnajdr commented May 9, 2022

gziolo commented May 9, 2022 •

edited

Loading

stefanfisk commented May 9, 2022

jeffpaul commented May 10, 2022

stefanfisk commented May 11, 2022

jsnajdr commented May 11, 2022

stefanfisk commented May 11, 2022

jsnajdr commented May 11, 2022

gziolo commented May 11, 2022

jsnajdr commented May 11, 2022

gziolo commented May 11, 2022

stefanfisk commented May 11, 2022 via email

jsnajdr commented May 11, 2022

jsnajdr commented May 11, 2022

		@@ -116,6 +116,13 @@ module.exports = {

		The output format for the generated asset file. There are two options available: 'php' or 'json'.

		##### `outputFilename`

Dependency Extraction Webpack Plugin: Make the plugin work when using optimizations.runtimeChunk = 'single' #26214

Dependency Extraction Webpack Plugin: Make the plugin work when using optimizations.runtimeChunk = 'single' #26214

Conversation

stefanfisk commented Oct 16, 2020 • edited by gziolo Loading

Description

How has this been tested?

Types of changes

Checklist:

sirreal commented Nov 13, 2020

stefanfisk commented Nov 17, 2020

stefanfisk commented Nov 17, 2020

stefanfisk commented Dec 30, 2020

gziolo commented Feb 1, 2021

stefanfisk commented Feb 1, 2021

gziolo left a comment

Choose a reason for hiding this comment

gziolo Nov 24, 2021

Choose a reason for hiding this comment

gziolo Nov 24, 2021

Choose a reason for hiding this comment

gziolo Nov 24, 2021

Choose a reason for hiding this comment

gziolo commented Nov 24, 2021

github-actions bot commented Nov 24, 2021

stefanfisk commented Nov 24, 2021

stefanfisk commented Nov 24, 2021

gziolo commented Nov 24, 2021 • edited Loading

sirreal commented Nov 24, 2021

noisysocks commented Nov 25, 2021 • edited Loading

jsnajdr commented May 9, 2022

gziolo commented May 9, 2022 • edited Loading

stefanfisk commented May 9, 2022

jeffpaul commented May 10, 2022

stefanfisk commented May 11, 2022

jsnajdr commented May 11, 2022

stefanfisk commented May 11, 2022

jsnajdr commented May 11, 2022

gziolo commented May 11, 2022

jsnajdr commented May 11, 2022

gziolo commented May 11, 2022

stefanfisk commented May 11, 2022 via email

jsnajdr commented May 11, 2022

jsnajdr commented May 11, 2022

Dependency Extraction Webpack Plugin: Make the plugin work when using `optimizations.runtimeChunk = 'single'` #26214

Dependency Extraction Webpack Plugin: Make the plugin work when using `optimizations.runtimeChunk = 'single'` #26214

stefanfisk commented Oct 16, 2020 •

edited by gziolo

Loading

gziolo commented Nov 24, 2021 •

edited

Loading

noisysocks commented Nov 25, 2021 •

edited

Loading

gziolo commented May 9, 2022 •

edited

Loading