From 997f52fbb30ec4b777b66edcdd75f594d9fe0c55 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 3 Aug 2023 20:02:18 +0100 Subject: [PATCH] fix[devtools/updateFiberRecursively]: mount suspense fallback set in timed out case (#27147) Fixes https://github.com/facebook/react/issues/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. Screenshot 2023-07-25 at 15 26 07 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. --- .../react-devtools-shared/src/backend/renderer.js | 12 ++++++++++++ packages/react-devtools-shared/src/hook.js | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index e694339687..506afcedcf 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -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 && diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 39af7daa37..4df992e23e 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -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);