Clear existing text content before inserting children

Fixes a bug when updating from a single text child (or
__dangerouslySetInnerHTML) to regular children, where the previous
text content never gets deleted.

Author: acdlite

scripts/fiber/tests-failing.txt

@@ -38,6 +38,9 @@ src/renderers/art/__tests__/ReactART-test.js
 * resolves refs before componentDidMount
 * resolves refs before componentDidUpdate
 
+src/renderers/dom/__tests__/ReactDOMProduction-test.js
+* should throw with an error code in production
+
 src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
 * should set style attribute when styles exist
 
@@ -77,8 +80,6 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 * should clear a single style prop when changing `style`
 * should update arbitrary attributes for tags containing dashes
 * should update styles when `style` changes from null to object
-* should empty element when removing innerHTML
-* should transition from innerHTML to children in nested el
 * should not incur unnecessary DOM mutations for attributes
 * should not incur unnecessary DOM mutations for string properties
 * should not incur unnecessary DOM mutations for boolean properties

scripts/fiber/tests-passing.txt

@@ -470,7 +470,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js
 * should use prod React
 * should handle a simple flow
 * should call lifecycle methods
-* should throw with an error code in production
 
 src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
 * should render strings as children
@@ -530,8 +529,10 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 * should skip reserved props on web components
 * should skip dangerouslySetInnerHTML on web components
 * should clear all the styles when removing `style`
+* should empty element when removing innerHTML
 * should transition from string content to innerHTML
 * should transition from innerHTML to string content
+* should transition from innerHTML to children in nested el
 * should transition from children to innerHTML in nested el
 * handles multiple child updates without interference
 * should generate the correct markup with className

src/renderers/dom/fiber/ReactDOMFiber.js

