Skip to content

[DevTools] Fix a crash when rendering a new class Component when simulating errored state#672

Closed
everettbu wants to merge 2 commits into
mainfrom
sebbie/rdt-force-error-new
Closed

[DevTools] Fix a crash when rendering a new class Component when simulating errored state#672
everettbu wants to merge 2 commits into
mainfrom
sebbie/rdt-force-error-new

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35985
Original author: eps1lon


Summary

The Fiber reconciler assumes that only committed Error boundaries could have been placed in forceErrorForFibers. However, DevTools let any unknown Fiber be considered a boundary for which we previously simulated an error. Not only does that incorrectly trigger updates resetting the state, it also causes a crash if we're rendering a Class Component for the first time as a result of simulating an errored state.

Now we stop considering all class Components as previously being an error boundary in fallback state. I left a comment in the Fiber reconciler why it's safe to assume that the instance is non-nullable when we're in the shouldError(fiber) === false path.

How did you test this change?

  • added test that reprod the crash

@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 9, 2026
@greptile-apps

greptile-apps Bot commented Mar 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a crash in the React DevTools error-simulation feature (overrideError) that occurred when toggling a forced error off on an ErrorBoundary whose fallback had rendered a new class component (one that had never been committed before).

Root cause: shouldErrorFiberAccordingToMap in renderer.js previously returned false for any fiber not present in forceErrorForFibers. In ReactFiberBeginWork.js, case false: unconditionally accessed workInProgress.stateNode (the class instance), which is null for a component being mounted for the first time — causing the crash.

Fix:

  • renderer.js: Return null instead of false for unknown fibers. A null return falls through the switch without matching case false: or case true:, so newly-mounted class components are left completely untouched.
  • ReactFiberBeginWork.js: Add a comment documenting the now-guaranteed invariant that workInProgress.stateNode is non-null in case false: (the component must have been committed in a prior render for it to appear in the error map).
  • New test that reproduces the crash and verifies both the errored and reset states render correctly.

The change is minimal, well-targeted, and the added comment improves long-term maintainability of the invariant.

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, well-scoped bug fix with a regression test covering the crash scenario.
  • The change is confined to a DEV-only code path, the logic is straightforward (returning null instead of false for unknown fibers), the invariant in the reconciler is correctly documented, and a new test directly reproduces and verifies the fix. The only minor issue is a slightly misleading test description.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/react-devtools-shared/src/backend/fiber/renderer.js Changes shouldErrorFiberAccordingToMap return type from boolean to `boolean
packages/react-reconciler/src/ReactFiberBeginWork.js Adds a clarifying comment to updateClassComponent's case false: block explaining why workInProgress.stateNode is guaranteed non-null in that path (the component must have been committed in a previous render). No logic changes — the comment documents the invariant that was previously implicit and unsafe when renderer.js returned false for unknown fibers.
packages/react-devtools-shared/src/tests/storeForceError-test.js New test file that reproduces the crash: mounts an ErrorBoundary, forces it into an errored state (which mounts a new AnyClassComponent as the fallback), then resets the error and verifies the original tree is restored. The test title is slightly misleading ("when filters are changed" when no filters are modified).

Fix All in Claude Code Fix All in Codex

Last reviewed commit: 443bbe0

const {render} = getVersionedRenderImplementation();

// @reactVersion >= 18.0
it('resets forced error and fallback states when filters are changed', async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misleading test description

The test description says "resets forced error and fallback states when filters are changed", but the test body never modifies store.componentFilters. It exercises toggling forceError on/off via agent.overrideError. The phrase "when filters are changed" likely was copied from another test suite and left unedited.

Suggested change
it('resets forced error and fallback states when filters are changed', async () => {
it('resets forced error and fallback states when simulated error is toggled off', async () => {
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/__tests__/storeForceError-test.js
Line: 37

Comment:
**Misleading test description**

The test description says *"resets forced error and fallback states when filters are changed"*, but the test body never modifies `store.componentFilters`. It exercises toggling `forceError` on/off via `agent.overrideError`. The phrase "when filters are changed" likely was copied from another test suite and left unedited.

```suggestion
  it('resets forced error and fallback states when simulated error is toggled off', async () => {
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

@everettbu

Copy link
Copy Markdown
Author

Upstream PR was closed or merged. Code is synced via branch mirror.

@everettbu everettbu closed this Mar 10, 2026
@everettbu
everettbu deleted the sebbie/rdt-force-error-new branch March 10, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants