From 30574ff805b95a01816a97c89971a55333e0fd05 Mon Sep 17 00:00:00 2001
From: Ben Alpert <spicyjalapeno@gmail.com>
Date: Fri, 18 Jul 2014 12:43:13 -0700
Subject: [PATCH] Use script tags to demarcate text components

tl;dr: `<script></script>A<script></script>B` instead of `<span>A</span><span>B</span>`.

Old:

![image](https://cloud.githubusercontent.com/assets/6820/3631177/af021294-0eb3-11e4-8b4a-d334cef48dea.png)

New:

![image](https://cloud.githubusercontent.com/assets/6820/3631172/a74a7866-0eb3-11e4-9591-1320ba0c326a.png)

This is nicer in that if you add styles on `span`, your text nodes now won't be affected. Similarly, the target of mouse events will now never be a ReactTextComponent span because the script elements aren't rendered at all.

Test Plan: jest, and some simple browser sanity checking.
---
 src/browser/ReactTextComponent.js             | 18 +++++------
 .../__tests__/ReactServerRendering-test.js    | 10 ++++---
 src/browser/ui/ReactDOMIDOperations.js        |  8 ++---
 src/browser/ui/dom/DOMChildrenOperations.js   | 26 +++++++++++++---
 src/browser/ui/dom/Danger.js                  | 13 +++++++-
 .../__tests__/ReactMultiChildText-test.js     | 30 ++++++++++++++-----
 6 files changed, 75 insertions(+), 30 deletions(-)

diff --git a/src/browser/ReactTextComponent.js b/src/browser/ReactTextComponent.js
index e23c720232180..58d1f64f82cae 100644
--- a/src/browser/ReactTextComponent.js
+++ b/src/browser/ReactTextComponent.js
@@ -33,8 +33,9 @@ var mixInto = require('mixInto');
  *  - 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 add a <script> tag before each text node so that
+ * they can undergo a reconciliation process similar to the one applied to
+ * elements.
  *
  * TODO: Investigate representing React components in the DOM with text nodes.
  *
@@ -71,16 +72,15 @@ mixInto(ReactTextComponent, {
     var escapedText = escapeTextForBrowser(this.props);
 
     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 add a preceding `script` 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(rootID) + '>' +
-        escapedText +
-      '</span>'
+      '<script ' + DOMPropertyOperations.createMarkupForID(rootID) +
+      '></script>' + escapedText
     );
   },
 
@@ -95,7 +95,7 @@ mixInto(ReactTextComponent, {
     var nextProps = nextComponent.props;
     if (nextProps !== this.props) {
       this.props = nextProps;
-      ReactComponent.BackendIDOperations.updateTextContentByID(
+      ReactComponent.BackendIDOperations.updateTextContentAfterByID(
         this._rootNodeID,
         nextProps
       );
diff --git a/src/browser/server/__tests__/ReactServerRendering-test.js b/src/browser/server/__tests__/ReactServerRendering-test.js
index 895ce51238e81..69b0968a4ae03 100644
--- a/src/browser/server/__tests__/ReactServerRendering-test.js
+++ b/src/browser/server/__tests__/ReactServerRendering-test.js
@@ -94,8 +94,8 @@ describe('ReactServerRendering', function() {
         '<div ' + 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>' +
+            '<script ' + ID_ATTRIBUTE_NAME + '="[^"]+"></script>My name is ' +
+            '<script ' + ID_ATTRIBUTE_NAME + '="[^"]+"></script>child' +
           '</span>' +
         '</div>'
       );
@@ -143,8 +143,10 @@ describe('ReactServerRendering', function() {
         expect(response).toMatch(
           '<span ' + ID_ATTRIBUTE_NAME + '="[^"]+" ' +
             ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+">' +
-            '<span ' + ID_ATTRIBUTE_NAME + '="[^"]+">Component name: </span>' +
-            '<span ' + ID_ATTRIBUTE_NAME + '="[^"]+">TestComponent</span>' +
+            '<script ' + ID_ATTRIBUTE_NAME + '="[^"]+"></script>' +
+            'Component name: ' +
+            '<script ' + ID_ATTRIBUTE_NAME + '="[^"]+"></script>' +
+            'TestComponent' +
           '</span>'
         );
         expect(lifecycle).toEqual(
diff --git a/src/browser/ui/ReactDOMIDOperations.js b/src/browser/ui/ReactDOMIDOperations.js
index ea206cd065b90..98456c4c47331 100644
--- a/src/browser/ui/ReactDOMIDOperations.js
+++ b/src/browser/ui/ReactDOMIDOperations.js
@@ -135,18 +135,18 @@ var ReactDOMIDOperations = {
   ),
 
   /**
-   * Updates a DOM node's text content set by `props.content`.
+   * Updates the text content after a DOM node.
    *
    * @param {string} id ID of the node to update.
    * @param {string} content Text content.
    * @internal
    */
-  updateTextContentByID: ReactPerf.measure(
+  updateTextContentAfterByID: ReactPerf.measure(
     'ReactDOMIDOperations',
-    'updateTextContentByID',
+    'updateTextContentAfterByID',
     function(id, content) {
       var node = ReactMount.getNode(id);
-      DOMChildrenOperations.updateTextContent(node, content);
+      DOMChildrenOperations.updateTextContentAfter(node, content);
     }
   ),
 
diff --git a/src/browser/ui/dom/DOMChildrenOperations.js b/src/browser/ui/dom/DOMChildrenOperations.js
index 0c53caf2c968f..46a42ebf6ea98 100644
--- a/src/browser/ui/dom/DOMChildrenOperations.js
+++ b/src/browser/ui/dom/DOMChildrenOperations.js
@@ -25,6 +25,8 @@ var ReactMultiChildUpdateTypes = require('ReactMultiChildUpdateTypes');
 var getTextContentAccessor = require('getTextContentAccessor');
 var invariant = require('invariant');
 
+var TEXT_NODE = 3;
+
 /**
  * The DOM property to use when setting text content.
  *
@@ -48,7 +50,7 @@ function insertChildAt(parentNode, childNode, index) {
   // browsers so we must replace it with `null`.
   parentNode.insertBefore(
     childNode,
-    parentNode.childNodes[index] || null
+    parentNode.children[index] || null
   );
 }
 
@@ -83,6 +85,16 @@ if (textContentAccessor === 'textContent') {
   };
 }
 
+function updateTextContentAfter(node, text) {
+  while (node.nextSibling && node.nextSibling.nodeType === TEXT_NODE) {
+    node.parentNode.removeChild(node.nextSibling);
+  }
+  if (text.length) {
+    var doc = node.ownerDocument || document;
+    node.parentNode.insertBefore(doc.createTextNode(text), node.nextSibling);
+  }
+}
+
 /**
  * Operations for updating with DOM children.
  */
@@ -90,7 +102,7 @@ var DOMChildrenOperations = {
 
   dangerouslyReplaceNodeWithMarkup: Danger.dangerouslyReplaceNodeWithMarkup,
 
-  updateTextContent: updateTextContent,
+  updateTextContentAfter: updateTextContentAfter,
 
   /**
    * Updates a component's children by processing a series of updates. The
@@ -111,7 +123,7 @@ var DOMChildrenOperations = {
       if (update.type === ReactMultiChildUpdateTypes.MOVE_EXISTING ||
           update.type === ReactMultiChildUpdateTypes.REMOVE_NODE) {
         var updatedIndex = update.fromIndex;
-        var updatedChild = update.parentNode.childNodes[updatedIndex];
+        var updatedChild = update.parentNode.children[updatedIndex];
         var parentID = update.parentID;
 
         invariant(
@@ -140,7 +152,13 @@ var DOMChildrenOperations = {
     // Remove updated children first so that `toIndex` is consistent.
     if (updatedChildren) {
       for (var j = 0; j < updatedChildren.length; j++) {
-        updatedChildren[j].parentNode.removeChild(updatedChildren[j]);
+        var nodeToRemove = updatedChildren[j];
+        // Remove trailing text nodes, probably from ReactTextComponent
+        while (nodeToRemove.nextSibling &&
+               nodeToRemove.nextSibling.nodeType === TEXT_NODE) {
+          nodeToRemove.parentNode.removeChild(nodeToRemove.nextSibling);
+        }
+        nodeToRemove.parentNode.removeChild(nodeToRemove);
       }
     }
 
diff --git a/src/browser/ui/dom/Danger.js b/src/browser/ui/dom/Danger.js
index 6e2dd071056e9..4b42f8579a993 100644
--- a/src/browser/ui/dom/Danger.js
+++ b/src/browser/ui/dom/Danger.js
@@ -30,6 +30,7 @@ var invariant = require('invariant');
 
 var OPEN_TAG_NAME_EXP = /^(<[^ \/>]+)/;
 var RESULT_INDEX_ATTR = 'data-danger-index';
+var TEXT_NODE = 3;
 
 /**
  * Extracts the `nodeName` from a string of markup.
@@ -54,7 +55,7 @@ var Danger = {
    * `markupList` should be the same.
    *
    * @param {array<string>} markupList List of markup strings to render.
-   * @return {array<DOMElement>} List of rendered nodes.
+   * @return {array<DOMElement|DOMDocumentFragment>} List of rendered nodes.
    * @internal
    */
   dangerouslyRenderMarkup: function(markupList) {
@@ -109,6 +110,7 @@ var Danger = {
         emptyFunction // Do nothing special with <script> tags.
       );
 
+      resultIndex = null;
       for (i = 0; i < renderNodes.length; ++i) {
         var renderNode = renderNodes[i];
         if (renderNode.hasAttribute &&
@@ -128,11 +130,20 @@ var Danger = {
           // we're done.
           resultListAssignmentCount += 1;
 
+        } else if (resultIndex != null && renderNode.nodeType === TEXT_NODE) {
+          // Text node, probably following a ReactTextComponent script --
+          // combine the two nodes into one document fragment
+          var fragment = document.createDocumentFragment();
+          fragment.appendChild(resultList[resultIndex]);
+          fragment.appendChild(renderNode);
+          resultList[resultIndex] = fragment;
+
         } else if (__DEV__) {
           console.error(
             "Danger: Discarding unexpected node:",
             renderNode
           );
+          resultIndex = null;
         }
       }
     }
diff --git a/src/core/__tests__/ReactMultiChildText-test.js b/src/core/__tests__/ReactMultiChildText-test.js
index 3ecef8ed7820f..c1ceae4a6c3ca 100644
--- a/src/core/__tests__/ReactMultiChildText-test.js
+++ b/src/core/__tests__/ReactMultiChildText-test.js
@@ -28,6 +28,21 @@ var ReactTestUtils = require('ReactTestUtils');
 
 var reactComponentExpect = require('reactComponentExpect');
 
+var TEXT_NODE = 3;
+
+var assertTextNodeAfter = function(node, text) {
+  expect(node.tagName).toBe('SCRIPT');
+  if (text === '') {
+    expect(
+      !node.nextSibling ||
+      node.nextSibling.nodeType !== TEXT_NODE
+    ).toBe(true);
+  } else {
+    expect(node.nextSibling.nodeType).toBe(TEXT_NODE);
+    expect(node.nextSibling.nodeValue).toBe(text);
+  }
+};
+
 var assertNodeText = function(instance, text) {
   expect(instance.getDOMNode().childNodes.length).toBe(1);
   expect(instance.getDOMNode().innerHTML).toBe('' + text);
@@ -38,15 +53,16 @@ var assertEmptyNode = function(instance) {
 };
 
 var assertMultiChild = function(instance, textOne, textTwo) {
-  expect(instance.getDOMNode().childNodes.length).toBe(2);
+  expect(instance.getDOMNode().childNodes.length).toBe(
+    2 + (textOne === '' ? 0 : 1) + (textTwo === '' ? 0 : 1)
+  );
   var firstTextDOMNode =
     reactComponentExpect(instance)
       .expectRenderedChildAt(0)
       .toBeTextComponent()
       .instance()
       .getDOMNode();
-  expect(firstTextDOMNode.childNodes.length).toBe(textOne === '' ? 0 : 1);
-  expect(firstTextDOMNode.innerHTML).toBe('' + textOne);
+  assertTextNodeAfter(firstTextDOMNode, textOne);
 
   var secondTextDOMNode =
     reactComponentExpect(instance)
@@ -54,20 +70,18 @@ var assertMultiChild = function(instance, textOne, textTwo) {
       .toBeTextComponent()
       .instance()
       .getDOMNode();
-  expect(secondTextDOMNode.childNodes.length).toBe(textTwo === '' ? 0 : 1);
-  expect(secondTextDOMNode.innerHTML).toBe('' + textTwo);
+  assertTextNodeAfter(secondTextDOMNode, textTwo);
 };
 
 var assertSingleChild = function(instance, text) {
-  expect(instance.getDOMNode().childNodes.length).toBe(1);
+  expect(instance.getDOMNode().childNodes.length).toBe(2);
   var textDOMNode =
     reactComponentExpect(instance)
       .expectRenderedChildAt(0)
       .toBeTextComponent()
       .instance()
       .getDOMNode();
-  expect(textDOMNode.childNodes.length).toBe(1);
-  expect(textDOMNode.innerHTML).toBe('' + text);
+  assertTextNodeAfter(textDOMNode, text);
 };
 
 // Helpers