Fix double-adding fibers when traversing
Dan Abramov committed
Apr 5, 2019 at 18:18 UTC
9170e5695ada95dc6898d1fcd65dd6dfeefde347
4 files changed
+74
-60
shells/dev/app/Toggle/index.js
new
+21
@@ -0,0 +1,21 @@
1
+import React, { useState } from 'react';
2
+
3
+export default function Toggle() {
4
+ const [show, setShow] = useState(false);
5
+ return (
6
+ <>
7
+ <h2>Toggle</h2>
8
+ <div>
9
+ <>
10
+ <button onClick={() => setShow(s => !s)}>Show child</button>
11
+ {show && ' '}
12
+ {show && <Greeting>Hello</Greeting>}
13
+ </>
14
+ </div>
15
+ </>
16
+ );
17
+}
18
+
19
+function Greeting({ children }) {
20
+ return <p>{children}</p>;
21
+}
shells/dev/app/index.js
+2
@@ -10,6 +10,7 @@ import ElementTypes from './ElementTypes';
10
import InspectableElements from './InspectableElements';
11
import InteractionTracing from './InteractionTracing';
12
import ToDoList from './ToDoList';
13
+import Toggle from './Toggle';
14
15
import './styles.css';
16
@@ -31,6 +32,7 @@ function mountTestApp() {
32
mountHelper(InspectableElements);
33
mountHelper(ElementTypes);
34
mountHelper(EditableProps);
35
+ mountHelper(Toggle);
36
mountHelper(DeeplyNestedComponents);
37
}
38
src/backend/renderer.js
+8
-5
@@ -743,23 +743,26 @@ export function attach(
743
}
744
}
745
746
- function mountFiber(fiber: Fiber, parentFiber: Fiber | null) {
746
+ function mountFiber(
747
+ fiber: Fiber,
748
+ parentFiber: Fiber | null,
749
+ traverseSiblings = false
750
+ ) {
751
if (__DEBUG__) {
752
debug('mountFiber()', fiber, parentFiber);
753
}
754
755
const shouldEnqueueMount = !shouldFilterFiber(fiber);
752
-
756
if (shouldEnqueueMount) {
757
enqueueMount(fiber, parentFiber);
758
}
759
760
if (fiber.child !== null) {
758
- mountFiber(fiber.child, shouldEnqueueMount ? fiber : parentFiber);
761
+ mountFiber(fiber.child, shouldEnqueueMount ? fiber : parentFiber, true);
762
}
763
761
- if (fiber.sibling) {
762
- mountFiber(fiber.sibling, parentFiber);
764
+ if (traverseSiblings && fiber.sibling !== null) {
765
+ mountFiber(fiber.sibling, parentFiber, true);
766
}
767
}
768
src/devtools/store.js
+43
-55
@@ -487,32 +487,26 @@ export default class Store extends EventEmitter {
487
debug('Add', `new root fiber ${id}`);
488
}
489
490
- if (this._idToElement.has(id)) {
491
- // The renderer's tree walking approach sometimes mounts the same Fiber twice with Suspense and Lazy.
492
- // For now, we avoid adding it to the tree twice by checking if it's already been mounted.
493
- // Maybe in the future we'll revisit this.
494
- } else {
495
- const supportsProfiling = operations[i] > 0;
496
- i++;
497
-
498
- this._roots = this._roots.concat(id);
499
- this._rootIDToRendererID.set(id, rendererID);
500
- this._rootIDToCapabilities.set(id, { supportsProfiling });
501
-
502
- this._idToElement.set(id, {
503
- children: [],
504
- depth: -1,
505
- displayName: null,
506
- id,
507
- key: null,
508
- ownerID: 0,
509
- parentID: 0,
510
- type,
511
- weight: 0,
512
- });
513
-
514
- haveRootsChanged = true;
515
- }
490
+ const supportsProfiling = operations[i] > 0;
491
+ i++;
492
+
493
+ this._roots = this._roots.concat(id);
494
+ this._rootIDToRendererID.set(id, rendererID);
495
+ this._rootIDToCapabilities.set(id, { supportsProfiling });
496
+
497
+ this._idToElement.set(id, {
498
+ children: [],
499
+ depth: -1,
500
+ displayName: null,
501
+ id,
502
+ key: null,
503
+ ownerID: 0,
504
+ parentID: 0,
505
+ type,
506
+ weight: 0,
507
+ });
508
+
509
+ haveRootsChanged = true;
510
} else {
511
parentID = ((operations[i]: any): number);
512
i++;
@@ -545,35 +539,29 @@ export default class Store extends EventEmitter {
539
);
540
}
541
548
- if (this._idToElement.has(id)) {
549
- // The renderer's tree walking approach sometimes mounts the same Fiber twice with Suspense and Lazy.
550
- // For now, we avoid adding it to the tree twice by checking if it's already been mounted.
551
- // Maybe in the future we'll revisit this.
552
- } else {
553
- parentElement = ((this._idToElement.get(parentID): any): Element);
554
- parentElement.children = parentElement.children.concat(id);
555
-
556
- const element: Element = {
557
- children: [],
558
- depth: parentElement.depth + 1,
559
- displayName,
560
- id,
561
- key,
562
- ownerID,
563
- parentID: parentElement.id,
564
- type,
565
- weight: 1,
566
- };
567
-
568
- this._idToElement.set(id, element);
569
-
570
- const oldAddedElementIDs = addedElementIDs;
571
- addedElementIDs = new Uint32Array(addedElementIDs.length + 1);
572
- addedElementIDs.set(oldAddedElementIDs);
573
- addedElementIDs[oldAddedElementIDs.length] = id;
574
-
575
- weightDelta = 1;
576
- }
542
+ parentElement = ((this._idToElement.get(parentID): any): Element);
543
+ parentElement.children = parentElement.children.concat(id);
544
+
545
+ const element: Element = {
546
+ children: [],
547
+ depth: parentElement.depth + 1,
548
+ displayName,
549
+ id,
550
+ key,
551
+ ownerID,
552
+ parentID: parentElement.id,
553
+ type,
554
+ weight: 1,
555
+ };
556
+
557
+ this._idToElement.set(id, element);
558
+
559
+ const oldAddedElementIDs = addedElementIDs;
560
+ addedElementIDs = new Uint32Array(addedElementIDs.length + 1);
561
+ addedElementIDs.set(oldAddedElementIDs);
562
+ addedElementIDs[oldAddedElementIDs.length] = id;
563
+
564
+ weightDelta = 1;
565
}
566
break;
567
case TREE_OPERATION_REMOVE: