Skip to content

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Sep 11, 2025

Stacked on #34464

DevTools previously always mounted the Root Fiber when it first encountered it. However, it also considered it newly mounted when we went from no children to some children. Duplicate mounts are not supported by the Store.

This was especially noticeable if you error in the shell e.g. render(<BrokenRender>) and then recover later (or just go from render(null) to render(<Something />)). This case is already covered by our runtime tests:

await act(async () => {
root2.render(<span>After 2</span>);
});

The alternate to this would be to always consider commits as mounts or updates and bring back handleCommitFiberUnmount. Since we never fully removed handleCommitFiberUnmount, that may be save? Though I wouldn't know if that's compatible with all renderers. So not mounting new, empty roots seems simpler considering we already treated updates to empty roots as unmounts.

@meta-cla meta-cla bot added the CLA Signed label Sep 11, 2025
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Sep 11, 2025
@eps1lon eps1lon marked this pull request as ready for review September 11, 2025 13:51
Comment on lines 5409 to 5380
if (prevFiber === null) {
throw new Error(
'Expected previous fiber when updating an existing root',
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This never runs, since wasMounted implies that prevFiber !== null.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it's for Flow.js. We have this in a lot of places in the backend. Especially to guard against moving this code further apart in the future.

@eps1lon eps1lon force-pushed the sebbie/09-11-_devtools_stop_mounting_empty_roots branch from 436bee1 to badf59c Compare September 11, 2025 17:14
@eps1lon eps1lon merged commit a9ad64c into facebook:main Sep 11, 2025
241 checks passed
@eps1lon eps1lon deleted the sebbie/09-11-_devtools_stop_mounting_empty_roots branch September 11, 2025 18:01
github-actions bot pushed a commit to code/lib-react that referenced this pull request Sep 13, 2025
github-actions bot pushed a commit to code/lib-react that referenced this pull request Sep 13, 2025
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