Add React usage check to detect React 19 incompatibilities - #1380
Conversation
Adds a static check that scans a plugin's JavaScript files for a bundled, outdated React runtime that breaks once WordPress upgrades to React 19. The primary, high-confidence signal is `Symbol.for( 'react.element' )`, which is only emitted by an inlined pre-React 19 JSX runtime (React 19 uses the `react.transitional.element` marker). The warning is suppressed when the runtime is externalized, detected via a `window.ReactJSXRuntime` reference or a `react-jsx-runtime` dependency in the sibling `*.asset.php` file. Usage of React APIs removed in React 19 (unmountComponentAtNode, findDOMNode, ReactCurrentOwner) is reported as a secondary signal. Registers the check, adds PHPUnit tests with passing/failing fixtures, and documents it in docs/checks.md and the changelog. Fixes WordPress#1356
|
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. |
PHPMD flagged $matched as an undefined variable in look_for_removed_react_apis since it was only created via the by-reference argument. Initialize it first.
…runtime-check # Conflicts: # docs/checks.md # includes/Checker/Default_Check_Repository.php
|
Hello, I've got these findings from Codex. Could you check it? Findings [P2] Removed API regex reports comments/strings as usage |
Address the review feedback on the React 19 runtime check: - Stop treating a react-jsx-runtime dependency in the sibling .asset.php file as proof the runtime is externalized. The Symbol.for( 'react.element' ) marker means the file already inlines a pre-19 runtime, so a declared dependency can hide a stale or mixed build. Only an in-file window.ReactJSXRuntime reference now suppresses the warning. - Ignore the removed-API identifiers when they appear only in comments or string literals, so changelog notes and translation strings no longer produce false positives. Update the fixtures and tests to cover both cases.
|
Thanks @davidperezgar, both are fair points. I pushed a fix for each. P1 (asset dependency bypass): You're right that the P2 (comments and strings): Also right. The raw scan would flag Happy to adjust either if you'd prefer a different approach. |
…runtime-check # Conflicts: # docs/checks.md
|
Bumping this gently. The Inlined React Runtime check has been green for a few weeks now and still mergeable. If anything on trunk has moved under it I'm glad to rebase; otherwise it's ready for a review pass whenever the team has bandwidth. |
jsnajdr
left a comment
jsnajdr
left a comment
There was a problem hiding this comment.
I think this is ready to ship, it will be very useful for the React 19 rollout we are planning for Gutenberg in the WP 7.2 cycle.
|
Thanks @jsnajdr, and thanks again for strengthening the detection before this landed. Glad it lines up with the WP 7.2 React 19 rollout, that's exactly the kind of breakage this is meant to catch early. Happy to help if anything else comes up as you start testing plugins against it. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate findings affect detection coverage, suppression behavior, result severity, API coverage, and line reporting.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 5
Open (5)
What changed in this PR
Adds a React compatibility check for bundled React runtimes and React 19-incompatible APIs, with registration, documentation, changelog updates, and PHPUnit fixtures.
Changes:
- Implements React runtime and removed-API detection.
- Registers and documents the new check.
- Adds passing and failing fixtures with test coverage.
| File | Reviewed change |
|---|---|
tests/phpunit/tests/Checker/Checks/React_Usage_Check_Tests.php |
Tests React usage detection and suppression behavior. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-without-errors/view.js |
Passing React usage fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-without-errors/react-is.js |
Passing React detection fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-without-errors/react-19.js |
React 19 compatibility fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-without-errors/modern.js |
Modern React usage fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-without-errors/load.php |
Passing fixture plugin bootstrap. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-without-errors/comment-only.js |
Comment-only detection fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/react.js |
Bundled React incompatibility fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/react-external-dom.js |
React DOM usage fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/react-dom.js |
Removed React DOM API fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/react-17-prod.js |
React 17 runtime fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/load.php |
Failing fixture plugin bootstrap. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/legacy.js |
Legacy React API fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/jsx-runtime.js |
Inlined JSX runtime fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/jsx-runtime-tree-shaken.js |
Tree-shaken JSX runtime fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/jsx-runtime-dev.js |
Development JSX runtime fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/hydrate.js |
Hydration API fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/asset-declared.js |
Asset-declared runtime fixture. |
tests/phpunit/testdata/plugins/test-plugin-react-usage-with-errors/asset-declared.asset.php |
Asset dependency metadata fixture. |
readme.txt |
Adds the changelog entry. |
includes/Checker/Default_Check_Repository.php |
Registers the React usage check. |
includes/Checker/Checks/Performance/React_Usage_Check.php |
Implements React compatibility scanning. |
docs/checks.md |
Documents the check. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if ( $this->check_inlined_packages( $result, $file, $contents ) ) { | ||
| continue; | ||
| } |
| // with WordPress. A `*.asset.php` dependency is deliberately not | ||
| // accepted as proof: the element marker means a pre-19 build is | ||
| // already inlined, and a declared dependency does not rule out a | ||
| // stale or mixed build that still bundles its own copy. |
| ); | ||
| } | ||
|
|
||
| $this->add_result_error_for_file( |
| * Only the documented public surface is matched. Internals such as | ||
| * `ReactCurrentOwner` are deliberately left out: they never appear in plugin | ||
| * code, only inside a React build that the plugin inlined, which the inlined | ||
| * package errors cover. |
| } | ||
|
|
||
| $before = substr( $contents, 0, $offset ); | ||
| $exploded = explode( PHP_EOL, $before ); |
|
Reviewed by AI: Opus 5.5 SummaryThis PR adds a ✅ What's good
|
|
Hi @gunjanjaswal 👋 Could you please change the PR title to:
and edit the PR description so that it's more up to date after recent changes. I can be much shorter, it doesn't need to describe any implementation details. They change quickly, as the detection heuristic is improved. And this is no longer true:
Detected React 19 incompatibilities are marked as errors, as they really have the potential to break the site. I'll soon propose some code changes as another stacked PR, but I can't modify the PR GitHub info itself. |
gunjanjaswal
changed the title
|
Done — retitled to "Add React usage check to detect React 19 incompatibilities" and trimmed the description down. Dropped the implementation walkthrough (it moves too fast to keep in sync) and corrected the severity note: incompatibilities are reported as errors, with warnings only for the removed-API calls. Looking forward to your stacked changes. |
I'm addressing most of the feedback in the stacked gunjanjaswal#2 PR. Must fix
Suggestions
Other fixes
Not done
|
The check is about compatibility, not performance: all but one of its results are about code that stops working when WordPress upgrades React. Only the development-build note is about size and speed. Follows the directory convention of the other checks, where the directory and namespace mirror the primary category.
The replacement was substituted into the translated sentence through a placeholder, which works for the APIs whose replacement is a code identifier but not for the two that need prose: "a ref on the element" and "JSX or createElement()" reached the user in English whatever the locale. Give those two the complete sentence as their own translatable string and keep the shared one for the replacements that are code. The reported text is unchanged.
2.0.0 and 2.1.0 are both released, so @SInCE 2.0.0 claimed the check had been available for two releases that never contained it. The next release is 2.2.0. Drop the changelog entry as well. It was inserted into the released 2.0.0 section, and per docs/releasing.md the changelog is written when the release is cut, which is why the other checks added since 2.1.0 do not carry one.
Comments and string literals were blanked with a single regular expression, which failed in two ways. It could not tell a regex literal from a division, so the quote in `var re = /"/g;` opened what looked like a string and blanked the code after it. Any removed API called later in such a file went unreported. PCRE also exhausted its recursion limit on the long string literals of a bundled file. preg_replace_callback then returned null and the fallback handed back the raw contents, so nothing was blanked at all and a mention in a comment was reported as a call, undoing what blanking is for. Walk the contents token by token instead. A slash opens a regular expression where no value precedes it and divides otherwise, which also covers `return/^a$/` and a slash inside a character class. There is no backtracking, so file size no longer matters: a 3 MB literal that used to defeat PCRE now scans in well under a second.
The fingerprints for `react/jsx-runtime` and `react` matched too readily once the file mentioned the element symbol for an unrelated reason, which `react-is` does and which reaches a great many bundles through `prop-types`. `jsxs?\s*[:=]` then matched a syntax highlighter defining `Prism.languages.jsx`, or a parser option object holding `jsx: true`, and reported an error against a plugin with no React problem at all. The same applies to `preact/compat`, which names the element symbol and exports React's internals sentinel while creating Preact vnodes. It never touches the React WordPress ships, yet it was reported as inlining react. Require `_owner`, a field React 19 dropped from the element object, so a package is only implicated when the file also builds pre-19 elements. The renderer is exempt: it consumes elements rather than creating them, so a file holding only a copy of `react-dom` has no factory, and `__reactFiber$` is specific enough by itself. That exemption also stops such a file from being reported as inlining react as well as react-dom. Verified against react, react-dom and react/jsx-runtime for 16, 17 and 18 in both development and production, and against the bundles of astra-sites and wp-table-builder, all of which are still detected.
Positions were counted by splitting on PHP_EOL, the line ending native to the machine running the check. A plugin's line endings have nothing to do with that, so a file written with line feeds collapsed onto line 1 when checked on Windows, and a file written with carriage returns alone collapsed onto line 1 everywhere. Split on all three line endings instead, counting a carriage return followed by a line feed once. The regression fixture uses carriage returns alone, the case that was wrong on every platform and the one line ending git will not rewrite on checkout.
The guard accepted any mention of window.React as proof that the package was externalized, reads and writes alike. A build that publishes itself under the global therefore silenced the very error it should raise: it can only publish a copy it carries, and it replaces the copy WordPress loaded for every script that runs after it. Take a read of the global as proof of externalizing, and let an assignment anywhere in the file override every read in it. The packages now carry the name of their global rather than a pattern, since two patterns are built from it. fiber-inspector.js restores coverage of the read: it is now the only file among those expected to stay silent whose silence depends on the guard.
The marker that establishes a pre-19 build is the name of the element symbol, which was matched only between quotes. Some minifiers rewrite every string in a bundle as a template literal, leaving a build with no quoted string in it at all, and such a file passed as having no React inlined. Accept the backtick alongside the two quotes. Only this marker read a delimiter; the rest match bare substrings.
The bundles WordPress ships patch React 19 into accepting the pre-19 element shape and warning about it, so whether an inlined copy fails outright depends on where its elements end up. Hedge the claim to match, and record in the class docblock why the severity is still an error: the patch is there to carry plugins through the upgrade, not to make the older shape supported. The removed-API warnings keep saying such a call stops working, because those APIs are gone from React 19 with nothing standing in for them.
The comment justified the skip for an inlined renderer, where a removed API found in the file is React's own code, but not for the other two packages, where the call may well be the plugin's own. Record the rest of the reasoning: a deprecated call is a small thing to raise beside a bundled copy of React, and it is not lost, because the marker goes when the inlined copy does and the next run reports it.
The check graded a development build above a production one, but nothing recorded why one should outrank the other, and the numbers were close enough to a submission threshold to matter. Report both at the default severity and let the message carry the difference.
A development build was detected by the documentation links React embeds in its warnings, and reported with its own wording. Inlining the package is the problem either way, so the second message and the sniffing behind it bought a branch through the reporting path and little else.
|
Merged your stacked #2, so everything above is now in this PR — the |
|
Thanks @gunjanjaswal, there is one more little fix that addresses the "PHP Code Linting" CI failure: #3. Fixes a too high complexity of the |
|
Can you please check if following issues are valid?
|
|
Both are valid, thanks for the careful testing — and sorry the line-453 point sat from last round without a reply. On the removed-API patterns: you're right that requiring the name immediately before On line 453: also agreed. With WordPress/gutenberg#78899 polyfilling @jsnajdr has been authoring the detection through the stacked PRs and has more queued — these two fit naturally there, so we'll fold both into the next one. I'll make sure they land. |
This one is real, quite a glaring omission 😨. Fixing in gunjanjaswal#4. It was missed because the corpus of some 10+ plugins that I'm locally testing on always contained a "normal" call to
I don't think this is valid. Yes, in a literal sense the sentence is false, because in Gutenberg we patch |
|
OK, we have all the required approvals, CI is green... Anything else needed to merge? |
|
Thanks @ernilambar for merging, and @jsnajdr for all the detection work along the way — the per-package |
|
Now that this has landed, I'd like to keep helping in this area. @jsnajdr you mentioned a make.wordpress.org post and that the detection heuristic will keep evolving — happy to take on follow-ups there (the polyfilled-API wording, more bundler shapes, fixtures from real plugins), or anything else on the plugin-check side you or @ernilambar would value a hand with. Just point me at it. |
Closes #1356.
Adds a
react_usagecheck that flags plugin builds likely to break when WordPress upgrades to React 19.It reports errors when a plugin inlines a pre-19 React package (
react/jsx-runtime,react, orreact-dom) into its bundle instead of loading the copy WordPress ships, since element objects from a pre-19 build are rejected by React 19. It reports warnings for calls to React APIs that React 19 removed, which still work today but stop working after the upgrade.The detection heuristic is being refined as it goes, so this description stays high-level on purpose.
AI Usage Disclosure
AI assistance (Claude Code) was used to help draft the check and its tests. I reviewed and understand every line and take responsibility for it.