Interactivity: Fix broken react usage in published package #58258
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
|
Size Change: -66 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
sirreal
force-pushed
the
interactivity-build-process
branch
from
1f488c5
to
7d6bb36
Compare
sirreal
added
[Type] Bug
This was referenced Jan 25, 2024
youknowriad
added
the
Backport to Gutenberg RC
sirreal
force-pushed
the
interactivity-build-process
branch
from
7d6bb36
to
9e136ff
Compare
|
Flaky tests detected in 9e136ff. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7695209496
|
cbravobernal
pushed a commit
that referenced
this pull request
Jan 30, 2024
@wordpress/interactivity depends on preact, not react. - Enable the "no react in scope" eslint rule for the interactivity packages. - Add pragmas to set the JSX transform the use createElement. - Import Preact's `h` as `createElement` explicitly. This setup ensures that `@wordpress/babel-plugin-import-jsx-pragma` does not add a `createElement` import from React because it detects createElement is already in scope.
|
I just cherry-picked this PR to the release/17.6 branch to get it included in the next release: |
cbravobernal
removed
the
Backport to Gutenberg RC
youknowriad
pushed a commit
that referenced
this pull request
Jan 31, 2024
@wordpress/interactivity depends on preact, not react. - Enable the "no react in scope" eslint rule for the interactivity packages. - Add pragmas to set the JSX transform the use createElement. - Import Preact's `h` as `createElement` explicitly. This setup ensures that `@wordpress/babel-plugin-import-jsx-pragma` does not add a `createElement` import from React because it detects createElement is already in scope.
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.
What?
Fix broken
@wordpress/interactivitypublished package.Why?
@wordpress/interactivitydepends on preact, not react.Packages are built two ways:
When this package is built for the plugin, it uses a different webpack config that correctly handles the jsx transform and the preact jsx auto imports:
gutenberg/tools/webpack/interactivity.js
Lines 58 to 67 in 388a1e9
However, when built for NPM it uses the standard configuration which imports
createElementfromreactresulting in a package that doesn't work as expected. You can see the unexpected react import here.How?
I consider this a short-term fix:
createElement.hascreateElementexplicitly.This setup ensures that
@wordpress/babel-plugin-import-jsx-pragmadoes not add acreateElementimport fromreact(because it detectscreateElementis already in scope.It is fragile because if we don't alias the preact
himport tocreateElementthen@wordpress/babel-plugin-import-jsx-pragmawould add a react import.Testing Instructions
I've tested this 2 ways
In WordPress, locally build and run WordPress and confirm that interactivity is still working. I tested with the default themes navigation block query loop by disabling its "force page reload" setting.
As an npm package. I published a version of this for testing (confirm no react here) and made a small app that I compiled with
webpack-cli(default config) and served locally. Gist of the source is here.Next
This is a short term fix so the package is not broken in the short term.
It would be nice to see if we can remove and deprecate some of our custom babel transforms and use babel's react preset with the automatic JSX runtime. That's a bigger and more complicated change.
Concerns
This is fragile and not a long term solution. It relies on adding and aliasing an import and there are limited safeguards to ensure it's done correctly.
Fortunately, I don't believe the other things using the Interactivity API (the view files listed here) can use Preact directly, which mitigates some of the problems.