From 794b770dbd6be48d42438ea76911d32c37c7dd6e Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 22 Jun 2023 08:51:24 +0100 Subject: [PATCH] fix[devtools]: check if fiber is unmounted before trying to highlight (#26983) For React Native environment, we sometimes spam the console with warnings `"Could not find Fiber with id ..."`. This is an attempt to fix this or at least reduce the amount of such potential warnings being thrown. Now checking if fiber is already unnmounted before trying to get native nodes for fiber. This might happen if you try to inspect an element in DevTools, but at the time when event has been received, the element was already unmounted. --- .../src/backend/legacy/renderer.js | 5 ++++ .../src/backend/renderer.js | 23 ++++++++----------- .../src/backend/types.js | 1 + .../src/backend/views/Highlighter/index.js | 16 +++++++++---- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 7790aec0e64b6..603d84d076548 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -1101,6 +1101,10 @@ export function attach( function unpatchConsoleForStrictMode() {} + function hasFiberWithId(id: number): boolean { + return idToInternalInstanceMap.has(id); + } + return { clearErrorsAndWarnings, clearErrorsForFiberID, @@ -1124,6 +1128,7 @@ export function attach( handleCommitFiberRoot, handleCommitFiberUnmount, handlePostCommitFiberRoot, + hasFiberWithId, inspectElement, logElementToConsole, overrideError, diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 7b12e583a9b76..69b41015f6310 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -2736,21 +2736,11 @@ export function attach( function findNativeNodesForFiberID(id: number) { try { - let fiber = findCurrentFiberUsingSlowPathById(id); + const fiber = findCurrentFiberUsingSlowPathById(id); if (fiber === null) { return null; } - // Special case for a timed-out Suspense. - const isTimedOutSuspense = - fiber.tag === SuspenseComponent && fiber.memoizedState !== null; - if (isTimedOutSuspense) { - // A timed-out Suspense's findDOMNode is useless. - // Try our best to find the fallback directly. - const maybeFallbackFiber = fiber.child && fiber.child.sibling; - if (maybeFallbackFiber != null) { - fiber = maybeFallbackFiber; - } - } + const hostFibers = findAllCurrentHostFibers(id); return hostFibers.map(hostFiber => hostFiber.stateNode).filter(Boolean); } catch (err) { @@ -2759,9 +2749,9 @@ export function attach( } } - function getDisplayNameForFiberID(id: number) { + function getDisplayNameForFiberID(id: number): null | string { const fiber = idToArbitraryFiberMap.get(id); - return fiber != null ? getDisplayNameForFiber(((fiber: any): Fiber)) : null; + return fiber != null ? getDisplayNameForFiber(fiber) : null; } function getFiberForNative(hostInstance: NativeType) { @@ -4456,6 +4446,10 @@ export function attach( traceUpdatesEnabled = isEnabled; } + function hasFiberWithId(id: number): boolean { + return idToArbitraryFiberMap.has(id); + } + return { cleanup, clearErrorsAndWarnings, @@ -4476,6 +4470,7 @@ export function attach( handleCommitFiberRoot, handleCommitFiberUnmount, handlePostCommitFiberRoot, + hasFiberWithId, inspectElement, logElementToConsole, patchConsoleForStrictMode, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 758ce827cf27f..c3a2f35a5d59b 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -379,6 +379,7 @@ export type RendererInterface = { handleCommitFiberRoot: (fiber: Object, commitPriority?: number) => void, handleCommitFiberUnmount: (fiber: Object) => void, handlePostCommitFiberRoot: (fiber: Object) => void, + hasFiberWithId: (id: number) => boolean, inspectElement: ( requestID: number, id: number, diff --git a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js index 207b112866abe..3f1ad02dbfcf7 100644 --- a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js +++ b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js @@ -104,15 +104,21 @@ export default function setupHighlighter( const renderer = agent.rendererInterfaces[rendererID]; if (renderer == null) { console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); + + hideOverlay(agent); + return; } - let nodes: ?Array = null; - if (renderer != null) { - nodes = ((renderer.findNativeNodesForFiberID( - id, - ): any): ?Array); + // In some cases fiber may already be unmounted + if (!renderer.hasFiberWithId(id)) { + hideOverlay(agent); + return; } + const nodes: ?Array = (renderer.findNativeNodesForFiberID( + id, + ): any); + if (nodes != null && nodes[0] != null) { const node = nodes[0]; // $FlowFixMe[method-unbinding]