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

Don't wrap text in <span> elements

Author: sophiebits

src/renderers/dom/client/ReactDOMComponentTree.js

@@ -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;
       }

src/renderers/dom/client/__tests__/ReactDOMComponentTree-test.js

@@ -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');

src/renderers/dom/client/utils/DOMChildrenOperations.js

@@ -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;

src/renderers/dom/client/validateDOMNesting.js

@@ -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,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
         );

src/renderers/dom/server/__tests__/ReactServerRendering-test.js

@@ -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(

src/renderers/dom/shared/ReactDOMTextComponent.js

@@ -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.
  *
@@ -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);
   },
 

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js

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