Fix switching between dangerouslySetInnerHTML and children

With this, ReactMultiChild handles all of the children-related operations for ReactDOMComponent so that we don't process operations out of order. This is necessary because ReactMultiChild does its own batching so there's no way without its cooperation to get the timing right here.

Ideally we'll factor this logic out a bit better in subsequent updates but this is the simplest way to fix #1232 which has embarrassingly been open for over a year.

Author: sophiebits

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',

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(

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:

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

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

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.
      *

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

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',
 };