[DevTools] Ignore new production renderers if we already use "worse" versions of React on a page#679
Closed
everettbu wants to merge 2 commits into
Conversation
…versions of React on a page
everettbu
added
CLA Signed
React Core Team
everettbu
marked this pull request as ready for review
March 10, 2026 19:27
Greptile SummaryThis PR fixes a bug where React DevTools would display the wrong extension icon/popup when a page uses multiple React renderers — specifically, a later production renderer could overwrite a previously-detected non-production (development/unminified/etc.) build, hiding the fact that a dev build was in use. The fix introduces a
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant React as React Renderer
participant Hook as __REACT_DEVTOOLS_GLOBAL_HOOK__
participant Listener as createReactRendererListener<br/>(installHook.js)
participant Reduce as reduceReactBuild
participant Window as window.postMessage
participant BG as Background Script
participant Icon as setExtensionIconAndPopup
React->>Hook: renderer registered (reactBuildType)
Hook->>Listener: emit('renderer', {reactBuildType})
Listener->>Reduce: reduceReactBuild(displayedReactBuild, reactBuildType)
Note over Reduce: If current is null or 'production' → return next<br/>If current is non-production and next is 'production' → keep current<br/>Otherwise → return next (last non-prod wins)
Reduce-->>Listener: updated displayedReactBuild
Listener->>Window: postMessage({type:'react-renderer-attached', reactBuildType: displayedReactBuild})
Window->>BG: message event
BG->>Icon: setExtensionIconAndPopup(reactBuildType, tabId)
Icon->>Icon: chrome.action.setIcon(...)
Icon->>Icon: chrome.action.setPopup(...)
Last reviewed commit: 209a41c |
![greptile-apps-staging[bot]](https://avatars.githubusercontent.com/in/919028?s=60&v=4){kind=small}
Comment on lines
+22
to
+27
| // We only display the "worst" build type, so if we've already detected a non-production build, | ||
| // we ignore any future production builds. This way if a page has multiple renderers, | ||
| // and at least one of them is a non-production build, we'll display that instead of "production". | ||
| return nextReactBuildType === 'production' | ||
| ? currentReactBuildType | ||
| : nextReactBuildType; |
There was a problem hiding this comment.
Severity ordering not preserved among non-production builds
The function returns nextReactBuildType whenever both the current and next types are non-production, meaning a later renderer can downgrade the displayed type from a more severe to a less severe one.
Consider this sequence:
- Renderer 1:
deadcode→displayedReactBuild = 'deadcode' - Renderer 2:
development→ since current is'deadcode'(not null, not'production') and next is not'production', the function returns'development'
The deadcode type (bad DCE in a production build — arguably the worst state) is silently replaced by 'development'. A developer would then see the less alarming development icon and miss that dead code elimination had failed.
A safer approach is to encode a severity ordering and only ever move toward a "worse" build type:
const BUILD_TYPE_SEVERITY: {[ReactBuildType]: number} = {
production: 0,
outdated: 1,
development: 2,
unminified: 3,
deadcode: 4,
};
function reduceReactBuild(
currentReactBuildType: null | ReactBuildType,
nextReactBuildType: ReactBuildType,
): ReactBuildType {
if (currentReactBuildType === null) {
return nextReactBuildType;
}
return BUILD_TYPE_SEVERITY[nextReactBuildType] >
BUILD_TYPE_SEVERITY[currentReactBuildType]
? nextReactBuildType
: currentReactBuildType;
}This guarantees the displayed build type never regresses to a less severe state.
Comment on lines
+36
to
+48
| return function ({reactBuildType}) { | ||
| displayedReactBuild = reduceReactBuild(displayedReactBuild, reactBuildType); | ||
|
|
||
| target.postMessage( | ||
| { | ||
| source: 'react-devtools-hook', | ||
| payload: { | ||
| type: 'react-renderer-attached', | ||
| reactBuildType: displayedReactBuild, | ||
| }, | ||
| }, | ||
| '*', | ||
| ); |
There was a problem hiding this comment.
Unnecessary postMessage when displayed build type hasn't changed
When a production renderer fires after a non-production build type has already been recorded, displayedReactBuild is unchanged (e.g., stays 'development'), but postMessage is still called. This triggers an unnecessary round-trip to the background script and redundant chrome.action.setIcon/chrome.action.setPopup calls.
Consider guarding the message with a check:
return function ({reactBuildType}) {
const prevDisplayedReactBuild = displayedReactBuild;
displayedReactBuild = reduceReactBuild(displayedReactBuild, reactBuildType);
if (displayedReactBuild === prevDisplayedReactBuild) {
return;
}
target.postMessage(
{
source: 'react-devtools-hook',
payload: {
type: 'react-renderer-attached',
reactBuildType: displayedReactBuild,
},
},
'*',
);
};This is not critical since the background script is idempotent, but it avoids unnecessary work in multi-renderer setups.
Greptile SummaryThis PR fixes a bug where the React DevTools extension could display the production build icon/popup even when a development (or otherwise non-production) React renderer was present on the page — specifically triggered by Next.js pages that load both a production server renderer and a development client renderer. The fix introduces a stateful listener factory ( Key observations:
Confidence Score: 4/5
Important Files Changed
Last reviewed commit: 209a41c |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+22
to
+27
| // We only display the "worst" build type, so if we've already detected a non-production build, | ||
| // we ignore any future production builds. This way if a page has multiple renderers, | ||
| // and at least one of them is a non-production build, we'll display that instead of "production". | ||
| return nextReactBuildType === 'production' | ||
| ? currentReactBuildType | ||
| : nextReactBuildType; |
There was a problem hiding this comment.
Comment inconsistent with actual ordering logic
The comment says "We only display the 'worst' build type" which implies a total severity ordering, but among non-production builds the function simply uses the latest non-production type seen — no priority is enforced. For example, if a 'deadcode' renderer is detected first and a 'development' renderer is detected second, the displayed type silently changes from 'deadcode' to 'development', even if 'deadcode' could be considered more severe.
This matches pre-existing behavior (last-renderer-wins for same-category types) and the PR explicitly scopes "worse" to mean any non-production build vs. production, so this isn't a regression. But the comment should be made more precise to avoid misleading future readers:
Suggested change
| // We only display the "worst" build type, so if we've already detected a non-production build, | |
| // we ignore any future production builds. This way if a page has multiple renderers, | |
| // and at least one of them is a non-production build, we'll display that instead of "production". | |
| return nextReactBuildType === 'production' | |
| ? currentReactBuildType | |
| : nextReactBuildType; | |
| // We only prevent production builds from hiding previously detected non-production builds. | |
| // Among non-production build types (development, deadcode, unminified, outdated), we use | |
| // the latest one seen, since all of them are equally "worse" than production. | |
| return nextReactBuildType === 'production' | |
| ? currentReactBuildType | |
| : nextReactBuildType; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-extensions/src/contentScripts/reactBuildType.js
Line: 22-27
Comment:
**Comment inconsistent with actual ordering logic**
The comment says *"We only display the 'worst' build type"* which implies a total severity ordering, but among non-production builds the function simply uses the **latest** non-production type seen — no priority is enforced. For example, if a `'deadcode'` renderer is detected first and a `'development'` renderer is detected second, the displayed type silently changes from `'deadcode'` to `'development'`, even if `'deadcode'` could be considered more severe.
This matches pre-existing behavior (last-renderer-wins for same-category types) and the PR explicitly scopes "worse" to mean *any non-production build vs. production*, so this isn't a regression. But the comment should be made more precise to avoid misleading future readers:
```suggestion
// We only prevent production builds from hiding previously detected non-production builds.
// Among non-production build types (development, deadcode, unminified, outdated), we use
// the latest one seen, since all of them are equally "worse" than production.
return nextReactBuildType === 'production'
? currentReactBuildType
: nextReactBuildType;
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+36
to
+48
| return function ({reactBuildType}) { | ||
| displayedReactBuild = reduceReactBuild(displayedReactBuild, reactBuildType); | ||
|
|
||
| target.postMessage( | ||
| { | ||
| source: 'react-devtools-hook', | ||
| payload: { | ||
| type: 'react-renderer-attached', | ||
| reactBuildType: displayedReactBuild, | ||
| }, | ||
| }, | ||
| '*', | ||
| ); |
There was a problem hiding this comment.
postMessage fires even when displayed build type is unchanged
target.postMessage is called on every renderer event regardless of whether displayedReactBuild actually changed. For example, if the page already shows 'development' and a second 'production' renderer fires, reduceReactBuild correctly keeps 'development', but a redundant message is still sent to the background page, which will re-apply the same icon and popup.
While harmless, this can be avoided with a simple guard:
Suggested change
| return function ({reactBuildType}) { | |
| displayedReactBuild = reduceReactBuild(displayedReactBuild, reactBuildType); | |
| target.postMessage( | |
| { | |
| source: 'react-devtools-hook', | |
| payload: { | |
| type: 'react-renderer-attached', | |
| reactBuildType: displayedReactBuild, | |
| }, | |
| }, | |
| '*', | |
| ); | |
| return function ({reactBuildType}) { | |
| const nextBuild = reduceReactBuild(displayedReactBuild, reactBuildType); | |
| if (nextBuild === displayedReactBuild) { | |
| return; | |
| } | |
| displayedReactBuild = nextBuild; | |
| target.postMessage( | |
| { | |
| source: 'react-devtools-hook', | |
| payload: { | |
| type: 'react-renderer-attached', | |
| reactBuildType: displayedReactBuild, | |
| }, | |
| }, | |
| '*', | |
| ); | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-extensions/src/contentScripts/reactBuildType.js
Line: 36-48
Comment:
**`postMessage` fires even when displayed build type is unchanged**
`target.postMessage` is called on every `renderer` event regardless of whether `displayedReactBuild` actually changed. For example, if the page already shows `'development'` and a second `'production'` renderer fires, `reduceReactBuild` correctly keeps `'development'`, but a redundant message is still sent to the background page, which will re-apply the same icon and popup.
While harmless, this can be avoided with a simple guard:
```suggestion
return function ({reactBuildType}) {
const nextBuild = reduceReactBuild(displayedReactBuild, reactBuildType);
if (nextBuild === displayedReactBuild) {
return;
}
displayedReactBuild = nextBuild;
target.postMessage(
{
source: 'react-devtools-hook',
payload: {
type: 'react-renderer-attached',
reactBuildType: displayedReactBuild,
},
},
'*',
);
};
```
How can I resolve this? If you propose a fix, please make it concise.
🦖 T-RexGenerated 2 test(s) for 1 file(s): 0 passed, 2 failed, 0 skipped Test Coverage SummaryValidated build type detection and severity ordering in the React DevTools extension, covering renderer event handling and build type aggregation logic. Two functional defects were identified: the build type reducer lacks proper severity ordering among non-production types (allowing less critical issues to overwrite more critical ones), and the event listener posts redundant messages when the effective build type hasn't changed, causing unnecessary UI updates. Potential Issues
Test Results (2 tests)
Model: testgen | Tokens: 212,132 (201,578 in, 10,554 out) | Cost: $0.7630 Generated by Greptile |
Author
|
Upstream PR was closed or merged. Code is synced via branch mirror. |
everettbu
deleted the
sebbie/03-10-_devtools_ignore_new_production_renderers_if_we_already_use_worse_versions_of_react_on_a_page
branch
March 11, 2026 10:24
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Mirror of facebook/react#35994
Original author: eps1lon
Summary
When a new build of React is detected on the screen, React DevTools will display a special icon and extension popup for this build. This helps discover if a page is accidentally using a development build.
However, right now the last seen React build is considered. This can hide if a React development build on the page is used.
Now we keep the seen React builds in state and only change the icon/popup if we see a new, worse React builds. Worse is is just any non-production build.
Originally reported in vercel/next.js#90744
How did you test this change?
npm run dev next) to ensure the development build icon is used