React: Upgrade to the new JSX transform #61692
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
force-pushed
the
try/new-jsx-2
branch
from
f6c8ec4 to
1b8dc96
Compare
|
Size Change: -1.13 kB (-0.06%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
|
It's interesting to see that this PR actually works as is but it's probably bundling "react/jsx-runtime" in all of our JS scripts that use jsx. |
youknowriad
force-pushed
the
try/new-jsx-2
branch
from
1307b7f to
705559a
Compare
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/client-assets.php |
youknowriad
force-pushed
the
try/new-jsx-2
branch
from
705559a to
2713e68
Compare
youknowriad
commented
May 16, 2024
Mamaduka
reviewed
May 16, 2024
youknowriad
commented
May 16, 2024
youknowriad
commented
May 16, 2024
tools/webpack/development.js
Outdated
| @@ -46,4 +46,33 @@ module.exports = [ | |||
| } ), | |||
| ], | |||
| }, | |||
| { | |||
| ...sharedConfig, | |||
| name: 'react-jsx-runtime', | |||
There was a problem hiding this comment.
I'm going to need someone to check that the built script for this is correct. I'm trying to extract it to its own script and using "React" as an external global variable. I think it's ok now but a confirmation would be great.
youknowriad
commented
May 16, 2024
| @@ -75,21 +75,10 @@ module.exports = ( api ) => { | |||
| ], | |||
| plugins: [ | |||
| require.resolve( '@wordpress/warning/babel-plugin' ), | |||
| [ | |||
| require.resolve( '@wordpress/babel-plugin-import-jsx-pragma' ), | |||
There was a problem hiding this comment.
We need to deprecate this and ensure a breaking change release for babel-preset-default and scripts packages.
|
Flaky tests detected in eed6746. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9108633575
|
youknowriad
added
[Type] Build Tooling
youknowriad
requested review from
spacedmonkey,
ajitbohra,
gziolo,
ntwb,
nerrad,
luisherranz and
DAreRodz
as code owners
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
youknowriad
force-pushed
the
try/new-jsx-2
branch
2 times, most recently
from
5c8d1ec to
d7d8cd5
Compare
We can still align as closely as possible with core in the production build, I don't see much value in dev mode though. As you pointed out in the testing instructions, e2e tests should basically cover everything. Now that I recall from the original PR, I think this ties to the way we currently do for our dev builds too. For instance, we don't really need to run a two-step build (babel -> webpack) for development. If we use only webpack in dev mode then it'll be a lot faster and enable advanced features like Fast Refresh. The production build can still be the same. This workflow could also make the transition to vite or other modern build tools easier too. It's really a bigger discussion that we had multiple times in different places though 😅. I understand that this is not the focus of this PR, but just want to state the possibility that we can still support |
|
Is it possible to ship a different source for |
|
@Mamaduka That won't work because for both modes (dev and prod), the packages ship with |
|
So I think this PR is ready, should we move forward with it? What's left? |
|
@youknowriad, are aiming to ship this with WP 6.6? |
|
@Mamaduka That's my hope yes, that way third-party developers could start fixing the warnings... sooner. |
Mamaduka
approved these changes
May 22, 2024
7 tasks
49 tasks
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
May 31, 2024
See WordPress/gutenberg#61692. See #6678. Props youknowriad. Fixes #61324. git-svn-id: https://develop.svn.wordpress.org/trunk@58271 602fd350-edb4-49c9-b593-d223f7449a82
|
I am concerned that this PR might have caused issues when using the scripts package in custom block development. Please see this comment. |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jun 4, 2024
Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org>
4 tasks
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org>
|
@youknowriad Heads up, as @t-hamano warns, we're seeing breakage too with |
|
Please refer to the following issue for the issue you are concerned about. The following dev notes may also be helpful. https://make.wordpress.org/core/2024/06/06/jsx-in-wordpress-6-6/ |
This was referenced Aug 21, 2024
Bump @wordpress/scripts from 27.9.0 to 28.5.0 in the wordpress group
Automattic/remote-data-blocks#1
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Alternative to #34221
What
React 17 introduced a new JSX runtime but we are still using the old JSX build process. As part of the React 19 upgrade, the React team is deprecating the old JSX so this is the second attempt to move away from it.
The initial PR #34221 tried to include the runtime into the "@wordpress/element" package but since, we've moved away from using @wordpress/element as the JSX target of our builds, which means we can take a simpler approach and just "externalize" the react/jsx-runtime script into a new WP script handle and global variable and use it while building our scripts.
We should be able to deprecate our package
@wordpress/babel-plugin-import-jsx-pragma.To learn more about the new JSX transform, read this post.
This definitely requires a dev note to nudge plugin authors to also switch to the upgraded "@wordpress/scripts".
Todo:
Anything I'm missing?
Testing instructions.
The e2e tests got us covered here in general.