Repository: facebook/react
Pull request: [DevTools] Stop mounting empty roots

Stacked on https://github.com/facebook/react/pull/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: https://github.com/facebook/react/blob/436bee1be7d2af736bfd1e3a768eee10b40300a1/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js#L698-L700

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.

Changed files:
filename: packages/react-devtools-shared/src/__tests__/store-test.js
patch: @@ -2489,7 +2489,7 @@ describe('Store', () => {
       withErrorsOrWarningsIgnored(['test-only:'], async () => {
         await act(() => render(<React.Fragment />));
       });
-      expect(store).toMatchInlineSnapshot(`[root]`);
+      expect(store).toMatchInlineSnapshot(``);
       expect(store.componentWithErrorCount).toBe(0);
       expect(store.componentWithWarningCount).toBe(0);
     });
@@ -3083,6 +3083,9 @@ describe('Store', () => {
 
   it('should handle an empty root', async () => {
     await actAsync(() => render(null));
+    expect(store).toMatchInlineSnapshot(``);
+
+    await actAsync(() => render(<span />));
     expect(store).toMatchInlineSnapshot(`[root]`);
   });
 });

filename: packages/react-devtools-shared/src/backend/fiber/renderer.js
patch: @@ -5322,12 +5322,12 @@ export function attach(
     root: FiberRoot,
     priorityLevel: void | number,
   ) {
-    const current = root.current;
+    const nextFiber = root.current;
 
     let prevFiber: null | Fiber = null;
     let rootInstance = rootToFiberInstanceMap.get(root);
     if (!rootInstance) {
-      rootInstance = createFiberInstance(current);
+      rootInstance = createFiberInstance(nextFiber);
       rootToFiberInstanceMap.set(root, rootInstance);
       idToDevToolsInstanceMap.set(rootInstance.id, rootInstance);
     } else {
@@ -5366,30 +5366,25 @@ export function attach(
       };
     }
 
-    if (prevFiber !== null) {
-      // TODO: relying on this seems a bit fishy.
-      const wasMounted =
-        prevFiber.memoizedState != null &&
-        prevFiber.memoizedState.element != null;
-      const isMounted =
-        current.memoizedState != null && current.memoizedState.element != null;
-      if (!wasMounted && isMounted) {
-        // Mount a new root.
-        setRootPseudoKey(currentRoot.id, current);
-        mountFiberRecursively(current, false);
-      } else if (wasMounted && isMounted) {
-        // Update an existing root.
-        updateFiberRecursively(rootInstance, current, prevFiber, false);
-      } else if (wasMounted && !isMounted) {
-        // Unmount an existing root.
-        unmountInstanceRecursively(rootInstance);
-        removeRootPseudoKey(currentRoot.id);
-        rootToFiberInstanceMap.delete(root);
-      }
-    } else {
+    const nextIsMounted = nextFiber.child !== null;
+    const prevWasMounted = prevFiber !== null && prevFiber.child !== null;
+    if (!prevWasMounted && nextIsMounted) {
       // Mount a new root.
-      setRootPseudoKey(currentRoot.id, current);
-      mountFiberRecursively(current, false);
+      setRootPseudoKey(currentRoot.id, nextFiber);
+      mountFiberRecursively(nextFiber, false);
+    } else if (prevWasMounted && nextIsMounted) {
+      if (prevFiber === null) {
+        throw new Error(
+          'Expected a previous Fiber when updating an existing root.',
+        );
+      }
+      // Update an existing root.
+      updateFiberRecursively(rootInstance, nextFiber, prevFiber, false);
+    } else if (prevWasMounted && !nextIsMounted) {
+      // Unmount an existing root.
+      unmountInstanceRecursively(rootInstance);
+      removeRootPseudoKey(currentRoot.id);
+      rootToFiberInstanceMap.delete(root);
     }
 
     if (isProfiling && isProfilingSupported) {