Update the default JSX pragma to React instead of @wordpress/element #54494
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
added
the
[Type] Build Tooling
youknowriad
requested review from
gziolo,
ntwb,
nerrad and
ajitbohra
as code owners
|
I did a pass on the documentation change on this branch just to have an idea. |
youknowriad
requested review from
mcsf,
Mamaduka,
mtias,
jsnajdr,
dmsnell,
noahtallen,
tyxla,
jorgefilipecosta,
ntsekouras and
ellatrix
youknowriad
commented
Sep 15, 2023
| "@wordpress/warning": "file:../warning", | ||
| "browserslist": "^4.21.9", | ||
| "core-js": "^3.31.0" | ||
| "core-js": "^3.31.0", | ||
| "react": "^18.2.0" |
There was a problem hiding this comment.
I'm not entirely sure that we need this dependency here but I just mirrored the previous @wordpress/element dependency.
There was a problem hiding this comment.
This dependency was added by @gziolo in #17027 and I don't understand the rationale very well. The preset doesn't use element or react. The problem was something like when a project (e.g., a block) depends on wp-scripts, it should install everything wp-scripts needs automatically, instead of the project's package.json having to specify additional dependencies explicitly.
There was a problem hiding this comment.
I don’t remember the exact reasoning behind it. Maybe there is still value in keeping it but as a peer dependency since it’s React now. Although it doesn’t seem to be necessary anymore for plugins when using wp-scripts build because it converts imports to react or wp.element. It might be different with unit tests though.
youknowriad
force-pushed
the
update/default-jsx-pragma
branch
from
1dee6c4
to
6b20c9f
Compare
|
Size Change: +416 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
youknowriad
force-pushed
the
update/default-jsx-pragma
branch
from
6b20c9f
to
8e851ac
Compare
|
I would love some feedback here. Personally, I feel good about this change and I'm thinking maybe we should land it after 6.4 beta1 that way it doesn't impact the next release but will be ready soon enough for 6.5. |
dmsnell
reviewed
Sep 18, 2023
There was a problem hiding this comment.
I've looked at this several times and frankly just don't feel qualified to understand the impact of the change. It seems conceptually like a good move, but I can't think of anything that would break from it. If you want it, I'll give it a 👍, but I would prefer that you lean on review from others on this one.
Thanks for pushing this through.
jsnajdr
approved these changes
Sep 19, 2023
tyxla
approved these changes
Sep 19, 2023
There was a problem hiding this comment.
Great to see, thanks @youknowriad.
Just one minor doc change, LGTM otherwise 🚀
|
Thanks for the reviews folks. It looks like beta1 for WordPress 6.4 is for next week. I'm going to hold off merging this one until that point to avoid impacting that release for the moment. It will give us time to use this in the plugin phase before 6.5. |
youknowriad
force-pushed
the
update/default-jsx-pragma
branch
from
b15f798
to
046c99a
Compare
youknowriad
added
the
Needs Dev Note
|
Flaky tests detected in 046c99a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6336744326
|
|
Ok, let's give this a try 😬 |
|
@youknowriad, when do you want to publish the dev note? If I follow correctly, the next step would be to stop using this Babel plugin in gutenberg/packages/babel-preset-default/index.js Lines 78 to 94 in 429e645 Regardless, the changes from this PR will take effect next Wednesday after npm publishing, but they won't get reflected in WordPress core until 6.5 cycle. |
|
@gziolo Good question, we don't really have a way to publish "posts" about npm changes. I was planning the dev note for 6.5 to avoid messing up with the 6.4 dev notes schedule but it's true that we need to surface this somehow to developers. Maybe we should include a dedicated paragraph in the next Gutenberg release post? |
That would be a good start. There is also https://developer.wordpress.org/news/, and that could be highlighted in the monthly summary post. |
|
cc @bph for help highlighting this change. |
anomiex
added a commit
to anomiex/gutenberg
that referenced
this pull request
Oct 9, 2023
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. After WordPress#54494, `@wordpress/icons`, `@wordpress/interactivity`, and `@wordpress/react-i18n` are now lacking peer dependencies on react. `@wordpress/interactivity` also appears to be lacking a dependency or peer dependency on `@babel/runtime`. Fixes WordPress#55171
lipemat
added a commit
to lipemat/types-js-boilerplate
that referenced
this pull request
Nov 10, 2023
32 tasks
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.
See #54074 for the reasoning/discussions
This PR implements the change proposed in #54074 to switch the default JSX pragma from @wordpress/element to React. In addition to the current changes, we may want to update our documentation and the files where we currently import
@wordpress/elementand might not need to. I decided to not update these in the same PR to ensure/clarify that with this change, everything continues to work exactly the same without any change to the code base. Also updating documentation and code will quickly result in conflicts unless we merge those PRs quickly, so I'm leaving them separate until the decision is made to merge the current PR.