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 Jan 12, 2016
1 parent a158405 commit 35b5dbe
Show file tree
Hide file tree
Showing 11 changed files with 218 additions and 49 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
47 changes: 47 additions & 0 deletions src/renderers/dom/client/utils/DOMChildrenOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
var DOMLazyTree = require('DOMLazyTree');
var Danger = require('Danger');
var ReactMultiChildUpdateTypes = require('ReactMultiChildUpdateTypes');
var ReactNativeComponent = require('ReactNativeComponent');
var ReactPerf = require('ReactPerf');

var setInnerHTML = require('setInnerHTML');
Expand Down Expand Up @@ -42,6 +43,47 @@ function insertLazyTreeChildAt(parentNode, childTree, referenceNode) {
DOMLazyTree.insertTreeBefore(parentNode, childTree, referenceNode);
}

function isClosingTextComment(node) {
return node.nodeType === 8 && node.nodeValue === ' /react-text ';
}

function removeDelimitedText(startNode) {
var parentNode = startNode.parentNode;
while (true) {
var node = startNode.nextSibling;
if (isClosingTextComment(node)) {
return node;
} else {
parentNode.removeChild(node);
}
}
}

function replaceDelimitedText(openingComment, stringText) {
var parentNode = openingComment.parentNode;
var nodeAfterComment = openingComment.nextSibling;
if (isClosingTextComment(nodeAfterComment)) {
// 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(nodeAfterComment);
} else {
removeDelimitedText(openingComment);
}
}
}

/**
* Operations for updating with DOM children.
*/
Expand All @@ -51,6 +93,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 Down Expand Up @@ -89,6 +133,9 @@ var DOMChildrenOperations = {
);
break;
case ReactMultiChildUpdateTypes.REMOVE_NODE:
if (ReactNativeComponent.isTextComponent(update.child)) {
parentNode.removeChild(removeDelimitedText(update.fromNode));
}
parentNode.removeChild(update.fromNode);
break;
}
Expand Down
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
47 changes: 31 additions & 16 deletions src/renderers/dom/shared/ReactDOMTextComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

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

Expand All @@ -29,8 +28,8 @@ var getNode = ReactDOMComponentTree.getNodeFromInstance;
* - 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 Down Expand Up @@ -59,6 +58,7 @@ assign(ReactDOMTextComponent.prototype, {
// Properties
this._domID = null;
this._mountIndex = 0;
this._closingComment = null;
},

/**
Expand All @@ -85,34 +85,41 @@ 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);
return lazyTree;
var openingComment = ownerDocument.createComment(openingValue);
var closingComment = ownerDocument.createComment(closingValue);
this._closingComment = closingComment;
var fragment = ownerDocument.createDocumentFragment();
fragment.appendChild(openingComment);
if (this._stringText) {
fragment.appendChild(ownerDocument.createTextNode(this._stringText));
}
fragment.appendChild(closingComment);
ReactDOMComponentTree.precacheNode(this, openingComment);
return DOMLazyTree(fragment);
} 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 @@ -133,7 +140,10 @@ 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);
DOMChildrenOperations.replaceDelimitedText(
getNode(this),
nextStringText
);
}
}
},
Expand All @@ -142,8 +152,13 @@ assign(ReactDOMTextComponent.prototype, {
return getNode(this);
},

getLastNativeNode: function() {
return this._closingComment;
},

unmountComponent: function() {
ReactDOMComponentTree.uncacheNode(this);
this._closingComment = null;
},

});
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 @@ -1172,8 +1172,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
94 changes: 82 additions & 12 deletions src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 foo = ReactDOM.findDOMNode(inst).children[0];
var bar = ReactDOM.findDOMNode(inst).children[1];
expect(foo.tagName).toBe('SPAN');
expect(bar.tagName).toBe('SPAN');

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);
var inst = ReactDOM.render(<div><span />{'foo'}{'bar'}</div>, el);

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><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');
});
});
Loading

0 comments on commit 35b5dbe

Please sign in to comment.