diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index cc5e8e4bb0b..b397e754bec 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -32,8 +32,6 @@ src/renderers/dom/shared/__tests__/ReactDOM-test.js * throws in render() if the update callback is not a function src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js -* should empty element when removing innerHTML -* should transition from innerHTML to children in nested el * should warn for children on void elements * should report component containing invalid styles * should clean up input value tracking @@ -104,7 +102,6 @@ src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js * throws when rendering null at the top level src/renderers/shared/shared/__tests__/ReactMultiChildText-test.js -* should correctly handle all possible children for render and update * should reorder keyed text nodes src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 076dbde55db..4459f673833 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -601,8 +601,10 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should update arbitrary attributes for tags containing dashes * should clear all the styles when removing `style` * should update styles when `style` changes from null to object +* 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 * should not incur unnecessary DOM mutations for attributes * should not incur unnecessary DOM mutations for string properties @@ -1397,6 +1399,7 @@ src/renderers/shared/shared/__tests__/ReactMultiChildReconcile-test.js * should insert non-empty children in middle where nulls were src/renderers/shared/shared/__tests__/ReactMultiChildText-test.js +* should correctly handle all possible children for render and update * should throw if rendering both HTML and children * should render between nested components and inline children diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index c5da108c5a8..c4c851d8e33 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -117,6 +117,22 @@ var DOMRenderer = ReactFiberReconciler({ updateProperties(domElement, type, oldProps, newProps, root); }, + 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 = ''; + }, + createTextInstance(text : string, internalInstanceHandle : Object) : TextInstance { var textNode : TextInstance = document.createTextNode(text); precacheFiberNode(internalInstanceHandle, textNode); diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 9f614a465ea..1e2435aa62c 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -68,6 +68,15 @@ var NoopRenderer = ReactFiberReconciler({ instance.prop = newProps.prop; }, + shouldSetTextContent(props : Props) : boolean { + return ( + typeof props.children === 'string' || + typeof props.children === 'number' + ); + }, + + resetTextContent(instance : Instance) : void {}, + createTextInstance(text : string) : TextInstance { var inst = { text : text, id: instanceCounter++ }; // Hide from unit tests diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index efc31ecbc72..b4e0d3a6f03 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -53,6 +53,7 @@ var { } = require('ReactPriorityLevel'); var { Placement, + ContentReset, } = require('ReactTypeOfSideEffect'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactFiberClassComponent = require('ReactFiberClassComponent'); @@ -210,14 +211,27 @@ module.exports = function( } 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. diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 36e205c96c9..7391676ab89 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -30,6 +30,7 @@ var { Placement, Update, Callback, + ContentReset, } = require('ReactTypeOfSideEffect'); module.exports = function( @@ -38,6 +39,7 @@ module.exports = function( ) { const commitUpdate = config.commitUpdate; + const resetTextContent = config.resetTextContent; const commitTextUpdate = config.commitTextUpdate; const appendChild = config.appendChild; @@ -83,6 +85,17 @@ module.exports = function( throw new Error('Expected to find a host parent.'); } + function getHostParentFiber(fiber : Fiber) : Fiber { + let parent = fiber.return; + while (parent) { + if (isHostParent(parent)) { + return parent; + } + parent = parent.return; + } + throw new Error('Expected to find a host parent.'); + } + function isHostParent(fiber : Fiber) : boolean { return ( fiber.tag === HostComponent || @@ -133,12 +146,29 @@ module.exports = function( } function commitPlacement(finishedWork : Fiber) : void { - // Clear effect from effect tag before any errors can be thrown, so that - // we don't attempt to do this again - finishedWork.effectTag &= ~Placement; - // Recursively insert all host nodes into the parent. - const parent = getHostParent(finishedWork); + const parentFiber = getHostParentFiber(finishedWork); + let parent; + switch (parentFiber.tag) { + case HostComponent: + parent = parentFiber.stateNode; + break; + case HostContainer: + parent = parentFiber.stateNode.containerInfo; + break; + case Portal: + parent = parentFiber.stateNode.containerInfo; + break; + default: + throw new Error('Invalid host parent fiber.'); + } + 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. diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index ca2fbb7660b..58cfbd19bb3 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -49,6 +49,9 @@ export type HostConfig = { prepareUpdate(instance : I, oldProps : P, newProps : P) : boolean, commitUpdate(instance : I, oldProps : P, newProps : P, internalInstanceHandle : OpaqueNode) : void, + shouldSetTextContent(props : P) : boolean, + resetTextContent(instance : I) : void, + createTextInstance(text : string, internalInstanceHandle : OpaqueNode) : TI, commitTextUpdate(textInstance : TI, oldText : string, newText : string) : void, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 9eb147df96d..febc08f59ec 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -40,6 +40,7 @@ var { Update, PlacementAndUpdate, Deletion, + ContentReset, Callback, Err, } = require('ReactTypeOfSideEffect'); @@ -168,11 +169,15 @@ module.exports = function(config : HostConfig) { pass1: while (true) { try { 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: { commitPlacement(effectfulFiber); diff --git a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js b/src/renderers/shared/fiber/ReactTypeOfSideEffect.js index f8dc19407c8..13ac498329c 100644 --- a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js +++ b/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 };