Merge pull request #5753 from mwiencek/no-text-span-2

Don't wrap text in <span> elements
This commit is contained in:
Ben Alpert
2016-02-18 12:02:17 -08:00
11 changed files with 295 additions and 53 deletions
@@ -87,10 +87,12 @@ function precacheChildNodes(inst, node) {
}
// We assume the child nodes are in the same order as the child instances.
for (; childNode !== null; childNode = childNode.nextSibling) {
if (childNode.nodeType === 1 &&
childNode.getAttribute(ATTR_NAME) === String(childID) ||
childNode.nodeType === 8 &&
childNode.nodeValue === ' react-empty: ' + childID + ' ') {
if ((childNode.nodeType === 1 &&
childNode.getAttribute(ATTR_NAME) === String(childID)) ||
(childNode.nodeType === 8 &&
childNode.nodeValue === ' react-text: ' + childID + ' ') ||
(childNode.nodeType === 8 &&
childNode.nodeValue === ' react-empty: ' + childID + ' ')) {
precacheNode(childInst, childNode);
continue outer;
}
@@ -100,7 +100,9 @@ describe('ReactDOMComponentTree', function() {
expect(renderAndGetInstance('main')._currentElement.type).toBe('main');
// This one's a text component!
expect(renderAndGetInstance('span')._stringText).toBe('goodbye.');
var root = renderAndQuery(null);
var inst = ReactDOMComponentTree.getInstanceFromNode(root.children[0].childNodes[2]);
expect(inst._stringText).toBe('goodbye.');
expect(renderAndGetClosest('b')._currentElement.type).toBe('main');
expect(renderAndGetClosest('img')._currentElement.type).toBe('main');
@@ -21,6 +21,11 @@ var setInnerHTML = require('setInnerHTML');
var setTextContent = require('setTextContent');
function getNodeAfter(parentNode, node) {
// Special case for text components, which return [open, close] comments
// from getNativeNode.
if (Array.isArray(node)) {
node = node[1];
}
return node ? node.nextSibling : parentNode.firstChild;
}
@@ -45,6 +50,78 @@ function insertLazyTreeChildAt(parentNode, childTree, referenceNode) {
DOMLazyTree.insertTreeBefore(parentNode, childTree, referenceNode);
}
function moveChild(parentNode, childNode, referenceNode) {
if (Array.isArray(childNode)) {
moveDelimitedText(parentNode, childNode[0], childNode[1], referenceNode);
} else {
insertChildAt(parentNode, childNode, referenceNode);
}
}
function removeChild(parentNode, childNode) {
if (Array.isArray(childNode)) {
var closingComment = childNode[1];
childNode = childNode[0];
removeDelimitedText(parentNode, childNode, closingComment);
parentNode.removeChild(closingComment);
}
parentNode.removeChild(childNode);
}
function moveDelimitedText(
parentNode,
openingComment,
closingComment,
referenceNode
) {
var node = openingComment;
while (true) {
var nextNode = node.nextSibling;
insertChildAt(parentNode, node, referenceNode);
if (node === closingComment) {
break;
}
node = nextNode;
}
}
function removeDelimitedText(parentNode, startNode, closingComment) {
while (true) {
var node = startNode.nextSibling;
if (node === closingComment) {
// The closing comment is removed by ReactMultiChild.
break;
} else {
parentNode.removeChild(node);
}
}
}
function replaceDelimitedText(openingComment, closingComment, stringText) {
var parentNode = openingComment.parentNode;
var nodeAfterComment = openingComment.nextSibling;
if (nodeAfterComment === closingComment) {
// There are no text nodes between the opening and closing comments; insert
// a new one if stringText isn't empty.
if (stringText) {
insertChildAt(
parentNode,
document.createTextNode(stringText),
nodeAfterComment
);
}
} else {
if (stringText) {
// Set the text content of the first node after the opening comment, and
// remove all following nodes up until the closing comment.
setTextContent(nodeAfterComment, stringText);
removeDelimitedText(parentNode, nodeAfterComment, closingComment);
} else {
removeDelimitedText(parentNode, openingComment, closingComment);
}
}
}
/**
* Operations for updating with DOM children.
*/
@@ -54,6 +131,8 @@ var DOMChildrenOperations = {
updateTextContent: setTextContent,
replaceDelimitedText: replaceDelimitedText,
/**
* Updates a component's children by processing a series of updates. The
* update configurations are each expected to have a `parentNode` property.
@@ -73,7 +152,7 @@ var DOMChildrenOperations = {
);
break;
case ReactMultiChildUpdateTypes.MOVE_EXISTING:
insertChildAt(
moveChild(
parentNode,
update.fromNode,
getNodeAfter(parentNode, update.afterNode)
@@ -92,7 +171,7 @@ var DOMChildrenOperations = {
);
break;
case ReactMultiChildUpdateTypes.REMOVE_NODE:
parentNode.removeChild(update.fromNode);
removeChild(parentNode, update.fromNode);
break;
}
}
@@ -102,6 +181,7 @@ var DOMChildrenOperations = {
ReactPerf.measureMethods(DOMChildrenOperations, 'DOMChildrenOperations', {
updateTextContent: 'updateTextContent',
replaceDelimitedText: 'replaceDelimitedText',
});
module.exports = DOMChildrenOperations;
@@ -384,6 +384,11 @@ if (__DEV__) {
}
didWarn[warnKey] = true;
var tagDisplayName = childTag;
if (childTag !== '#text') {
tagDisplayName = '<' + childTag + '>';
}
if (invalidParent) {
var info = '';
if (ancestorTag === 'table' && childTag === 'tr') {
@@ -393,9 +398,9 @@ if (__DEV__) {
}
warning(
false,
'validateDOMNesting(...): <%s> cannot appear as a child of <%s>. ' +
'validateDOMNesting(...): %s cannot appear as a child of <%s>. ' +
'See %s.%s',
childTag,
tagDisplayName,
ancestorTag,
ownerInfo,
info
@@ -403,9 +408,9 @@ if (__DEV__) {
} else {
warning(
false,
'validateDOMNesting(...): <%s> cannot appear as a descendant of ' +
'validateDOMNesting(...): %s cannot appear as a descendant of ' +
'<%s>. See %s.',
childTag,
tagDisplayName,
ancestorTag,
ownerInfo
);
@@ -103,8 +103,8 @@ describe('ReactServerRendering', function() {
ID_ATTRIBUTE_NAME + '="[^"]+" ' +
ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+">' +
'<span ' + ID_ATTRIBUTE_NAME + '="[^"]+">' +
'<span ' + ID_ATTRIBUTE_NAME + '="[^"]+">My name is </span>' +
'<span ' + ID_ATTRIBUTE_NAME + '="[^"]+">child</span>' +
'<!-- react-text: [0-9]+ -->My name is <!-- /react-text -->' +
'<!-- react-text: [0-9]+ -->child<!-- /react-text -->' +
'</span>' +
'</div>'
);
@@ -153,8 +153,8 @@ describe('ReactServerRendering', function() {
'<span ' + ROOT_ATTRIBUTE_NAME + '="" ' +
ID_ATTRIBUTE_NAME + '="[^"]+" ' +
ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+">' +
'<span ' + ID_ATTRIBUTE_NAME + '="[^"]+">Component name: </span>' +
'<span ' + ID_ATTRIBUTE_NAME + '="[^"]+">TestComponent</span>' +
'<!-- react-text: [0-9]+ -->Component name: <!-- /react-text -->' +
'<!-- react-text: [0-9]+ -->TestComponent<!-- /react-text -->' +
'</span>'
);
expect(lifecycle).toEqual(
@@ -13,7 +13,6 @@
var DOMChildrenOperations = require('DOMChildrenOperations');
var DOMLazyTree = require('DOMLazyTree');
var DOMPropertyOperations = require('DOMPropertyOperations');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
var ReactPerf = require('ReactPerf');
@@ -21,16 +20,14 @@ var assign = require('Object.assign');
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
var validateDOMNesting = require('validateDOMNesting');
var getNode = ReactDOMComponentTree.getNodeFromInstance;
/**
* Text nodes violate a couple assumptions that React makes about components:
*
* - When mounting text into the DOM, adjacent text nodes are merged.
* - Text nodes cannot be assigned a React root ID.
*
* This component is used to wrap strings in elements so that they can undergo
* the same reconciliation that is applied to elements.
* This component is used to wrap strings between comment nodes so that they
* can undergo the same reconciliation that is applied to elements.
*
* TODO: Investigate representing React components in the DOM with text nodes.
*
@@ -49,6 +46,8 @@ var ReactDOMTextComponent = function(text) {
// Properties
this._domID = null;
this._mountIndex = 0;
this._openingComment = null;
this._commentNodes = null;
};
assign(ReactDOMTextComponent.prototype, {
@@ -77,34 +76,44 @@ assign(ReactDOMTextComponent.prototype, {
if (parentInfo) {
// parentInfo should always be present except for the top-level
// component when server rendering
validateDOMNesting('span', this, parentInfo);
validateDOMNesting('#text', this, parentInfo);
}
}
var domID = nativeContainerInfo._idCounter++;
var openingValue = ' react-text: ' + domID + ' ';
var closingValue = ' /react-text ';
this._domID = domID;
this._nativeParent = nativeParent;
if (transaction.useCreateElement) {
var ownerDocument = nativeContainerInfo._ownerDocument;
var el = ownerDocument.createElement('span');
ReactDOMComponentTree.precacheNode(this, el);
var lazyTree = DOMLazyTree(el);
DOMLazyTree.queueText(lazyTree, this._stringText);
var openingComment = ownerDocument.createComment(openingValue);
var closingComment = ownerDocument.createComment(closingValue);
var lazyTree = DOMLazyTree(ownerDocument.createDocumentFragment());
DOMLazyTree.queueChild(lazyTree, DOMLazyTree(openingComment));
if (this._stringText) {
DOMLazyTree.queueChild(
lazyTree,
DOMLazyTree(ownerDocument.createTextNode(this._stringText))
);
}
DOMLazyTree.queueChild(lazyTree, DOMLazyTree(closingComment));
this._openingComment = openingComment;
ReactDOMComponentTree.precacheNode(this, closingComment);
return lazyTree;
} else {
var escapedText = escapeTextContentForBrowser(this._stringText);
if (transaction.renderToStaticMarkup) {
// Normally we'd wrap this in a `span` for the reasons stated above, but
// since this is a situation where React won't take over (static pages),
// we can simply return the text as it is.
// Normally we'd wrap this between comment nodes for the reasons stated
// above, but since this is a situation where React won't take over
// (static pages), we can simply return the text as it is.
return escapedText;
}
return (
'<span ' + DOMPropertyOperations.createMarkupForID(domID) + '>' +
escapedText +
'</span>'
'<!--' + openingValue + '-->' + escapedText +
'<!--' + closingValue + '-->'
);
}
},
@@ -125,16 +134,29 @@ assign(ReactDOMTextComponent.prototype, {
// and/or updateComponent to do the actual update for consistency with
// other component types?
this._stringText = nextStringText;
DOMChildrenOperations.updateTextContent(getNode(this), nextStringText);
var commentNodes = this.getNativeNode();
DOMChildrenOperations.replaceDelimitedText(
commentNodes[0],
commentNodes[1],
nextStringText
);
}
}
},
getNativeNode: function() {
return getNode(this);
var nativeNode = this._commentNodes;
if (nativeNode) {
return nativeNode;
}
nativeNode = [this._openingComment, this._nativeNode];
this._commentNodes = nativeNode;
return nativeNode;
},
unmountComponent: function() {
this._openingComment = null;
this._commentNodes = null;
ReactDOMComponentTree.uncacheNode(this);
},
@@ -1260,8 +1260,8 @@ describe('ReactDOMComponent', function() {
'match the DOM tree generated by the browser.'
);
expect(console.error.argsForCall[1][0]).toBe(
'Warning: validateDOMNesting(...): <span> cannot appear as a child ' +
'of <table>. See Foo > table > span.'
'Warning: validateDOMNesting(...): #text cannot appear as a child ' +
'of <table>. See Foo > table > #text.'
);
});
@@ -13,26 +13,96 @@
var React;
var ReactDOM;
var ReactDOMServer;
describe('ReactDOMTextComponent', function() {
beforeEach(function() {
React = require('React');
ReactDOM = require('ReactDOM');
ReactDOMServer = require('ReactDOMServer');
});
it('updates a mounted text component in place', function() {
var el = document.createElement('div');
var inst = ReactDOM.render(<div>{'foo'}{'bar'}</div>, el);
var inst = ReactDOM.render(<div><span />{'foo'}{'bar'}</div>, el);
var foo = ReactDOM.findDOMNode(inst).children[0];
var bar = ReactDOM.findDOMNode(inst).children[1];
expect(foo.tagName).toBe('SPAN');
expect(bar.tagName).toBe('SPAN');
var foo = ReactDOM.findDOMNode(inst).childNodes[2];
var bar = ReactDOM.findDOMNode(inst).childNodes[5];
expect(foo.data).toBe('foo');
expect(bar.data).toBe('bar');
inst = ReactDOM.render(<div>{'baz'}{'qux'}</div>, el);
// After the update, the spans should have stayed in place (as opposed to
// getting unmounted and remounted)
expect(ReactDOM.findDOMNode(inst).children[0]).toBe(foo);
expect(ReactDOM.findDOMNode(inst).children[1]).toBe(bar);
inst = ReactDOM.render(<div><span />{'baz'}{'qux'}</div>, el);
// After the update, the text nodes should have stayed in place (as opposed
// to getting unmounted and remounted)
expect(ReactDOM.findDOMNode(inst).childNodes[2]).toBe(foo);
expect(ReactDOM.findDOMNode(inst).childNodes[5]).toBe(bar);
expect(foo.data).toBe('baz');
expect(bar.data).toBe('qux');
});
it('can be toggled in and out of the markup', function() {
var el = document.createElement('div');
var inst = ReactDOM.render(<div>{'foo'}<div />{'bar'}</div>, el);
var container = ReactDOM.findDOMNode(inst);
var childDiv = container.childNodes[3];
var childNodes;
inst = ReactDOM.render(<div>{null}<div />{null}</div>, el);
container = ReactDOM.findDOMNode(inst);
childNodes = container.childNodes;
expect(childNodes.length).toBe(1);
expect(childNodes[0]).toBe(childDiv);
inst = ReactDOM.render(<div>{'foo'}<div />{'bar'}</div>, el);
container = ReactDOM.findDOMNode(inst);
childNodes = container.childNodes;
expect(childNodes.length).toBe(7);
expect(childNodes[1].data).toBe('foo');
expect(childNodes[3]).toBe(childDiv);
expect(childNodes[5].data).toBe('bar');
});
it('can reconcile text merged by Node.normalize()', function() {
var el = document.createElement('div');
var inst = ReactDOM.render(<div>{'foo'}{'bar'}{'baz'}</div>, el);
var container = ReactDOM.findDOMNode(inst);
container.normalize();
inst = ReactDOM.render(<div>{'bar'}{'baz'}{'qux'}</div>, el);
container = ReactDOM.findDOMNode(inst);
expect(container.textContent).toBe('barbazqux');
});
it('can reconcile text from pre-rendered markup', function() {
var el = document.createElement('div');
var reactEl = <div>{'foo'}{'bar'}{'baz'}</div>;
el.innerHTML = ReactDOMServer.renderToString(reactEl);
ReactDOM.render(reactEl, el);
expect(el.textContent).toBe('foobarbaz');
reactEl = <div>{''}{''}{''}</div>;
el.innerHTML = ReactDOMServer.renderToString(reactEl);
ReactDOM.render(reactEl, el);
expect(el.textContent).toBe('');
});
it('can reconcile text arbitrarily split into multiple nodes', function() {
var el = document.createElement('div');
var inst = ReactDOM.render(<div><span />{'foobarbaz'}</div>, el);
var container = ReactDOM.findDOMNode(inst);
var childNodes = container.childNodes;
var textNode = childNodes[2];
textNode.textContent = 'foo';
container.insertBefore(document.createTextNode('bar'), childNodes[3]);
container.insertBefore(document.createTextNode('baz'), childNodes[3]);
inst = ReactDOM.render(<div><span />{'barbazqux'}</div>, el);
container = ReactDOM.findDOMNode(inst);
expect(container.textContent).toBe('barbazqux');
});
});
@@ -49,24 +49,40 @@ var expectChildren = function(d, children) {
expect(textNode.data).toBe('' + children);
}
} else {
expect(outerNode.childNodes.length).toBe(children.length);
var openingCommentNode;
var closingCommentNode;
var mountIndex = 0;
for (var i = 0; i < children.length; i++) {
var child = children[i];
if (typeof child === 'string') {
textNode = outerNode.childNodes[i].firstChild;
openingCommentNode = outerNode.childNodes[mountIndex];
expect(openingCommentNode.nodeType).toBe(8);
expect(openingCommentNode.nodeValue).toMatch(' react-text: [0-9]+ ');
if (child === '') {
expect(textNode).toBe(null);
textNode = null;
closingCommentNode = openingCommentNode.nextSibling;
mountIndex += 2;
} else {
expect(textNode).not.toBe(null);
textNode = openingCommentNode.nextSibling;
closingCommentNode = textNode.nextSibling;
mountIndex += 3;
}
if (textNode) {
expect(textNode.nodeType).toBe(3);
expect(textNode.data).toBe('' + child);
}
expect(closingCommentNode.nodeType).toBe(8);
expect(closingCommentNode.nodeValue).toBe(' /react-text ');
} else {
var elementDOMNode = outerNode.childNodes[i];
var elementDOMNode = outerNode.childNodes[mountIndex];
expect(elementDOMNode.tagName).toBe('DIV');
mountIndex++;
}
}
}
@@ -194,4 +210,38 @@ describe('ReactMultiChildText', function() {
ReactTestUtils.renderIntoDocument(<div><h1>{['A', 'B']}</h1></div>);
}).not.toThrow();
});
it('should reorder keyed text nodes', function() {
spyOn(console, 'error');
var container = document.createElement('div');
ReactDOM.render(
<div>{new Map([['a', 'alpha'], ['b', 'beta']])}</div>,
container
);
var childNodes = container.firstChild.childNodes;
var alpha1 = childNodes[0];
var alpha2 = childNodes[1];
var alpha3 = childNodes[2];
var beta1 = childNodes[3];
var beta2 = childNodes[4];
var beta3 = childNodes[5];
ReactDOM.render(
<div>{new Map([['b', 'beta'], ['a', 'alpha']])}</div>,
container
);
childNodes = container.firstChild.childNodes;
expect(childNodes[0]).toBe(beta1);
expect(childNodes[1]).toBe(beta2);
expect(childNodes[2]).toBe(beta3);
expect(childNodes[3]).toBe(alpha1);
expect(childNodes[4]).toBe(alpha2);
expect(childNodes[5]).toBe(alpha3);
// Using Maps as children gives a single warning
expect(console.error.calls.length).toBe(1);
});
});
+13 -3
View File
@@ -27,7 +27,7 @@ function addValue(obj, key, val) {
obj[key] = (obj[key] || 0) + val;
}
// Composites don't have any built-in ID: we have to make our own
// Composite/text components don't have any built-in ID: we have to make our own
var compositeIDMap;
var compositeIDCounter = 17000;
function getIDOfComposite(inst) {
@@ -43,6 +43,14 @@ function getIDOfComposite(inst) {
}
}
function getID(inst) {
if (inst.hasOwnProperty('_rootNodeID')) {
return inst._rootNodeID;
} else {
return getIDOfComposite(inst);
}
}
var ReactDefaultPerf = {
_allMeasurements: [], // last item in the list is the current one
_mountStack: [0],
@@ -224,8 +232,10 @@ var ReactDefaultPerf = {
} else if (fnName === 'replaceNodeWithMarkup') {
// Old node is already unmounted; can't get its instance
id = ReactDOMComponentTree.getInstanceFromNode(args[1].node)._rootNodeID;
} else if (fnName === 'replaceDelimitedText') {
id = getID(ReactDOMComponentTree.getInstanceFromNode(args[1]));
} else if (typeof id === 'object') {
id = ReactDOMComponentTree.getInstanceFromNode(args[0])._rootNodeID;
id = getID(ReactDOMComponentTree.getInstanceFromNode(args[0]));
}
ReactDefaultPerf._recordWrite(
id,
@@ -291,7 +301,7 @@ var ReactDefaultPerf = {
fnName === 'receiveComponent')) {
rv = func.apply(this, args);
entry.hierarchy[this._rootNodeID] =
entry.hierarchy[getID(this)] =
ReactDefaultPerf._compositeStack.slice();
return rv;
} else {
+1
View File
@@ -27,6 +27,7 @@ var DOM_OPERATION_TYPES = {
'deleteValueForProperty': 'remove attribute',
'setValueForStyles': 'update styles',
'replaceNodeWithMarkup': 'replace',
'replaceDelimitedText': 'replace',
'updateTextContent': 'set textContent',
};