Add missing package dependencies

[anomiex]

What?

Add missing peer dependencies of @wordpress/icons, @wordpress/interactivity, and @wordpress/react-i18n on react.

Add missing dependency of @wordpress/interactivity on @babel/runtime.

Why?

Build code transformations can introduce dependencies on packages such as react and @babel/runtime. These need to be declared if the package is to function correctly with yarn's p'n'p or pnpm with hoisting disabled.

Fixes #55171

How?

Adding the missing dependencies. Normally I'd probably have done the @babel/runtime dep as a peer dep, but I see you use a normal dependency everywhere else so I followed along.

Testing Instructions

1. Follow the reproduction instructions in the linked bug to set up the test and reproduce the bug.
2. Do yarn add <package>@file:/path/to/gutenberg/packages/<package> or pnpm add <package>@file:/path/to/gutenberg/packages/<package> to point yarn or pnpm at the locally built version of the package.
3. Repeat the final instruction in the reproduction to check that the bug is fixed.

[anomiex]

Pinging @youknowriad, @jsnajdr, and @tyxla as people responsible for #54494, and @gziolo since you merged the similar #54118 for me recently.

[gziolo]

It looks good for @wordpress/icons and @wordpress/react-i18n, but there is probably some more work necessary for @wordpress/interactivity that uses preact. @luisherranz and @DAreRodz should know best how to improve the configuration.

[luisherranz]

but there is probably some more work necessary for @wordpress/interactivity that uses preact. @luisherranz and @DAreRodz should know best how to improve the configuration.

Indeed, @wordpress/interactivity uses preact, not react, which is already listed as a dependency.

Why does it need @babel/runtime listed as a peer dependency? No polyfills/runtimes should be added to this package.

[gziolo]

Why does it need @babel/runtime listed as a peer dependency? No polyfills/runtimes should be added to this package.

See the file defined as the main entry point in package.json:

https://unpkg.com/@wordpress/interactivity@2.4.0/build/index.js

There is the following line:

var _interopRequireDefault = require("@babel/runtime/helpers/interopRequireDefault");

[gziolo]

This is also an interesting example:

https://unpkg.com/browse/@wordpress/interactivity@2.4.0/build-module/hooks.js

The first line is:

import { createElement } from "react";

[jsnajdr]

Why does it need @babel/runtime listed as a peer dependency?

This is typical for files transpiled by Babel. Many transforms use a helper function, and either a copy of that helper function is pasted into each transpiled file, leading to hundreds of copies, or it's imported from a runtime package. There is a @babel/plugin-transform-runtime plugin for that and it has a helpers: boolean option specifying whether to use imports or inline.

import { createElement } from "react";

That's certainly a bug, probably a regression introduced by #54494. This import is auto-inserted by the JSX transform. For interactivity files, the transform should be configured to import from preact.

[luisherranz]

For interactivity files, the transform should be configured to import from preact.

That's what we are doing, so it looks like a bug. I'll take a look.

https://github.com/WordPress/gutenberg/blob/trunk/tools/webpack/interactivity.js#L45-L46

See the file defined as the main entry point in package.json:

Ok. It looks like we could avoid default exports then, although we will move to modules in the near future, so those webpack runtime patches won't be necessary:

https://unpkg.com/browse/@wordpress/interactivity@2.4.0/build-module/index.js

[youknowriad]

That's what we are doing, so it looks like a bug. I'll take a look.

https://github.com/WordPress/gutenberg/blob/trunk/tools/webpack/interactivity.js#L45-L46

I think this should override @wordpress/babel-plugin-import-jsx-pragma in addition to '@babel/preset-react'

[luisherranz]

Ok. This problem was not introduced in #54494 because previous to that, the build and build-module folders already included a import { createElement } from "@wordpress/element"; line.

This is what is happening: Gutenberg is doing two transpilation passes. The first one generates the build and build-module folders. The second one generates the final bundles.

We're ignoring the build folders for the @wordpress/interactivity package because those include the React JSX transform (previously @wordpress/element) and @wordpress/interactivity doesn't need that first transpilation.

So we are generating the bundle using the src folder directly:

module.exports = {
	...baseConfig,
	entry: {
		index: {
			import: `./packages/interactivity/src/index.js`,

That also explains why there's not @babel/runtime either in the final bundle, because there are no default imports when everything is bundled in a single pass.

When I was integrating the private version of the @wordpress/interactivity package with WordPress Core, I added an alias to point to the src folder manually to fix this problem:

resolve: {
  alias: {
    '@wordpress/interactivity': normalizeJoin( baseDir, 'node_modules/@wordpress/interactivity/src/index.js' ),
  },
},

As I'm currently working on migrating this package to ES modules, I don't think it makes sense to improve how things work today in this regard. So feel free to add the react and @babel/runtime to the package.json if that fixes things.

I'll make sure to refactor all this once this package is using only ES modules.

[youknowriad]

We're ignoring the build folders for the @wordpress/interactivity package because those include the React JSX transform (previously @wordpress/element) and @wordpress/interactivity doesn't need that first transpilation.

That doesn't feel ideal to me. I think core and Gutenberg should consume the package in the exact same way any third party application does.

[luisherranz]

I agree, but I don't think that it makes sense to do this work now because I have started working on the ES module migration so the bundling configuration will change. I'll make sure to fix this before the package is publicly available.

[anomiex]

Ok. This problem was not introduced in #54494 because previous to that, the build and build-module folders already included a import { createElement } from "@wordpress/element"; line.

Looks like in that case #54494 changed the problem from "missing @wordpress/element dep" to "missing react dep".

Anyway, if you all want to update this PR once you've figured out what exactly needs updating for @wordpress/interactivity, feel free to go ahead and do it instead of waiting for me. 👍

So far it's only @wordpress/icons that's causing a problem for my repo, the other two I found after checking for any other packages affected by the same change.