There's plenty of e2e failures, let's see what causes them. I did a quick local check before submitting the PR and the editor loaded OK.

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: jsnajdr <[email protected]>
Co-authored-by: aduth <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: youknowriad <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

I did a quick local check before submitting the PR and the editor loaded OK.

Turns out I was looking at a different wp-env instance, the editor is currently completely broken with this PR. 🤦‍♂️ And the reason is an interesting esbuild tree shaking puzzle.

There is the admin-ui package which is bundled. It's not a standalone script or a script module, but is bundled both by scripts (editor) and script modules (routes/*). This package contains a BreadCrumb component which imports from @wordpress/route. And @wordpress/route is solely a script module, there's no way how a mere script can interpret and use it.

Now we have the editor script which bundles @wordpress/admin-ui. If the script tried to use BreadCrumbs, it would fail because of the @wordpress/route import that is invalid in scripts. Fortunately the script doesn't use the component and the BreadCrumbs code is eliminated by tree shaking.

But after this PR, that tree shaking doesn't work any more. Why? There are two ways how things can be eliminated by tree shaking:

• in the original trunk build, the code is in multiple files. There is admin-ui/build-module/index.js which imports and re-exports admin-ui/build-module/breadcrumbs/index.js. Here the bundler detects that the re-export is not used and eliminates an entire file. Including the offending @wordpress/route import.
• after this PR, the admin-ui package is bundled into one file, admin-ui/build-module/index.js. The BreadCrumbs code is part of that file. If the bundler wants to tree-shake BreadCrumbs, it has to erase the code the file. That's what esbuild does, with one exception: it keeps the import '@wordpress/route' statement, because it thinks it has side-effects. And this import statement breaks the editor script.

The @wordpress/route/package.json file has the sideEffect: false flag, but esbuild doesn't see that, because the import is externalized. The bundler never looks at the package.json. Only our onResolve callback does and this callback doesn't look at sideEffects.

I'll try to fix this by returning the correct value of sideEffects from the onResolve callbacks. It's part of the OnResolveResult type, so it should work.

Size Change: +49 kB (+1.9%)

Total Size: 2.62 MB

_{compressed-size-action}

The editor itself including e2e tests is now OK after the sideEffects patch. What is still failing is Storybook (npm run storybook:build):

Module build failed (from ./node_modules/babel-loader/lib/index.js):
RangeError: ./packages/components/build-module/index.js: Invalid string length
    at _append (./node_modules/@babel/generator/src/buffer.ts:270:25)
    at append (./node_modules/@babel/generator/src/buffer.ts:163:10)

The packages/components/build-module/index.js file is now a 3MB bundle of the entire package, and something in the Storybook pipeline has trouble working with such a big file.

@mirka do you have any clue what might cause this? Is there a part of Storybook that works directly with the source, e.g., doing syntax highlighting or something similar?

@mirka do you have any clue what might cause this? Is there a part of Storybook that works directly with the source, e.g., doing syntax highlighting or something similar?

The docgen maybe? I believe it's running source files through react-docgen-typescript to build the prop tables. (At least from my experience working with this docgen library directly, it starts parsing from the source files.)

The docgen maybe? I believe it's running source files through react-docgen-typescript to build the prop tables.

I managed to fix this error by excluding the build-module folders from babel-loader transpilation. A webpackFinal callback that finds the babel-loader rule (added by addon-webpack5-compiler-babel) and modifies its exclude field.

Seems that react-docgen-typescript works only with the type definitions in build-types and doesn't touch the large built files. After all, these files have types stripped, so the tool wouldn't find any useful info anyway.

Next possible steps to improve the Storybook build:

• eliminate the Babel addon completely. We do transpilation (JSX) with TypeScript, so maybe all we need is to configure TS better in the Storybook config.
• a much more ambitious step: eliminate also the webpack5 builder 🙂 And replace it with something custom based on our esbuild pipeline.

• a much more ambitious step: eliminate also the webpack5 builder 🙂 And replace it with something custom based on our esbuild pipeline.

Related: #72488

Flaky tests detected in f41e47d.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20026376102
📝 Reported issues:

• [Flaky Test] should open and edit templates correctly when active_templates experiment is enabled #73752 in /test/e2e/specs/site-editor/template-id-format.spec.js

For me this PR as it stands feels like it's doing too much (I'm willing to be convinced). It's fixing side effects which seems like a good change but also changing how we transpile packages to avoid just fixing the exported file name extensions. Isn't there a simpler way to solve this that is esbuild native (tell it to generate the right extensions and the right imports)

I think "bundling" during transpiration phase is a big change and I'm not sure we understand all what it means.

I think "bundling" during transpiration phase is a big change and I'm not sure we understand all what it means.

Yes, I agree that this is most likely an unsuccessful experiment. It completely reorganizes the package structure, with uncertain consequences for consumers.

The bundled index.js files can be big, up to 3MB. And the Storybook fix shows that Babel can have problems working with such files. We've see Node.js failures (RangeError when appending to a big string) and Babel warnings:

[BABEL] Note: The code generator has deoptimised the styling of {filename} as it exceeds the max of 500KB.

I'll now do the following:

• merge the side-effects and Storybook changes as separate PRs, as they are valuable and may improve build times and bundle sizes.
• explore configuring esbuild to output compliant relative import statements on its own.

Closing this after merging the valuable parts separately (#73780 and #73795) and after the much simpler alternative in #73822 looks much more promising.