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.
facebook · Jun 22, 2023 · 794b770 · 794b770
1 parent 254cbdb
commit 794b770
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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<HTMLElement> = null;
-    if (renderer != null) {
-      nodes = ((renderer.findNativeNodesForFiberID(
-        id,
-      ): any): ?Array<HTMLElement>);
+    // In some cases fiber may already be unmounted
+    if (!renderer.hasFiberWithId(id)) {
+      hideOverlay(agent);
+      return;
     }
 
+    const nodes: ?Array<HTMLElement> = (renderer.findNativeNodesForFiberID(
+      id,
+    ): any);
+
     if (nodes != null && nodes[0] != null) {
       const node = nodes[0];
       // $FlowFixMe[method-unbinding]