Merge pull request #4427 from spicyj/gh-1232

Fix switching between dangerouslySetInnerHTML and children
facebook · Jul 20, 2015 · 16483e3 · 16483e3
2 parents 3ca5c15 + caae627
commit 16483e3
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 29 deletions.
diff --git a/src/renderers/dom/client/ReactDOMIDOperations.js b/src/renderers/dom/client/ReactDOMIDOperations.js
@@ -19,7 +19,6 @@ var ReactMount = require('ReactMount');
 var ReactPerf = require('ReactPerf');
 
 var invariant = require('invariant');
-var setInnerHTML = require('setInnerHTML');
 
 /**
  * Errors for properties that should not be updated with `updatePropertyByID()`.
@@ -115,18 +114,6 @@ var ReactDOMIDOperations = {
     CSSPropertyOperations.setValueForStyles(node, styles);
   },
 
-  /**
-   * Updates a DOM node's innerHTML.
-   *
-   * @param {string} id ID of the node to update.
-   * @param {string} html An HTML string.
-   * @internal
-   */
-  updateInnerHTMLByID: function(id, html) {
-    var node = ReactMount.getNode(id);
-    setInnerHTML(node, html);
-  },
-
   /**
    * Updates a DOM node's text content set by `props.content`.
    *
@@ -171,7 +158,6 @@ ReactPerf.measureMethods(ReactDOMIDOperations, 'ReactDOMIDOperations', {
   updatePropertyByID: 'updatePropertyByID',
   deletePropertyByID: 'deletePropertyByID',
   updateStylesByID: 'updateStylesByID',
-  updateInnerHTMLByID: 'updateInnerHTMLByID',
   updateTextContentByID: 'updateTextContentByID',
   dangerouslyReplaceNodeWithMarkupByID: 'dangerouslyReplaceNodeWithMarkupByID',
   dangerouslyProcessChildrenUpdates: 'dangerouslyProcessChildrenUpdates',

diff --git a/src/renderers/dom/client/__tests__/ReactDOMIDOperations-test.js b/src/renderers/dom/client/__tests__/ReactDOMIDOperations-test.js
@@ -15,6 +15,7 @@ describe('ReactDOMIDOperations', function() {
   var DOMPropertyOperations = require('DOMPropertyOperations');
   var ReactDOMIDOperations = require('ReactDOMIDOperations');
   var ReactMount = require('ReactMount');
+  var ReactMultiChildUpdateTypes = require('ReactMultiChildUpdateTypes');
   var keyOf = require('keyOf');
 
   it('should disallow updating special properties', function() {
@@ -44,9 +45,17 @@ describe('ReactDOMIDOperations', function() {
 
     var html = '\n  \t  <span>  \n  testContent  \t  </span>  \n  \t';
 
-    ReactDOMIDOperations.updateInnerHTMLByID(
-      'testID',
-      html
+    ReactDOMIDOperations.dangerouslyProcessChildrenUpdates(
+      [{
+        parentID: 'testID',
+        parentNode: null,
+        type: ReactMultiChildUpdateTypes.SET_MARKUP,
+        markupIndex: null,
+        content: html,
+        fromIndex: null,
+        toIndex: null,
+      }],
+      []
     );
 
     expect(

diff --git a/src/renderers/dom/client/utils/DOMChildrenOperations.js b/src/renderers/dom/client/utils/DOMChildrenOperations.js
@@ -15,6 +15,7 @@
 var Danger = require('Danger');
 var ReactMultiChildUpdateTypes = require('ReactMultiChildUpdateTypes');
 
+var setInnerHTML = require('setInnerHTML');
 var setTextContent = require('setTextContent');
 var invariant = require('invariant');
 
@@ -123,10 +124,16 @@ var DOMChildrenOperations = {
             update.toIndex
           );
           break;
+        case ReactMultiChildUpdateTypes.SET_MARKUP:
+          setInnerHTML(
+            update.parentNode,
+            update.content
+          );
+          break;
         case ReactMultiChildUpdateTypes.TEXT_CONTENT:
           setTextContent(
             update.parentNode,
-            update.textContent
+            update.content
           );
           break;
         case ReactMultiChildUpdateTypes.REMOVE_NODE:

diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js
@@ -884,10 +884,7 @@ ReactDOMComponent.Mixin = {
       }
     } else if (nextHtml != null) {
       if (lastHtml !== nextHtml) {
-        BackendIDOperations.updateInnerHTMLByID(
-          this._rootNodeID,
-          nextHtml
-        );
+        this.updateMarkup('' + nextHtml);
       }
     } else if (nextChildren != null) {
       this.updateChildren(nextChildren, transaction, context);

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -309,6 +309,30 @@ describe('ReactDOMComponent', function() {
       expect(container.firstChild.innerHTML).toEqual('adieu');
     });
 
+    it('should transition from innerHTML to children in nested el', function() {
+      var container = document.createElement('div');
+      React.render(
+        <div><div dangerouslySetInnerHTML={{__html: 'bonjour'}} /></div>,
+        container
+      );
+
+      expect(container.textContent).toEqual('bonjour');
+      React.render(<div><div><span>adieu</span></div></div>, container);
+      expect(container.textContent).toEqual('adieu');
+    });
+
+    it('should transition from children to innerHTML in nested el', function() {
+      var container = document.createElement('div');
+      React.render(<div><div><span>adieu</span></div></div>, container);
+
+      expect(container.textContent).toEqual('adieu');
+      React.render(
+        <div><div dangerouslySetInnerHTML={{__html: 'bonjour'}} /></div>,
+        container
+      );
+      expect(container.textContent).toEqual('bonjour');
+    });
+
     it('should not incur unnecessary DOM mutations', function() {
       var container = document.createElement('div');
       React.render(<div value="" />, container);

diff --git a/src/renderers/shared/reconciler/ReactMultiChild.js b/src/renderers/shared/reconciler/ReactMultiChild.js
@@ -53,14 +53,14 @@ var markupQueue = [];
  * @param {number} toIndex Destination index.
  * @private
  */
