diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index da68b422e9..dd4907f9b9 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -90,7 +90,6 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js * throws in setState if the update callback is not a function * throws in replaceState if the update callback is not a function * throws in forceUpdate if the update callback is not a function -* unmounts and remounts a root in the same batch src/renderers/shared/shared/__tests__/refs-test.js * Should increase refs with an increase in divs diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index a7ff053d78..b8bc8acea7 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1559,6 +1559,7 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js * does not update one component twice in a batch (#2410) * does not update one component twice in a batch (#6371) * unstable_batchedUpdates should return value from a callback +* unmounts and remounts a root in the same batch src/renderers/shared/shared/__tests__/refs-destruction-test.js * should remove refs when destroying the parent diff --git a/src/renderers/art/ReactARTFiber.js b/src/renderers/art/ReactARTFiber.js index 1c6160d039..89bbc61fda 100644 --- a/src/renderers/art/ReactARTFiber.js +++ b/src/renderers/art/ReactARTFiber.js @@ -357,7 +357,11 @@ class Surface extends Component { } componentWillUnmount() { - ARTRenderer.unmountContainer(this._mountNode); + ARTRenderer.updateContainer( + null, + this._mountNode, + this, + ); } render() { diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 3bbc4d52a0..738b5bf53a 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -259,7 +259,7 @@ function warnAboutUnstableUse() { warned = true; } -function renderSubtreeIntoContainer(parentComponent : ?ReactComponent, element : ReactElement, containerNode : DOMContainerElement | Document, callback: ?Function) { +function renderSubtreeIntoContainer(parentComponent : ?ReactComponent, children : ReactNodeList, containerNode : DOMContainerElement | Document, callback: ?Function) { let container : DOMContainerElement = containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any); let root; @@ -268,9 +268,9 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent { container._reactRootContainer = null; - DOMRenderer.unmountContainer(root); - } + }); }, findDOMNode: findDOMNode, diff --git a/src/renderers/native/ReactNativeFiber.js b/src/renderers/native/ReactNativeFiber.js index 41fe052b7e..42b39aca47 100644 --- a/src/renderers/native/ReactNativeFiber.js +++ b/src/renderers/native/ReactNativeFiber.js @@ -329,7 +329,7 @@ const NativeRenderer = ReactFiberReconciler({ // But creates an additional child Fiber for raw text children. // No additional native views are created though. // It's not clear to me which is better so I'm deferring for now. - // More context @ github.com/facebook/react/pull/8560#discussion_r92111303 + // More context @ github.com/facebook/react/pull/8560#discussion_r92111303 return false; }, @@ -375,8 +375,9 @@ const ReactNative = { const root = roots.get(containerTag); if (root) { // TODO: Is it safe to reset this now or should I wait since this unmount could be deferred? - roots.delete(containerTag); - NativeRenderer.unmountContainer(root); + NativeRenderer.updateContainer(null, root, null, () => { + roots.delete(containerTag); + }); } }, diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 90e7500c50..6dde764abd 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -198,10 +198,11 @@ var ReactNoop = { unmountRootWithID(rootID : string) { const root = roots.get(rootID); - roots.delete(rootID); - rootContainers.delete(rootID); if (root) { - NoopRenderer.unmountContainer(root); + NoopRenderer.updateContainer(null, root, null, () => { + roots.delete(rootID); + rootContainers.delete(rootID); + }); } }, diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index d8af96af6b..c22ef6a47f 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -15,6 +15,7 @@ import type { Fiber } from 'ReactFiber'; import type { FiberRoot } from 'ReactFiberRoot'; import type { PriorityLevel } from 'ReactPriorityLevel'; +import type { ReactNodeList } from 'ReactTypes'; var { findCurrentUnmaskedContext, @@ -70,9 +71,8 @@ export type HostConfig = { }; export type Reconciler = { - mountContainer(element : ReactElement, containerInfo : C, parentComponent : ?ReactComponent) : OpaqueNode, - updateContainer(element : ReactElement, container : OpaqueNode, parentComponent : ?ReactComponent) : void, - unmountContainer(container : OpaqueNode) : void, + mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent) : OpaqueNode, + updateContainer(element : ReactNodeList, container : OpaqueNode, parentComponent : ?ReactComponent) : void, performWithPriority(priorityLevel : PriorityLevel, fn : Function) : void, /* eslint-disable no-undef */ // FIXME: ESLint complains about type parameter @@ -98,7 +98,7 @@ getContextForSubtree._injectFiber(function(fiber : Fiber) { module.exports = function(config : HostConfig) : Reconciler { var { - scheduleSetState, + scheduleTopLevelSetState, scheduleUpdateCallback, performWithPriority, batchedUpdates, @@ -108,12 +108,12 @@ module.exports = function(config : HostConfig, containerInfo : C, parentComponent : ?ReactComponent, callback: ?Function) : OpaqueNode { + mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent, callback: ?Function) : OpaqueNode { const context = getContextForSubtree(parentComponent); const root = createFiberRoot(containerInfo, context); const current = root.current; - scheduleSetState(current, { element }); + scheduleTopLevelSetState(current, { element }); if (callback) { scheduleUpdateCallback(current, callback); } @@ -128,32 +128,27 @@ module.exports = function(config : HostConfig, container : OpaqueNode, parentComponent : ?ReactComponent, callback: ?Function) : void { + updateContainer(element : ReactNodeList, container : OpaqueNode, parentComponent : ?ReactComponent, callback: ?Function) : void { // TODO: If this is a nested container, this won't be the root. const root : FiberRoot = (container.stateNode : any); const current = root.current; root.pendingContext = getContextForSubtree(parentComponent); - scheduleSetState(current, { element }); + scheduleTopLevelSetState(current, { element }); if (callback) { scheduleUpdateCallback(current, callback); } - if (__DEV__ && ReactFiberInstrumentation.debugTool) { - ReactFiberInstrumentation.debugTool.onUpdateContainer(root); - } - }, - - unmountContainer(container : OpaqueNode) : void { - // TODO: If this is a nested container, this won't be the root. - const root : FiberRoot = (container.stateNode : any); - const current = root.current; - - scheduleSetState(current, { element: [] }); - - if (__DEV__ && ReactFiberInstrumentation.debugTool) { - ReactFiberInstrumentation.debugTool.onUnmountContainer(root); + if (__DEV__) { + if (ReactFiberInstrumentation.debugTool) { + if (element === null) { + ReactFiberInstrumentation.debugTool.onUpdateContainer(root); + } else { + // This is an unmount + ReactFiberInstrumentation.debugTool.onUnmountContainer(root); + } + } } }, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 03ce6685dd..356c25422a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -59,6 +59,7 @@ var { addReplaceUpdate, addForceUpdate, addCallback, + addTopLevelUpdate, } = require('ReactFiberUpdateQueue'); var { @@ -1058,6 +1059,12 @@ module.exports = function(config : HostConfig(config : HostConfig, + priorityLevel : PriorityLevel +) : void { + const isTopLevelUnmount = partialState === null; + + const update = { + priorityLevel, + partialState, + callback: null, + isReplace: false, + isForced: false, + isTopLevelUnmount, + next: null, + }; + const update2 = insertUpdate(fiber, update); + + if (isTopLevelUnmount) { + // Drop all updates that are lower-priority, so that the tree is not + // remounted. We need to do this for both queues. + const queue1 = fiber.updateQueue; + const queue2 = fiber.alternate && fiber.alternate.updateQueue; + + if (queue1 && update.next) { + update.next = null; + queue1.last = update; + } + if (queue2 && update2 && update2.next) { + update2.next = null; + queue2.last = update; + } + } +} +exports.addTopLevelUpdate = addTopLevelUpdate; + function getStateFromUpdate(update, instance, prevState, props) { const partialState = update.partialState; if (typeof partialState === 'function') { @@ -412,12 +459,14 @@ function beginUpdateQueue( if (update.isForced) { queue.hasForceUpdate = true; } - if (update.callback) { + // Second condition ignores top-level unmount callbacks if they are not the + // last update in the queue, since a subsequent update will cause a remount. + if (update.callback && !(update.isTopLevelUnmount && update.next)) { + const callbackUpdate = cloneUpdate(update); if (callbackList && callbackList.last) { - callbackList.last.next = update; - callbackList.last = update; + callbackList.last.next = callbackUpdate; + callbackList.last = callbackUpdate; } else { - const callbackUpdate = cloneUpdate(update); callbackList = { first: callbackUpdate, last: callbackUpdate, diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index 3ee13eaa63..94a163a80d 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -654,6 +654,7 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.unmountRootWithID('d'); ReactNoop.unmountRootWithID('e'); ReactNoop.unmountRootWithID('f'); + ReactNoop.flush(); expect(ReactNoop.getChildren('a')).toEqual(null); expect(ReactNoop.getChildren('b')).toEqual(null); expect(ReactNoop.getChildren('c')).toEqual(null);