Add missing package dependencies

[anomiex]

What?

Closes #73257

Add missing dependencies on react.

Why?

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

How?

Add missing dependencies on react.

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.

[github-actions]

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: anomiex <bjorsch@git.wordpress.org>
Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org>
Co-authored-by: aduth <aduth@git.wordpress.org>

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

[anomiex]

It seems unlikely the failing test is due to this PR, but I don't have the ability to re-run it.

[manzoorwanijk]

I have this question #73257 (comment)

[aduth]

Given what you discovered at #73257 (comment) , I wonder if a better fix is to configure the JSX import source in the builds to point to @wordpress/element instead of react.

A problem I foresee with the approach as currently proposed is that it requires that someone define react as a dependency of their project to use one of these packages, when in reality @wordpress/element is what we expect to be used. This pierces that abstraction.

[manzoorwanijk]

I wonder if a better fix is to configure the JSX import source in the builds to point to @wordpress/element instead of react.

That doc mentions

The /jsx-runtime and /jsx-dev-runtime subpaths are hard-coded by design and cannot be changed.

Which means that we may need some post build modification to fix the issue for packages other than @wordpress/element

It means that we may need to set the export paths for those sub paths from the @wordpress/element package after adding those files from the react package and then update the JSX import source in our build config for those dependent packages which are built before distribution like admin-ui and dataviews

[aduth]

It means that we may need to set the export paths for those sub paths from the @wordpress/element package

Yes, that sounds to me like something we should do 👍 If @wordpress/element is meant to act as a drop-in for React and React does exactly that.

[manzoorwanijk]

#73391 is an attempt to deal with it.

[youknowriad]

👍

[aduth]

Just to close the loop on the previous discussion, I was under the mistaken impression that @wordpress/element is our targeted abstraction for working with React, and therefore should be the "pragma" import source. While this is historically true, I wasn't aware of the change in direction with #54074 to continue using @wordpress/element primarily for a few utility functions, and React proper for everything else. The confusion is compounded by the fact we continue to use @wordpress/element as an abstraction to React for most code 😅

[aduth]

How can we make these sorts of issues easier to discover? Maybe we should have some linting that requires a package to define React as a peer dependency if it defines @wordpress/element as a dependency?

[youknowriad]

Maybe we should have some linting that requires a package to define React as a peer dependency if it defines @wordpress/element as a dependency?

I think it's more it should be a peer dependency if there's JSX

[aduth]

I think it's more it should be a peer dependency if there's JSX

That sounds more correct and also more difficult to test for 😅 I suppose we could check for the existence .jsx and .tsx files, which is also an imperfect check until we enforce correct file extensions.

[github-actions]

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

• Required label: Any label starting with [Type].
• Labels found: [Type] Bug, [Type] Build Tooling.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.