Skip to content

Commit

Permalink
Move nodes around by reference instead of by index
Browse files Browse the repository at this point in the history 
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 (facebook#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.)
  • Loading branch information
@sophiebits
sophiebits committed Jan 5, 2016
1 parent 0ebc7b6 commit 2792657
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 126 deletions.
102 changes: 16 additions & 86 deletions src/renderers/dom/client/utils/DOMChildrenOperations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ var ReactPerf = require('ReactPerf');

var setInnerHTML = require('setInnerHTML');
var setTextContent = require('setTextContent');
var invariant = require('invariant');

function getNodeAfter(parentNode, node) {
return node ? node.nextSibling : parentNode.firstChild;
}

/**
* Inserts `childNode` as a child of `parentNode` at the `index`.
Expand All @@ -28,27 +31,14 @@ var invariant = require('invariant');
* @param {number} index Index at which to insert the child.
* @internal
*/
function insertChildAt(parentNode, childNode, index) {
// We can rely exclusively on `insertBefore(node, null)` instead of also using
function insertChildAt(parentNode, childNode, referenceNode) {
// We rely exclusively on `insertBefore(node, null)` instead of also using
// `appendChild(node)`. (Using `undefined` is not allowed by all browsers so
// we are careful to use `null`.)

// In Safari, .childNodes[index] can return a DOM node with id={index} so we
// use .item() instead which is immune to this bug. (See #3560.) In contrast
// to the spec, IE8 throws an error if index is larger than the list size.
var referenceNode =
index < parentNode.childNodes.length ?
parentNode.childNodes.item(index) : null;

parentNode.insertBefore(childNode, referenceNode);
}

function insertLazyTreeChildAt(parentNode, childTree, index) {
// See above.
var referenceNode =
index < parentNode.childNodes.length ?
parentNode.childNodes.item(index) : null;

function insertLazyTreeChildAt(parentNode, childTree, referenceNode) {
DOMLazyTree.insertTreeBefore(parentNode, childTree, referenceNode);
}

Expand All @@ -69,81 +59,21 @@ var DOMChildrenOperations = {
* @internal
*/
processUpdates: function(parentNode, updates) {
var update;
// Mapping from parent IDs to initial child orderings.
var initialChildren = null;
// List of children that will be moved or removed.
var updatedChildren = null;

var markupList = null;

for (var i = 0; i < updates.length; i++) {
update = updates[i];
if (update.type === ReactMultiChildUpdateTypes.MOVE_EXISTING ||
update.type === ReactMultiChildUpdateTypes.REMOVE_NODE) {
var updatedIndex = update.fromIndex;
var updatedChild = parentNode.childNodes[updatedIndex];

invariant(
updatedChild,
'processUpdates(): Unable to find child %s of element %s. This ' +
'probably means the DOM was unexpectedly mutated (e.g., by the ' +
'browser), usually due to forgetting a <tbody> when using tables, ' +
'nesting tags like <form>, <p>, or <a>, or using non-SVG elements ' +
'in an <svg> parent.',
updatedIndex,
parentNode,
);

initialChildren = initialChildren || {};
initialChildren[updatedIndex] = updatedChild;

updatedChildren = updatedChildren || [];
updatedChildren.push(updatedChild);
} else if (update.type === ReactMultiChildUpdateTypes.INSERT_MARKUP) {
// Replace each HTML string with an index into the markup list
if (typeof update.content === 'string') {
markupList = markupList || [];
update.content = markupList.push(update.markup);
}
}
}

var renderedMarkup;
if (markupList) {
renderedMarkup = Danger.dangerouslyRenderMarkup(markupList);
}

// Remove updated children first so that `toIndex` is consistent.
if (updatedChildren) {
for (var j = 0; j < updatedChildren.length; j++) {
parentNode.removeChild(updatedChildren[j]);
}
}

for (var k = 0; k < updates.length; k++) {
update = updates[k];
var update = updates[k];
switch (update.type) {
case ReactMultiChildUpdateTypes.INSERT_MARKUP:
if (renderedMarkup) {
insertChildAt(
parentNode,
renderedMarkup[update.content],
update.toIndex
);
} else {
insertLazyTreeChildAt(
parentNode,
update.content,
update.toIndex
);
}
insertLazyTreeChildAt(
parentNode,
update.content,
getNodeAfter(parentNode, update.afterNode)
);
break;
case ReactMultiChildUpdateTypes.MOVE_EXISTING:
insertChildAt(
parentNode,
initialChildren[update.fromIndex],
update.toIndex
update.fromNode,
getNodeAfter(parentNode, update.afterNode)
);
break;
case ReactMultiChildUpdateTypes.SET_MARKUP:
Expand All @@ -159,7 +89,7 @@ var DOMChildrenOperations = {
);
break;
case ReactMultiChildUpdateTypes.REMOVE_NODE:
// Already removed by the for-loop above.
parentNode.removeChild(update.fromNode);
break;
}
}
Expand Down
14 changes: 9 additions & 5 deletions src/renderers/shared/reconciler/ReactChildReconciler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ var ReactChildReconciler = {
updateChildren: function(
prevChildren,
nextChildren,
removedNodes,
transaction,
context) {
// We currently don't have a way to track moves here but if we use iterators
Expand All @@ -79,14 +80,15 @@ var ReactChildReconciler = {
// TODO: If nothing has changed, return the prevChildren object so that we
// can quickly bailout if nothing has changed.
if (!nextChildren && !prevChildren) {
return null;
return;
}
var name;
var prevChild;
for (name in nextChildren) {
if (!nextChildren.hasOwnProperty(name)) {
continue;
}
var prevChild = prevChildren && prevChildren[name];
prevChild = prevChildren && prevChildren[name];
var prevElement = prevChild && prevChild._currentElement;
var nextElement = nextChildren[name];
if (prevChild != null &&
Expand All @@ -97,7 +99,8 @@ var ReactChildReconciler = {
nextChildren[name] = prevChild;
} else {
if (prevChild) {
ReactReconciler.unmountComponent(prevChild, name);
removedNodes[name] = ReactReconciler.getNativeNode(prevChild);
ReactReconciler.unmountComponent(prevChild);
}
// The child must be instantiated before it's mounted.
var nextChildInstance = instantiateReactComponent(nextElement);
Expand All @@ -108,10 +111,11 @@ var ReactChildReconciler = {
for (name in prevChildren) {
if (prevChildren.hasOwnProperty(name) &&
!(nextChildren && nextChildren.hasOwnProperty(name))) {
ReactReconciler.unmountComponent(prevChildren[name]);
prevChild = prevChildren[name];
removedNodes[name] = ReactReconciler.getNativeNode(prevChild);
ReactReconciler.unmountComponent(prevChild);
}
}
return nextChildren;
},

/**
Expand Down
Loading

0 comments on commit 2792657

Please sign in to comment.