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

wpcom-block-editor: bundle the react-jsx-runtime code #97319

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented Dec 11, 2024

This is an attempt to fix the following problem:

The wpcom-block-editor app builds several scripts like default.editor.min.js and then they are deployed to wpcom, available at https://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 like React.* or wp.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, like wp-data, which is not necessarily in sync with what the real dependencies of the widgets.wp.com script are. Both are in different codebases, released and deployed at different times to different places. If the script on widgets.wp.com is ever updated to require a new dependency, the Jetpack wp_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 like window.ReactJSXRuntime.jsx. And the dependency array that's written into default.editor.min.asset.php includes a new script dependency, react-jsx-runtime. But that asset.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:

If you're using @​wordpress/scripts for building JS scripts to target WordPress 6.5 or earlier, you should not upgrade to this version and continue using v5.

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 the react-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 like wp-data, and these scripts transitively depend on react-jsx-runtime. WP 6.6 declares that dependency in wp-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 Jetpack wp_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. The default.editor.js script will reference an undefined window.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 the react-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.

@jsnajdr jsnajdr self-assigned this Dec 11, 2024
@matticbot matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 11, 2024
@matticbot
Copy link
Contributor

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.

Copy link
Member

@jeherve jeherve left a 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! 🚢

@jeherve jeherve added [Pri] High Address as soon as possible after BLOCKER issues [Feature] Post/Page Editor The editor for editing posts and pages. [Status] Ready to Merge [Product] WordPress.com All features accessible on and related to WordPress.com. [Feature Group] Editor Experience Features related to Gutenberg integration on WordPress.com. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 11, 2024
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 11, 2024
Copy link
Member

@sirreal sirreal left a 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!

@jsnajdr jsnajdr merged commit 3a5b61c into trunk Dec 11, 2024
20 of 22 checks passed
@jsnajdr jsnajdr deleted the fix/wpcom-block-editor-jsx-runtime-bundle branch December 11, 2024 12:25
@github-actions github-actions bot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Ready to Merge labels Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature Group] Editor Experience Features related to Gutenberg integration on WordPress.com. [Feature] Post/Page Editor The editor for editing posts and pages. [Pri] High Address as soon as possible after BLOCKER issues [Product] WordPress.com All features accessible on and related to WordPress.com. [Type] Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants