Skip to content

Commit

Permalink
fix[devtools/updateFiberRecursively]: mount suspense fallback set in …
Browse files Browse the repository at this point in the history
…timed out case (#27147)

Fixes #26793.

I have received a constantly reproducible example of the error, that is
mentioned in the issue above.
When starting `Reload and Profile` in DevTools, React reports an unmount
of a functional component inside Suspense's fallback via
[`onCommitFiberUnmount`](https://github.com/facebook/react/blob/3ff846d106de9273f59d1e4457793a5fcf625aef/packages/react-devtools-shared/src/hook.js#L408-L413)
in
[`commitDeletionEffectsOnFiber`](https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberCommitWork.js#L2025),
but this fiber was never registered as mounted in DevTools.

While debugging, I've noticed that in timed-out case for Suspense trees
we only check if both previous fallback child set and next fiber
fallback child set are non-null, but in these recursive calls there is
also a case when previous fallback child set is null and next set is
non-null, so we were skipping the branch.

<img width="1746" alt="Screenshot 2023-07-25 at 15 26 07"
src="https://github.com/facebook/react/assets/28902667/da21a682-9973-43ec-9653-254ba98a0a3f">

After these changes, the issue is no longer reproducible, but I am not
sure if this is the right solution, since I don't know if this case is
correct from reconciler perspective.
  • Loading branch information
hoxyq authored Aug 3, 2023
1 parent 493f72b commit 997f52f
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
12 changes: 12 additions & 0 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,18 @@ export function attach(
const prevFallbackChildSet = prevFiberChild
? prevFiberChild.sibling
: null;

if (prevFallbackChildSet == null && nextFallbackChildSet != null) {
mountFiberRecursively(
nextFallbackChildSet,
shouldIncludeInTree ? nextFiber : parentFiber,
true,
traceNearestHostComponentUpdate,
);

shouldResetChildren = true;
}

if (
nextFallbackChildSet != null &&
prevFallbackChildSet != null &&
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ export function installHook(target: any): DevToolsHook | null {

let uidCounter = 0;

function inject(renderer: ReactRenderer) {
function inject(renderer: ReactRenderer): number {
const id = ++uidCounter;
renderers.set(id, renderer);

Expand Down

0 comments on commit 997f52f

Please sign in to comment.