-function enqueueMarkup(parentID, markup, toIndex) {
+function enqueueInsertMarkup(parentID, markup, toIndex) {
   // NOTE: Null values reduce hidden classes.
   updateQueue.push({
     parentID: parentID,
     parentNode: null,
     type: ReactMultiChildUpdateTypes.INSERT_MARKUP,
     markupIndex: markupQueue.push(markup) - 1,
-    textContent: null,
+    content: null,
     fromIndex: null,
     toIndex: toIndex,
   });
@@ -81,7 +81,7 @@ function enqueueMove(parentID, fromIndex, toIndex) {
     parentNode: null,
     type: ReactMultiChildUpdateTypes.MOVE_EXISTING,
     markupIndex: null,
-    textContent: null,
+    content: null,
     fromIndex: fromIndex,
     toIndex: toIndex,
   });
@@ -101,12 +101,32 @@ function enqueueRemove(parentID, fromIndex) {
     parentNode: null,
     type: ReactMultiChildUpdateTypes.REMOVE_NODE,
     markupIndex: null,
-    textContent: null,
+    content: null,
     fromIndex: fromIndex,
     toIndex: null,
   });
 }
 
+/**
+ * Enqueues setting the markup of a node.
+ *
+ * @param {string} parentID ID of the parent component.
+ * @param {string} markup Markup that renders into an element.
+ * @private
+ */
+function enqueueSetMarkup(parentID, markup) {
+  // NOTE: Null values reduce hidden classes.
+  updateQueue.push({
+    parentID: parentID,
+    parentNode: null,
+    type: ReactMultiChildUpdateTypes.SET_MARKUP,
+    markupIndex: null,
+    content: markup,
+    fromIndex: null,
+    toIndex: null,
+  });
+}
+
 /**
  * Enqueues setting the text content.
  *
@@ -121,7 +141,7 @@ function enqueueTextContent(parentID, textContent) {
     parentNode: null,
     type: ReactMultiChildUpdateTypes.TEXT_CONTENT,
     markupIndex: null,
-    textContent: textContent,
+    content: textContent,
     fromIndex: null,
     toIndex: null,
   });
@@ -237,6 +257,38 @@ var ReactMultiChild = {
       }
     },
 
+    /**
+     * Replaces any rendered children with a markup string.
+     *
+     * @param {string} nextMarkup String of markup.
+     * @internal
+     */
+    updateMarkup: function(nextMarkup) {
+      updateDepth++;
+      var errorThrown = true;
+      try {
+        var prevChildren = this._renderedChildren;
+        // Remove any rendered children.
+        ReactChildReconciler.unmountChildren(prevChildren);
+        for (var name in prevChildren) {
+          if (prevChildren.hasOwnProperty(name)) {
+            this._unmountChildByName(prevChildren[name], name);
+          }
+        }
+        this.setMarkup(nextMarkup);
+        errorThrown = false;
+      } finally {
+        updateDepth--;
+        if (!updateDepth) {
+          if (errorThrown) {
+            clearQueue();
+          } else {
+            processQueue();
+          }
+        }
+      }
+    },
+
     /**
      * Updates the rendered children with new children.
      *
@@ -355,7 +407,7 @@ var ReactMultiChild = {
      * @protected
      */
     createChild: function(child, mountImage) {
-      enqueueMarkup(this._rootNodeID, mountImage, child._mountIndex);
+      enqueueInsertMarkup(this._rootNodeID, mountImage, child._mountIndex);
     },
 
     /**
@@ -378,6 +430,16 @@ var ReactMultiChild = {
       enqueueTextContent(this._rootNodeID, textContent);
     },
 
+    /**
+     * Sets this markup string.
+     *
+     * @param {string} markup Markup to set.
+     * @protected
+     */
+    setMarkup: function(markup) {
+      enqueueSetMarkup(this._rootNodeID, markup);
+    },
+
     /**
      * Mounts a child with the supplied name.
      *

diff --git a/src/renderers/shared/reconciler/ReactMultiChildUpdateTypes.js b/src/renderers/shared/reconciler/ReactMultiChildUpdateTypes.js
@@ -25,6 +25,7 @@ var ReactMultiChildUpdateTypes = keyMirror({
   INSERT_MARKUP: null,
   MOVE_EXISTING: null,
   REMOVE_NODE: null,
+  SET_MARKUP: null,
   TEXT_CONTENT: null,
 });
 

diff --git a/src/test/ReactDefaultPerfAnalysis.js b/src/test/ReactDefaultPerfAnalysis.js
@@ -20,11 +20,11 @@ var DOM_OPERATION_TYPES = {
   INSERT_MARKUP: 'set innerHTML',
   MOVE_EXISTING: 'move',
   REMOVE_NODE: 'remove',
+  SET_MARKUP: 'set innerHTML',
   TEXT_CONTENT: 'set textContent',
   'updatePropertyByID': 'update attribute',
   'deletePropertyByID': 'delete attribute',
   'updateStylesByID': 'update styles',
-  'updateInnerHTMLByID': 'set innerHTML',
   'dangerouslyReplaceNodeWithMarkupByID': 'replace',
 };