@samitouri / QOS-React / commits / 27926572f6

Move nodes around by reference instead of by index

This makes things easier if we ever want to use more than one DOM node for a component. Notably, this is more convenient if we want to remove the wrappers around text components (since text nodes can be split and joined however a browser feels like) or if we want to support returning more than one element from render (#2127). I left the old indexes so that implementations aren't forced to use the node/image if they prefer indices, because I'm not sure yet whether the changes corresponding to my rewrite of DOMChildrenOperations are easy or hard yet in React Native. (The tests pass with and without the DOMChildrenOperations changes here.)

Ben Alpert committed Jan 4, 2016 at 19:28 UTC 27926572f6bd468d8dc4e7e855ea9ce87fc4b406
3 files changed +90 -126
src/renderers/dom/client/utils/DOMChildrenOperations.js
+16 -86
@@ -18,7 +18,10 @@ var ReactPerf = require('ReactPerf');
18
19 var setInnerHTML = require('setInnerHTML');
20 var setTextContent = require('setTextContent');
21 -var invariant = require('invariant');
21 +
22 +function getNodeAfter(parentNode, node) {
23 + return node ? node.nextSibling : parentNode.firstChild;
24 +}
25
26 /**
27 * Inserts `childNode` as a child of `parentNode` at the `index`.
@@ -28,27 +31,14 @@ var invariant = require('invariant');
31 * @param {number} index Index at which to insert the child.
32 * @internal
33 */
31 -function insertChildAt(parentNode, childNode, index) {
32 - // We can rely exclusively on `insertBefore(node, null)` instead of also using
34 +function insertChildAt(parentNode, childNode, referenceNode) {
35 + // We rely exclusively on `insertBefore(node, null)` instead of also using
36 // `appendChild(node)`. (Using `undefined` is not allowed by all browsers so
37 // we are careful to use `null`.)
35 -
36 - // In Safari, .childNodes[index] can return a DOM node with id={index} so we
37 - // use .item() instead which is immune to this bug. (See #3560.) In contrast
38 - // to the spec, IE8 throws an error if index is larger than the list size.
39 - var referenceNode =
40 - index < parentNode.childNodes.length ?
41 - parentNode.childNodes.item(index) : null;
42 -
38 parentNode.insertBefore(childNode, referenceNode);
39 }
40
46 -function insertLazyTreeChildAt(parentNode, childTree, index) {
47 - // See above.
48 - var referenceNode =
49 - index < parentNode.childNodes.length ?
50 - parentNode.childNodes.item(index) : null;
51 -
41 +function insertLazyTreeChildAt(parentNode, childTree, referenceNode) {
42 DOMLazyTree.insertTreeBefore(parentNode, childTree, referenceNode);
43 }
44
@@ -69,81 +59,21 @@ var DOMChildrenOperations = {
59 * @internal
60 */
61 processUpdates: function(parentNode, updates) {
72 - var update;
73 - // Mapping from parent IDs to initial child orderings.
74 - var initialChildren = null;
75 - // List of children that will be moved or removed.
76 - var updatedChildren = null;
77 -
78 - var markupList = null;
79 -
80 - for (var i = 0; i < updates.length; i++) {
81 - update = updates[i];
82 - if (update.type === ReactMultiChildUpdateTypes.MOVE_EXISTING ||
83 - update.type === ReactMultiChildUpdateTypes.REMOVE_NODE) {
84 - var updatedIndex = update.fromIndex;
85 - var updatedChild = parentNode.childNodes[updatedIndex];
86 -
87 - invariant(
88 - updatedChild,
89 - 'processUpdates(): Unable to find child %s of element %s. This ' +
90 - 'probably means the DOM was unexpectedly mutated (e.g., by the ' +
91 - 'browser), usually due to forgetting a <tbody> when using tables, ' +
92 - 'nesting tags like <form>, <p>, or <a>, or using non-SVG elements ' +
93 - 'in an <svg> parent.',
94 - updatedIndex,
95 - parentNode,
96 - );
97 -
98 - initialChildren = initialChildren || {};
99 - initialChildren[updatedIndex] = updatedChild;
100 -
101 - updatedChildren = updatedChildren || [];
102 - updatedChildren.push(updatedChild);
103 - } else if (update.type === ReactMultiChildUpdateTypes.INSERT_MARKUP) {
104 - // Replace each HTML string with an index into the markup list
105 - if (typeof update.content === 'string') {
106 - markupList = markupList || [];
107 - update.content = markupList.push(update.markup);
108 - }
109 - }
110 - }
111 -
112 - var renderedMarkup;
113 - if (markupList) {
114 - renderedMarkup = Danger.dangerouslyRenderMarkup(markupList);
115 - }
116 -
117 - // Remove updated children first so that `toIndex` is consistent.
118 - if (updatedChildren) {
119 - for (var j = 0; j < updatedChildren.length; j++) {
120 - parentNode.removeChild(updatedChildren[j]);
121 - }
122 - }
123 -
62 for (var k = 0; k < updates.length; k++) {
125 - update = updates[k];
63 + var update = updates[k];
64 switch (update.type) {
65 case ReactMultiChildUpdateTypes.INSERT_MARKUP:
128 - if (renderedMarkup) {
129 - insertChildAt(
130 - parentNode,
131 - renderedMarkup[update.content],
132 - update.toIndex
133 - );
134 - } else {
135 - insertLazyTreeChildAt(
136 - parentNode,
137 - update.content,
138 - update.toIndex
139 - );
140 - }
66 + insertLazyTreeChildAt(
67 + parentNode,
68 + update.content,
69 + getNodeAfter(parentNode, update.afterNode)
70 + );
71 break;
72 case ReactMultiChildUpdateTypes.MOVE_EXISTING:
73 insertChildAt(
74 parentNode,
145 - initialChildren[update.fromIndex],
146 - update.toIndex
75 + update.fromNode,
76 + getNodeAfter(parentNode, update.afterNode)
77 );
78 break;
79 case ReactMultiChildUpdateTypes.SET_MARKUP:
@@ -159,7 +89,7 @@ var DOMChildrenOperations = {
89 );
90 break;
91 case ReactMultiChildUpdateTypes.REMOVE_NODE:
162 - // Already removed by the for-loop above.
92 + parentNode.removeChild(update.fromNode);
93 break;
94 }
95 }
src/renderers/shared/reconciler/ReactChildReconciler.js
+9 -5
@@ -71,6 +71,7 @@ var ReactChildReconciler = {
71 updateChildren: function(
72 prevChildren,
73 nextChildren,
74 + removedNodes,
75 transaction,
76 context) {
77 // We currently don't have a way to track moves here but if we use iterators
@@ -79,14 +80,15 @@ var ReactChildReconciler = {
80 // TODO: If nothing has changed, return the prevChildren object so that we
81 // can quickly bailout if nothing has changed.
82 if (!nextChildren && !prevChildren) {
82 - return null;
83 + return;
84 }
85 var name;
86 + var prevChild;
87 for (name in nextChildren) {
88 if (!nextChildren.hasOwnProperty(name)) {
89 continue;
90 }
89 - var prevChild = prevChildren && prevChildren[name];
91 + prevChild = prevChildren && prevChildren[name];
92 var prevElement = prevChild && prevChild._currentElement;
93 var nextElement = nextChildren[name];
94 if (prevChild != null &&
@@ -97,7 +99,8 @@ var ReactChildReconciler = {
99 nextChildren[name] = prevChild;
100 } else {
101 if (prevChild) {
100 - ReactReconciler.unmountComponent(prevChild, name);
102 + removedNodes[name] = ReactReconciler.getNativeNode(prevChild);
103 + ReactReconciler.unmountComponent(prevChild);
104 }
105 // The child must be instantiated before it's mounted.
106 var nextChildInstance = instantiateReactComponent(nextElement);
@@ -108,10 +111,11 @@ var ReactChildReconciler = {
111 for (name in prevChildren) {
112 if (prevChildren.hasOwnProperty(name) &&
113 !(nextChildren && nextChildren.hasOwnProperty(name))) {
111 - ReactReconciler.unmountComponent(prevChildren[name]);
114 + prevChild = prevChildren[name];
115 + removedNodes[name] = ReactReconciler.getNativeNode(prevChild);
116 + ReactReconciler.unmountComponent(prevChild);
117 }
118 }
114 - return nextChildren;
119 },
120
121 /**
src/renderers/shared/reconciler/ReactMultiChild.js
+65 -35
@@ -19,6 +19,7 @@ var ReactReconciler = require('ReactReconciler');
19 var ReactChildReconciler = require('ReactChildReconciler');
20
21 var flattenChildren = require('flattenChildren');
22 +var invariant = require('invariant');
23
24 /**
25 * Make an update for markup to be rendered and inserted at a supplied index.
@@ -27,13 +28,15 @@ var flattenChildren = require('flattenChildren');
28 * @param {number} toIndex Destination index.
29 * @private
30 */
30 -function makeInsertMarkup(markup, toIndex) {
31 +function makeInsertMarkup(markup, afterNode, toIndex) {
32 // NOTE: Null values reduce hidden classes.
33 return {
34 type: ReactMultiChildUpdateTypes.INSERT_MARKUP,
35 content: markup,
36 fromIndex: null,
37 + fromNode: null,
38 toIndex: toIndex,
39 + afterNode: afterNode,
40 };
41 }
42
@@ -44,13 +47,15 @@ function makeInsertMarkup(markup, toIndex) {
47 * @param {number} toIndex Destination index of the element.
48 * @private
49 */
47 -function makeMove(fromIndex, toIndex) {
50 +function makeMove(child, afterNode, toIndex) {
51 // NOTE: Null values reduce hidden classes.
52 return {
53 type: ReactMultiChildUpdateTypes.MOVE_EXISTING,
54 content: null,
52 - fromIndex: fromIndex,
55 + fromIndex: child._mountIndex,
56 + fromNode: ReactReconciler.getNativeNode(child),
57 toIndex: toIndex,
58 + afterNode: afterNode,
59 };
60 }
61
@@ -60,13 +65,15 @@ function makeMove(fromIndex, toIndex) {
65 * @param {number} fromIndex Index of the element to remove.
66 * @private
67 */
63 -function makeRemove(fromIndex) {
68 +function makeRemove(child, node) {
69 // NOTE: Null values reduce hidden classes.
70 return {
71 type: ReactMultiChildUpdateTypes.REMOVE_NODE,
72 content: null,
68 - fromIndex: fromIndex,
73 + fromIndex: child._mountIndex,
74 + fromNode: node,
75 toIndex: null,
76 + afterNode: null,
77 };
78 }
79
@@ -82,7 +89,9 @@ function makeSetMarkup(markup) {
89 type: ReactMultiChildUpdateTypes.SET_MARKUP,
90 content: markup,
91 fromIndex: null,
92 + fromNode: null,
93 toIndex: null,
94 + afterNode: null,
95 };
96 }
97
@@ -98,7 +107,9 @@ function makeTextContent(textContent) {
107 type: ReactMultiChildUpdateTypes.TEXT_CONTENT,
108 content: textContent,
109 fromIndex: null,
110 + fromNode: null,
111 toIndex: null,
112 + afterNode: null,
113 };
114 }
115
@@ -161,7 +172,13 @@ var ReactMultiChild = {
172 );
173 },
174
164 - _reconcilerUpdateChildren: function(prevChildren, nextNestedChildrenElements, transaction, context) {
175 + _reconcilerUpdateChildren: function(
176 + prevChildren,
177 + nextNestedChildrenElements,
178 + removedNodes,
179 + transaction,
180 + context
181 + ) {
182 var nextChildren;
183 if (__DEV__) {
184 if (this._currentElement) {
@@ -171,15 +188,17 @@ var ReactMultiChild = {
188 } finally {
189 ReactCurrentOwner.current = null;
190 }
174 - return ReactChildReconciler.updateChildren(
175 - prevChildren, nextChildren, transaction, context
191 + ReactChildReconciler.updateChildren(
192 + prevChildren, nextChildren, removedNodes, transaction, context
193 );
194 + return nextChildren;
195 }
196 }
197 nextChildren = flattenChildren(nextNestedChildrenElements);
180 - return ReactChildReconciler.updateChildren(
181 - prevChildren, nextChildren, transaction, context
198 + ReactChildReconciler.updateChildren(
199 + prevChildren, nextChildren, removedNodes, transaction, context
200 );
201 + return nextChildren;
202 },
203
204 /**
@@ -224,15 +243,13 @@ var ReactMultiChild = {
243 var prevChildren = this._renderedChildren;
244 // Remove any rendered children.
245 ReactChildReconciler.unmountChildren(prevChildren);
227 - // TODO: The setTextContent operation should be enough
228 - var updates = [];
246 for (var name in prevChildren) {
247 if (prevChildren.hasOwnProperty(name)) {
231 - updates.push(this._unmountChild(prevChildren[name]));
248 + invariant(false, 'updateTextContent called on non-empty component.');
249 }
250 }
251 // Set new text content.
235 - updates.push(makeTextContent(nextContent));
252 + var updates = [makeTextContent(nextContent)];
253 processQueue(this, updates);
254 },
255
@@ -246,13 +263,12 @@ var ReactMultiChild = {
263 var prevChildren = this._renderedChildren;
264 // Remove any rendered children.
265 ReactChildReconciler.unmountChildren(prevChildren);
249 - var updates = [];
266 for (var name in prevChildren) {
267 if (prevChildren.hasOwnProperty(name)) {
252 - updates.push(this._unmountChild(prevChildren[name]));
268 + invariant(false, 'updateTextContent called on non-empty component.');
269 }
270 }
255 - updates.push(makeSetMarkup(nextMarkup));
271 + var updates = [makeSetMarkup(nextMarkup)];
272 processQueue(this, updates);
273 },
274
@@ -276,8 +292,13 @@ var ReactMultiChild = {
292 */
293 _updateChildren: function(nextNestedChildrenElements, transaction, context) {
294 var prevChildren = this._renderedChildren;
295 + var removedNodes = {};
296 var nextChildren = this._reconcilerUpdateChildren(
280 - prevChildren, nextNestedChildrenElements, transaction, context
297 + prevChildren,
298 + nextNestedChildrenElements,
299 + removedNodes,
300 + transaction,
301 + context
302 );
303 if (!nextChildren && !prevChildren) {
304 return;
@@ -288,6 +309,7 @@ var ReactMultiChild = {
309 // `lastIndex` will be the last index visited in `prevChildren`.
310 var lastIndex = 0;
311 var nextIndex = 0;
312 + var lastPlacedNode = null;
313 for (name in nextChildren) {
314 if (!nextChildren.hasOwnProperty(name)) {
315 continue;
@@ -297,7 +319,7 @@ var ReactMultiChild = {
319 if (prevChild === nextChild) {
320 updates = enqueue(
321 updates,
300 - this.moveChild(prevChild, nextIndex, lastIndex)
322 + this.moveChild(prevChild, lastPlacedNode, nextIndex, lastIndex)
323 );
324 lastIndex = Math.max(prevChild._mountIndex, lastIndex);
325 prevChild._mountIndex = nextIndex;
@@ -305,23 +327,29 @@ var ReactMultiChild = {
327 if (prevChild) {
328 // Update `lastIndex` before `_mountIndex` gets unset by unmounting.
329 lastIndex = Math.max(prevChild._mountIndex, lastIndex);
308 - updates = enqueue(updates, this._unmountChild(prevChild));
330 + // The `removedNodes` loop below will actually remove the child.
331 }
332 // The child must be instantiated before it's mounted.
333 updates = enqueue(
334 updates,
313 - this._mountChildAtIndex(nextChild, nextIndex, transaction, context)
335 + this._mountChildAtIndex(
336 + nextChild,
337 + lastPlacedNode,
338 + nextIndex,
339 + transaction,
340 + context
341 + )
342 );
343 }
344 nextIndex++;
345 + lastPlacedNode = ReactReconciler.getNativeNode(nextChild);
346 }
347 // Remove children that are no longer present.
319 - for (name in prevChildren) {
320 - if (prevChildren.hasOwnProperty(name) &&
321 - !(nextChildren && nextChildren.hasOwnProperty(name))) {
348 + for (name in removedNodes) {
349 + if (removedNodes.hasOwnProperty(name)) {
350 updates = enqueue(
351 updates,
324 - this._unmountChild(prevChildren[name])
352 + this._unmountChild(prevChildren[name], removedNodes[name])
353 );
354 }
355 }
@@ -334,7 +362,8 @@ var ReactMultiChild = {
362
363 /**
364 * Unmounts all rendered children. This should be used to clean up children
337 - * when this component is unmounted.
365 + * when this component is unmounted. It does not actually perform any
366 + * backend operations.
367 *
368 * @internal
369 */
@@ -360,12 +389,12 @@ var ReactMultiChild = {
389 * @param {number} lastIndex Last index visited of the siblings of `child`.
390 * @protected
391 */
363 - moveChild: function(child, toIndex, lastIndex) {
392 + moveChild: function(child, afterNode, toIndex, lastIndex) {
393 // If the index of `child` is less than `lastIndex`, then it needs to
394 // be moved. Otherwise, we do not need to move it because a child will be
395 // inserted or moved before `child`.
396 if (child._mountIndex < lastIndex) {
368 - return makeMove(child._mountIndex, toIndex);
397 + return makeMove(child, afterNode, toIndex);
398 }
399 },
400
@@ -376,8 +405,8 @@ var ReactMultiChild = {
405 * @param {string} mountImage Markup to insert.
406 * @protected
407 */
379 - createChild: function(child, mountImage) {
380 - return makeInsertMarkup(mountImage, child._mountIndex);
408 + createChild: function(child, afterNode, mountImage) {
409 + return makeInsertMarkup(mountImage, afterNode, child._mountIndex);
410 },
411
412 /**
@@ -386,8 +415,8 @@ var ReactMultiChild = {
415 * @param {ReactComponent} child Child to remove.
416 * @protected
417 */
389 - removeChild: function(child) {
390 - return makeRemove(child._mountIndex);
418 + removeChild: function(child, node) {
419 + return makeRemove(child, node);
420 },
421
422 /**
@@ -403,6 +432,7 @@ var ReactMultiChild = {
432 */
433 _mountChildAtIndex: function(
434 child,
435 + afterNode,
436 index,
437 transaction,
438 context) {
@@ -414,7 +444,7 @@ var ReactMultiChild = {
444 context
445 );
446 child._mountIndex = index;
417 - return this.createChild(child, mountImage);
447 + return this.createChild(child, afterNode, mountImage);
448 },
449
450 /**
@@ -425,8 +455,8 @@ var ReactMultiChild = {
455 * @param {ReactComponent} child Component to unmount.
456 * @private
457 */
428 - _unmountChild: function(child) {
429 - var update = this.removeChild(child);
458 + _unmountChild: function(child, node) {
459 + var update = this.removeChild(child, node);
460 child._mountIndex = null;
461 return update;
462 },