@samitouri / QOS-React / commits / cccc4ae2c6

Add index field to each fiber

We use this to track the index slot that this Fiber had at reconciliation time. We will use this for two purposes: 1) We use it to quickly determine if a move is needed. 2) We also use it to model a sparce linked list, since we can have null/false/undefined in our child set and these take up a slot for the implicit key.

Sebastian Markbage committed Sep 20, 2016 at 18:09 UTC cccc4ae2c61282938920ff57633df9e1513cc52a
3 files changed +148 -18
src/renderers/shared/fiber/ReactChildFiber.js
+76 -18
@@ -61,6 +61,11 @@ function deleteChild(
61 returnFiber : Fiber,
62 childToDelete : Fiber
63 ) {
64 + if (!shouldTrackSideEffects) {
65 + // Noop.
66 + return;
67 + }
68 +
69 // TODO: Add this child to the side-effect queue for deletion.
70 }
71
@@ -68,6 +73,10 @@ function deleteRemainingChildren(
73 returnFiber : Fiber,
74 currentFirstChild : ?Fiber
75 ) {
76 + if (!shouldTrackSideEffects) {
77 + // Noop.
78 + return null;
79 + }
80 // TODO: Add these children to the side-effect queue for deletion.
81 return null;
82 }
@@ -81,10 +90,6 @@ function mapAndDeleteRemainingChildren(
90 // we will then undo the deletion as we restore children. Implicit (null) keys
91 // don't get added to this set.
92 const existingChildren : Map<string, Fiber> = new Map();
84 - // TODO: This also needs to store the "previous index of this node". That lets
85 - // us determine whether something needs to be a placement. It might be best to
86 - // just store this on the fiber itself since that lets us use the "implicit"
87 - // index resolution mechanism without adding null values to the linked list.
93 let existingChild = currentFirstChild;
94 while (existingChild) {
95 if (existingChild.key !== null) {
@@ -109,10 +114,11 @@ function mapAndDeleteRemainingChildren(
114 function ChildReconciler(shouldClone, shouldTrackSideEffects) {
115
116 function useFiber(fiber : Fiber, priority : PriorityLevel) {
112 - // We currently set sibling to null here because it is easy to forget to do
113 - // before returning it.
117 + // We currently set sibling to null and index to 0 here because it is easy
118 + // to forget to do before returning it. E.g. for the single child case.
119 if (shouldClone) {
120 const clone = cloneFiber(fiber, priority);
121 + clone.index = 0;
122 clone.sibling = null;
123 return clone;
124 } else {
@@ -120,11 +126,36 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
126 fiber.pendingWorkPriority > priority) {
127 fiber.pendingWorkPriority = priority;
128 }
129 + fiber.index = 0;
130 fiber.sibling = null;
131 return fiber;
132 }
133 }
134
135 + function placeChild(newFiber : Fiber, lastPlacedIndex : number, newIndex : number) {
136 + newFiber.index = newIndex;
137 + if (!shouldTrackSideEffects) {
138 + // Noop.
139 + return lastPlacedIndex;
140 + }
141 + const current = newFiber.alternate;
142 + if (current) {
143 + const oldIndex = current.index;
144 + if (oldIndex < lastPlacedIndex) {
145 + // This is a move.
146 + // TODO: Schedule a move side-effect for this child.
147 + return lastPlacedIndex;
148 + } else {
149 + // This item can stay in place.
150 + return oldIndex;
151 + }
152 + } else {
153 + // This is an insertion.
154 + // TODO: Schedule an insertion side-effect for this child.
155 + return lastPlacedIndex;
156 + }
157 + }
158 +
159 function updateTextNode(
160 returnFiber : Fiber,
161 current : ?Fiber,
@@ -234,17 +265,19 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
265
266 function updateSlot(
267 returnFiber : Fiber,
237 - oldFiber : Fiber,
268 + oldFiber : ?Fiber,
269 newChild : any,
270 priority : PriorityLevel
271 ) : ?Fiber {
272 // Update the fiber if the keys match, otherwise return null.
273
274 + const key = oldFiber ? oldFiber.key : null;
275 +
276 if (typeof newChild === 'string' || typeof newChild === 'number') {
277 // Text nodes doesn't have keys. If the previous node is implicitly keyed
278 // we can continue to replace it without aborting even if it is not a text
279 // node.
247 - if (oldFiber.key !== null) {
280 + if (key !== null) {
281 return null;
282 }
283 return updateTextNode(returnFiber, oldFiber, '' + newChild, priority);
@@ -253,7 +286,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
286 if (typeof newChild === 'object' && newChild !== null) {
287 switch (newChild.$$typeof) {
288 case REACT_ELEMENT_TYPE: {
256 - if (newChild.key === oldFiber.key) {
289 + if (newChild.key === key) {
290 return updateElement(
291 returnFiber,
292 oldFiber,
@@ -266,7 +299,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
299 }
300
301 case REACT_COROUTINE_TYPE: {
269 - if (newChild.key === oldFiber.key) {
302 + if (newChild.key === key) {
303 return updateCoroutine(
304 returnFiber,
305 oldFiber,
@@ -279,7 +312,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
312 }
313
314 case REACT_YIELD_TYPE: {
282 - if (newChild.key === oldFiber.key) {
315 + if (newChild.key === key) {
316 return updateYield(
317 returnFiber,
318 oldFiber,
@@ -295,7 +328,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
328 if (isArray(newChild) || getIteratorFn(newChild)) {
329 // Fragments doesn't have keys so if the previous key is implicit we can
330 // update it.
298 - if (oldFiber.key !== null) {
331 + if (key !== null) {
332 return null;
333 }
334 return updateFragment(returnFiber, oldFiber, newChild, priority);
@@ -433,13 +466,32 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
466 let previousNewFiber : ?Fiber = null;
467
468 let oldFiber = currentFirstChild;
469 + let lastPlacedIndex = 0;
470 let newIdx = 0;
471 + let nextOldFiber = null;
472 for (; oldFiber && newIdx < newChildren.length; newIdx++) {
438 - const nextOldFiber = oldFiber.sibling; // In-case we mutate this fiber.
439 - const newFiber = updateSlot(returnFiber, oldFiber, newChildren[newIdx], priority);
473 + if (oldFiber) {
474 + if (oldFiber.index > newIdx) {
475 + nextOldFiber = oldFiber;
476 + oldFiber = null;
477 + } else {
478 + nextOldFiber = oldFiber.sibling;
479 + }
480 + }
481 + const newFiber = updateSlot(
482 + returnFiber,
483 + oldFiber,
484 + newChildren[newIdx],
485 + priority
486 + );
487 if (!newFiber) {
488 + // TODO: This breaks on empty slots like null children. That's
489 + // unfortunate because it triggers the slow path all the time. We need
490 + // a better way to communicate whether this was a miss or null,
491 + // boolean, undefined, etc.
492 break;
493 }
494 + lastPlacedIndex = placeChild(newFiber, lastPlacedIndex, newIdx);
495 if (!previousNewFiber) {
496 // TODO: Move out of the loop. This only happens for the first run.
497 resultingFirstChild = newFiber;
@@ -467,7 +519,14 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
519 for (; newIdx < newChildren.length; newIdx++) {
520 // TODO: Since the mutation of existing fibers can happen at any order
521 // we might break the link before we're done with it. :(
470 - const nextOldFiber = oldFiber ? oldFiber.sibling : null;
522 + if (oldFiber) {
523 + if (oldFiber.index > newIdx) {
524 + nextOldFiber = oldFiber;
525 + oldFiber = null;
526 + } else {
527 + nextOldFiber = oldFiber.sibling;
528 + }
529 + }
530 const newFiber = updateFromMap(
531 existingChildren,
532 returnFiber,
@@ -476,6 +535,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
535 priority
536 );
537 if (newFiber) {
538 + lastPlacedIndex = placeChild(newFiber, lastPlacedIndex, newIdx);
539 if (!previousNewFiber) {
540 resultingFirstChild = newFiber;
541 } else {
@@ -485,9 +545,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
545 }
546 // We will keep traversing the oldFiber in order, in case the new child
547 // has a null key that we'll need to match in the same slot.
488 - if (oldFiber) {
489 - oldFiber = nextOldFiber;
490 - }
548 + oldFiber = nextOldFiber;
549 }
550
551 // TODO: Add deletion side-effects to the returnFiber's side-effects.
src/renderers/shared/fiber/ReactFiber.js
+3
@@ -71,6 +71,7 @@ export type Fiber = Instance & {
71 // Singly Linked List Tree Structure.
72 child: ?Fiber,
73 sibling: ?Fiber,
74 + index: number,
75
76 // The ref last used to attach this node.
77 // I'll avoid adding an owner field for prod and model that as functions.
@@ -156,6 +157,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
157
158 child: null,
159 sibling: null,
160 + index: 0,
161
162 ref: null,
163
@@ -221,6 +223,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
223 alt.stateNode = fiber.stateNode;
224 alt.child = fiber.child;
225 alt.sibling = fiber.sibling; // This should always be overridden. TODO: null
226 + alt.index = fiber.index; // This should always be overridden.
227 alt.ref = fiber.ref;
228 // pendingProps is here for symmetry but is unnecessary in practice for now.
229 // TODO: Pass in the new pendingProps as an argument maybe?
src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js
+69
@@ -94,4 +94,73 @@ describe('ReactTopLevelFragment', function() {
94
95 });
96
97 + it('preserves state if an implicit key slot switches from/to null', function() {
98 +
99 + var instance = null;
100 +
101 + class Stateful extends React.Component {
102 + render() {
103 + instance = this;
104 + return <div>World</div>;
105 + }
106 + }
107 +
108 + function Fragment({ condition }) {
109 + return condition ? [null, <Stateful />] :
110 + [<div>Hello</div>, <Stateful />];
111 + }
112 + ReactNoop.render(<Fragment />);
113 + ReactNoop.flush();
114 +
115 + var instanceA = instance;
116 +
117 + expect(instanceA).not.toBe(null);
118 +
119 + ReactNoop.render(<Fragment condition={true} />);
120 + ReactNoop.flush();
121 +
122 + var instanceB = instance;
123 +
124 + expect(instanceB).toBe(instanceA);
125 +
126 + ReactNoop.render(<Fragment condition={false} />);
127 + ReactNoop.flush();
128 +
129 + var instanceC = instance;
130 +
131 + expect(instanceC === instanceA).toBe(true);
132 +
133 + });
134 +
135 + it('should preserve state in a reorder', function() {
136 +
137 + var instance = null;
138 +
139 + class Stateful extends React.Component {
140 + render() {
141 + instance = this;
142 + return <div>Hello</div>;
143 + }
144 + }
145 +
146 + function Fragment({ condition }) {
147 + return condition ? [[<div key="b">World</div>, <Stateful key="a" />]] :
148 + [[<Stateful key="a" />, <div key="b">World</div>], <div />];
149 + }
150 + ReactNoop.render(<Fragment />);
151 + ReactNoop.flush();
152 +
153 + var instanceA = instance;
154 +
155 + expect(instanceA).not.toBe(null);
156 +
157 + ReactNoop.render(<Fragment condition={true} />);
158 + ReactNoop.flush();
159 +
160 + var instanceB = instance;
161 +
162 + expect(instanceB).toBe(instanceA);
163 +
164 + });
165 +
166 });