Skip to content

Commit

Permalink
Don't wrap text in <span> elements
Browse files Browse the repository at this point in the history
Instead, use opening and closing comment nodes to delimit text data.
  • Loading branch information
mwiencek committed Feb 18, 2016
1 parent e8e56e8 commit 2038500
Show file tree
Hide file tree
Showing 11 changed files with 297 additions and 55 deletions.
10 changes: 6 additions & 4 deletions src/renderers/dom/client/ReactDOMComponentTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
84 changes: 82 additions & 2 deletions src/renderers/dom/client/utils/DOMChildrenOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,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;
}

Expand Down Expand Up @@ -51,6 +56,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.
*/
Expand All @@ -60,6 +137,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.
Expand All @@ -79,7 +158,7 @@ var DOMChildrenOperations = {
);
break;
case ReactMultiChildUpdateTypes.MOVE_EXISTING:
insertChildAt(
moveChild(
parentNode,
update.fromNode,
getNodeAfter(parentNode, update.afterNode)
Expand All @@ -98,7 +177,7 @@ var DOMChildrenOperations = {
);
break;
case ReactMultiChildUpdateTypes.REMOVE_NODE:
parentNode.removeChild(update.fromNode);
removeChild(parentNode, update.fromNode);
break;
}
}
Expand All @@ -108,6 +187,7 @@ var DOMChildrenOperations = {

ReactPerf.measureMethods(DOMChildrenOperations, 'DOMChildrenOperations', {
updateTextContent: 'updateTextContent',
replaceDelimitedText: 'replaceDelimitedText',
});

module.exports = DOMChildrenOperations;
13 changes: 9 additions & 4 deletions src/renderers/dom/client/validateDOMNesting.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand All @@ -393,19 +398,19 @@ 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
);
} 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
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>'
);
Expand Down Expand Up @@ -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(
Expand Down
58 changes: 40 additions & 18 deletions src/renderers/dom/shared/ReactDOMTextComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,21 @@

var DOMChildrenOperations = require('DOMChildrenOperations');
var DOMLazyTree = require('DOMLazyTree');
var DOMPropertyOperations = require('DOMPropertyOperations');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
var ReactPerf = require('ReactPerf');

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.
*
Expand All @@ -49,6 +46,8 @@ var ReactDOMTextComponent = function(text) {
// Properties
this._domID = null;
this._mountIndex = 0;
this._openingComment = null;
this._commentNodes = null;
};

assign(ReactDOMTextComponent.prototype, {
Expand Down Expand Up @@ -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 + '-->'
);
}
},
Expand All @@ -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);
},

Expand Down
4 changes: 2 additions & 2 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
);
});

Expand Down
Loading

0 comments on commit 2038500

Please sign in to comment.