From 9170e5695ada95dc6898d1fcd65dd6dfeefde347 Mon Sep 17 00:00:00 2001
From: Dan Abramov <dan.abramov@gmail.com>
Date: Fri, 5 Apr 2019 18:18:39 +0100
Subject: [PATCH] Fix double-adding fibers when traversing

---
 shells/dev/app/Toggle/index.js | 21 +++++++++
 shells/dev/app/index.js        |  2 +
 src/backend/renderer.js        | 13 ++++--
 src/devtools/store.js          | 84 +++++++++++++++-------------------
 4 files changed, 67 insertions(+), 53 deletions(-)
 create mode 100644 shells/dev/app/Toggle/index.js

diff --git a/shells/dev/app/Toggle/index.js b/shells/dev/app/Toggle/index.js
new file mode 100644
index 0000000000..5e96cd447b
--- /dev/null
+++ b/shells/dev/app/Toggle/index.js
@@ -0,0 +1,21 @@
+import React, { useState } from 'react';
+
+export default function Toggle() {
+  const [show, setShow] = useState(false);
+  return (
+    <>
+      <h2>Toggle</h2>
+      <div>
+        <>
+          <button onClick={() => setShow(s => !s)}>Show child</button>
+          {show && ' '}
+          {show && <Greeting>Hello</Greeting>}
+        </>
+      </div>
+    </>
+  );
+}
+
+function Greeting({ children }) {
+  return <p>{children}</p>;
+}
diff --git a/shells/dev/app/index.js b/shells/dev/app/index.js
index 1daae45595..4857c0c2cf 100644
--- a/shells/dev/app/index.js
+++ b/shells/dev/app/index.js
@@ -10,6 +10,7 @@ import ElementTypes from './ElementTypes';
 import InspectableElements from './InspectableElements';
 import InteractionTracing from './InteractionTracing';
 import ToDoList from './ToDoList';
+import Toggle from './Toggle';
 
 import './styles.css';
 
@@ -31,6 +32,7 @@ function mountTestApp() {
   mountHelper(InspectableElements);
   mountHelper(ElementTypes);
   mountHelper(EditableProps);
+  mountHelper(Toggle);
   mountHelper(DeeplyNestedComponents);
 }
 
diff --git a/src/backend/renderer.js b/src/backend/renderer.js
index 29caf523d7..cf0b973e68 100644
--- a/src/backend/renderer.js
+++ b/src/backend/renderer.js
@@ -743,23 +743,26 @@ export function attach(
     }
   }
 
-  function mountFiber(fiber: Fiber, parentFiber: Fiber | null) {
+  function mountFiber(
+    fiber: Fiber,
+    parentFiber: Fiber | null,
+    traverseSiblings = false
+  ) {
     if (__DEBUG__) {
       debug('mountFiber()', fiber, parentFiber);
     }
 
     const shouldEnqueueMount = !shouldFilterFiber(fiber);
-
     if (shouldEnqueueMount) {
       enqueueMount(fiber, parentFiber);
     }
 
     if (fiber.child !== null) {
-      mountFiber(fiber.child, shouldEnqueueMount ? fiber : parentFiber);
+      mountFiber(fiber.child, shouldEnqueueMount ? fiber : parentFiber, true);
     }
 
-    if (fiber.sibling) {
-      mountFiber(fiber.sibling, parentFiber);
+    if (traverseSiblings && fiber.sibling !== null) {
+      mountFiber(fiber.sibling, parentFiber, true);
     }
   }
 
diff --git a/src/devtools/store.js b/src/devtools/store.js
index e92f36845c..ccfe7985d6 100644
--- a/src/devtools/store.js
+++ b/src/devtools/store.js
@@ -487,32 +487,26 @@ export default class Store extends EventEmitter {
               debug('Add', `new root fiber ${id}`);
             }
 
-            if (this._idToElement.has(id)) {
-              // The renderer's tree walking approach sometimes mounts the same Fiber twice with Suspense and Lazy.
-              // For now, we avoid adding it to the tree twice by checking if it's already been mounted.
-              // Maybe in the future we'll revisit this.
-            } else {
-              const supportsProfiling = operations[i] > 0;
-              i++;
+            const supportsProfiling = operations[i] > 0;
+            i++;
 
-              this._roots = this._roots.concat(id);
-              this._rootIDToRendererID.set(id, rendererID);
-              this._rootIDToCapabilities.set(id, { supportsProfiling });
+            this._roots = this._roots.concat(id);
+            this._rootIDToRendererID.set(id, rendererID);
+            this._rootIDToCapabilities.set(id, { supportsProfiling });
 
-              this._idToElement.set(id, {
-                children: [],
-                depth: -1,
-                displayName: null,
-                id,
-                key: null,
-                ownerID: 0,
-                parentID: 0,
-                type,
-                weight: 0,
-              });
+            this._idToElement.set(id, {
+              children: [],
+              depth: -1,
+              displayName: null,
+              id,
+              key: null,
+              ownerID: 0,
+              parentID: 0,
+              type,
+              weight: 0,
+            });
 
-              haveRootsChanged = true;
-            }
+            haveRootsChanged = true;
           } else {
             parentID = ((operations[i]: any): number);
             i++;
@@ -545,35 +539,29 @@ export default class Store extends EventEmitter {
               );
             }
 
-            if (this._idToElement.has(id)) {
-              // The renderer's tree walking approach sometimes mounts the same Fiber twice with Suspense and Lazy.
-              // For now, we avoid adding it to the tree twice by checking if it's already been mounted.
-              // Maybe in the future we'll revisit this.
-            } else {
-              parentElement = ((this._idToElement.get(parentID): any): Element);
-              parentElement.children = parentElement.children.concat(id);
+            parentElement = ((this._idToElement.get(parentID): any): Element);
+            parentElement.children = parentElement.children.concat(id);
 
-              const element: Element = {
-                children: [],
-                depth: parentElement.depth + 1,
-                displayName,
-                id,
-                key,
-                ownerID,
-                parentID: parentElement.id,
-                type,
-                weight: 1,
-              };
+            const element: Element = {
+              children: [],
+              depth: parentElement.depth + 1,
+              displayName,
+              id,
+              key,
+              ownerID,
+              parentID: parentElement.id,
+              type,
+              weight: 1,
+            };
 
-              this._idToElement.set(id, element);
+            this._idToElement.set(id, element);
 
-              const oldAddedElementIDs = addedElementIDs;
-              addedElementIDs = new Uint32Array(addedElementIDs.length + 1);
-              addedElementIDs.set(oldAddedElementIDs);
-              addedElementIDs[oldAddedElementIDs.length] = id;
+            const oldAddedElementIDs = addedElementIDs;
+            addedElementIDs = new Uint32Array(addedElementIDs.length + 1);
+            addedElementIDs.set(oldAddedElementIDs);
+            addedElementIDs[oldAddedElementIDs.length] = id;
 
-              weightDelta = 1;
-            }
+            weightDelta = 1;
           }
           break;
         case TREE_OPERATION_REMOVE: