-
Notifications
You must be signed in to change notification settings - Fork 2k
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
wpcom-block-editor: bundle the react-jsx-runtime code #97319
Conversation
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to reproduce the issue on a site running WordPress 6.5.5, Jetpack 12.5.1, a hardcoded time()
version for the WordPress.com block editor bundle, pointing to the old version of the bundle before yesterday's revert.
Updating the bundle to the one generated after this PR fixes the issue, and brings back the expected functionality. Nice work! 🚢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested, but I can confirm that the DEWP parts look correct for opting out of externalization of those packages.
Thanks for the detailed explanation and investigation, fascinating!
This is an attempt to fix the following problem:
The
wpcom-block-editor
app builds several scripts likedefault.editor.min.js
and then they are deployed to wpcom, available athttps://widgets.wp.com/wpcom-block-editor/default.editor.min.js
. They are built with@wordpress/dependency-extraction-webpack-plugin
, which means that WordPress module imports are resolved to globals likeReact.*
orwp.data.*
.These scripts are then loaded by individual sites that have Jetpack installed, with
wp_enqueue_script
calls like this one:https://github.com/Automattic/jetpack/blob/ceebd5264525bb78288ad3a311f3c43b0fbcbc7a/projects/packages/jetpack-mu-wpcom/src/features/wpcom-block-editor/class-jetpack-wpcom-block-editor.php#L331-L348
A notable thing about this is that this
wp_enqueue_script
call in Jetpack contains a hardcoded list of script dependencies, likewp-data
, which is not necessarily in sync with what the real dependencies of thewidgets.wp.com
script are. Both are in different codebases, released and deployed at different times to different places. If the script onwidgets.wp.com
is ever updated to require a new dependency, the Jetpackwp_enqueue_script
doesn't know and will be out of sync.Exactly this happened recently. In May, @youknowriad merged a Gutenberg PR (WordPress/gutenberg#61692) where the dependency extraction plugin newly externalizes the
react-jsx-runtime
package. Instead of that package being bundled in the build output, it's replaced with global references likewindow.ReactJSXRuntime.jsx
. And the dependency array that's written intodefault.editor.min.asset.php
includes a new script dependency,react-jsx-runtime
. But thatasset.php
file is not used, Jetpack has its own harcoded array.The new dependency extraction plugin was released with a major version bumb (5 to 6), with a warning about breaking changes in changelog:
Then, in November, @holdercp merged a Renovate PR in Calypso (#91492) that upgrades the dependency extraction plugin from v5 to v6.11, together with other webpack packages. You can find the changelog warning for 6.0.0 on the PR, buried quite deeply.
The PR also has a bot comment reminding that you shouldn't forget to deploy
wpcom-block-editor
to wpcom. But the deploy was not done.The deploy was done only five weeks later, when @nightnei and me did another, unrelated fix in
wpcom-block-editor
in #97202. This deploy also contained thereact-jsx-runtime
change.Now, if you run WordPress 6.6 (released in July this year), everything will work. The
default.editor.js
script has dependencies on script likewp-data
, and these scripts transitively depend onreact-jsx-runtime
. WP 6.6 declares that dependency inwp-includes/assets/script-loader-packages.php
. And the actual script is also there, in/wp-includes/js/dist/vendor/react-jsx-runtime.js
. Even though the Jetpackwp_enqueue_script
doesn't declare the dependency, it's still there and it's loaded.But what if you run on WordPress 6.5? This version doesn't know anything about
react-jsx-runtime
yet. Thedefault.editor.js
script will reference an undefinedwindow.ReactJSXRuntime
global and will fail.Note that it's not that relevant which version of Jetpack you are running. No released version declares the
react-jsx-runtime
dependency, and even if it did, WordPress 6.5 wouldn't have the script anyway. It would have it if you ran a recent version of the Gutenberg plugin, because the plugin overrides and upgrades these scripts.How do I fix it in this PR? I patch the
wpcom-block-editor/webpack.config.js
to stop externalizing thereact-jsx-runtime
dependency and to bundle it instead. Then it will work even on older WordPress versions.There are other scripts in the
apps
directory that should have the same change. Let's do that when we have an agreement that this is the fix we want to do.The alternative is to downgrade the dependency extraction plugin back to v5. But IMO using the latest version and configuring it appropriately is a better option.