@@ -89,6 +89,22 @@ var DOMRenderer = ReactFiberReconciler({
     return true;
   },
 
+  shouldSetTextContent(props : Props) : boolean {
+    return (
+      typeof props.children === 'string' ||
+      typeof props.children === 'number' ||
+      (
+        typeof props.dangerouslySetInnerHTML === 'object' &&
+        props.dangerouslySetInnerHTML !== null &&
+        typeof props.dangerouslySetInnerHTML.__html === 'string'
+      )
+    );
+  },
+
+  resetTextContent(domElement : Instance) : void {
+    domElement.textContent = '';
+  },
+
   commitUpdate(domElement : Instance, oldProps : Props, newProps : Props) : void {
     if (typeof newProps.className !== 'undefined') {
       domElement.className = newProps.className;

src/renderers/noop/ReactNoop.js

@@ -88,6 +88,15 @@ var NoopRenderer = ReactFiberReconciler({
     return true;
   },
 
+  shouldSetTextContent(props : Props) : boolean {
+    return (
+      typeof props.children === 'string' ||
+      typeof props.children === 'number'
+    );
+  },
+
+  resetTextContent(instance : Instance) : void {},
+
   commitUpdate(instance : Instance, oldProps : Props, newProps : Props) : void {
     instance.prop = newProps.prop;
   },

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -50,6 +50,7 @@ var {
 } = require('ReactPriorityLevel');
 var {
   Placement,
+  ContentReset,
 } = require('ReactTypeOfSideEffect');
 var ReactCurrentOwner = require('ReactCurrentOwner');
 var ReactFiberClassComponent = require('ReactFiberClassComponent');
@@ -207,14 +208,27 @@ module.exports = function<T, P, I, TI, C>(
   }
 
   function updateHostComponent(current, workInProgress) {
+    const nextProps = workInProgress.pendingProps;
+    const prevProps = current ? current.memoizedProps : null;
     let nextChildren = workInProgress.pendingProps.children;
-    if (typeof nextChildren === 'string' || typeof nextChildren === 'number') {
+    const isDirectTextChild = config.shouldSetTextContent(nextProps);
+    if (isDirectTextChild) {
       // We special case a direct text child of a host node. This is a common
       // case. We won't handle it as a reified child. We will instead handle
       // this in the host environment that also have access to this prop. That
       // avoids allocating another HostText fiber and traversing it.
       nextChildren = null;
+    } else if (prevProps && (
+      config.shouldSetTextContent(prevProps) ||
+      prevProps.children === null ||
+      typeof prevProps.children === 'undefined' ||
+      typeof prevProps.children === 'boolean'
+    )) {
+      // If we're switching from a direct text child to a normal child, or to
+      // empty, we need to schedule the text content to be reset.
+      workInProgress.effectTag |= ContentReset;
     }
+
     if (workInProgress.pendingProps.hidden &&
         workInProgress.pendingWorkPriority !== OffscreenPriority) {
       // If this host component is hidden, we can bail out on the children.

src/renderers/shared/fiber/ReactFiberCommitWork.js

@@ -29,6 +29,7 @@ var {
   Placement,
   Update,
   Callback,
+  ContentReset,
 } = require('ReactTypeOfSideEffect');
 
 module.exports = function<T, P, I, TI, C>(
@@ -37,6 +38,7 @@ module.exports = function<T, P, I, TI, C>(
 ) {
 
   const updateContainer = config.updateContainer;
+  const resetTextContent = config.resetTextContent;
   const commitUpdate = config.commitUpdate;
   const commitTextUpdate = config.commitTextUpdate;
 
@@ -67,12 +69,12 @@ module.exports = function<T, P, I, TI, C>(
     }
   }
 
-  function getHostParent(fiber : Fiber) : ?I {
+  function getHostParent(fiber : Fiber) : ?Fiber {
     let parent = fiber.return;
     while (parent) {
       switch (parent.tag) {
         case HostComponent:
-          return parent.stateNode;
+          return parent;
         case HostContainer:
           // TODO: Currently we use the updateContainer feature to update these,
           // but we should be able to handle this case too.
@@ -124,10 +126,19 @@ module.exports = function<T, P, I, TI, C>(
 
   function commitInsertion(finishedWork : Fiber) : void {
     // Recursively insert all host nodes into the parent.
-    const parent = getHostParent(finishedWork);
-    if (!parent) {
+    const parentFiber = getHostParent(finishedWork);
+    if (!parentFiber) {
       return;
     }
+    const parent : I = parentFiber.stateNode;
+
+    if (parentFiber.effectTag & ContentReset) {
+      // Reset the text content of the parent before doing any insertions
+      resetTextContent(parent);
+      // Clear ContentReset from the effect tag
+      parentFiber.effectTag &= ~ContentReset;
+    }
+
     const before = getHostSibling(finishedWork);
     // We only have the top Fiber that was inserted but we need recurse down its
     // children to find all the terminal nodes.
@@ -219,7 +230,8 @@ module.exports = function<T, P, I, TI, C>(
 
   function commitDeletion(current : Fiber) : void {
     // Recursively delete all host nodes from the parent.
-    const parent = getHostParent(current);
+    const parentFiber = getHostParent(current);
+    const parent = parentFiber ? parentFiber.stateNode : null;
     // Detach refs and call componentWillUnmount() on the whole subtree.
     unmountHostComponents(parent, current);
 

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -50,6 +50,9 @@ export type HostConfig<T, P, I, TI, C> = {
   prepareUpdate(instance : I, oldProps : P, newProps : P) : boolean,
   commitUpdate(instance : I, oldProps : P, newProps : P) : void,
 
+  shouldSetTextContent(props : P) : boolean,
+  resetTextContent(instance : I) : void,
+
   createTextInstance(text : string, internalInstanceHandle : OpaqueNode) : TI,
   commitTextUpdate(textInstance : TI, oldText : string, newText : string) : void,
 

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -40,6 +40,7 @@ var {
   Update,
   PlacementAndUpdate,
   Deletion,
+  ContentReset,
   Callback,
   Err,
 } = require('ReactTypeOfSideEffect');
@@ -160,12 +161,17 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
     // First, we'll perform all the host insertions, updates, deletions and
     // ref unmounts.
     let effectfulFiber = finishedWork.firstEffect;
+
     while (effectfulFiber) {
+      if (effectfulFiber.effectTag & ContentReset) {
+        config.resetTextContent(effectfulFiber.stateNode);
+      }
+
       // The following switch statement is only concerned about placement,
       // updates, and deletions. To avoid needing to add a case for every
       // possible bitmap value, we remove the secondary effects from the
       // effect tag and switch on that value.
-      let primaryEffectTag = effectfulFiber.effectTag & ~(Callback | Err);
+      let primaryEffectTag = effectfulFiber.effectTag & ~(Callback | Err | ContentReset);
       switch (primaryEffectTag) {
         case Placement: {
           commitInsertion(effectfulFiber);

src/renderers/shared/fiber/ReactTypeOfSideEffect.js

@@ -12,14 +12,15 @@
 
 'use strict';
 
-export type TypeOfSideEffect = 0 | 1 | 2 | 3 | 4 | 8 | 16;
+export type TypeOfSideEffect = 0 | 1 | 2 | 3 | 4 | 8 | 16 | 32;
 
 module.exports = {
-  NoEffect: 0,                                // 0b00000
-  Placement: 1,                               // 0b00001
-  Update: 2,                                  // 0b00010
-  PlacementAndUpdate: 3,                      // 0b00011
-  Deletion: 4,                                // 0b00100
-  Callback: 8,                                // 0b01000
-  Err: 16,                                    // 0b10000
+  NoEffect: 0,                                // 0b000000
+  Placement: 1,                               // 0b000001
+  Update: 2,                                  // 0b000010
+  PlacementAndUpdate: 3,                      // 0b000011
+  Deletion: 4,                                // 0b000100
+  ContentReset: 8,                            // 0b001000
+  Callback: 16,                               // 0b010000
+  Err: 32,                                    // 0b100000
 };