{"commit":{"shortRevision":"a9365b97e5","authoredAt":"2016-12-16T18:31:39-08:00","subject":"Remove unmountContainer","authorName":"Andrew Clark","authorEmail":"acdlite@fb.com","revision":"a9365b97e54c56616cd338efa2c1781f2637beb4"},"patch":"diff --git a\/scripts\/fiber\/tests-failing.txt b\/scripts\/fiber\/tests-failing.txt\nindex da68b422e9..dd4907f9b9 100644\n--- a\/scripts\/fiber\/tests-failing.txt\n+++ b\/scripts\/fiber\/tests-failing.txt\n@@ -90,7 +90,6 @@ src\/renderers\/shared\/shared\/__tests__\/ReactUpdates-test.js\n * throws in setState if the update callback is not a function\n * throws in replaceState if the update callback is not a function\n * throws in forceUpdate if the update callback is not a function\n-* unmounts and remounts a root in the same batch\n \n src\/renderers\/shared\/shared\/__tests__\/refs-test.js\n * Should increase refs with an increase in divs\ndiff --git a\/scripts\/fiber\/tests-passing.txt b\/scripts\/fiber\/tests-passing.txt\nindex 83bc0ef4bd..6cd7643feb 100644\n--- a\/scripts\/fiber\/tests-passing.txt\n+++ b\/scripts\/fiber\/tests-passing.txt\n@@ -1550,6 +1550,7 @@ src\/renderers\/shared\/shared\/__tests__\/ReactUpdates-test.js\n * does not update one component twice in a batch (#2410)\n * does not update one component twice in a batch (#6371)\n * unstable_batchedUpdates should return value from a callback\n+* unmounts and remounts a root in the same batch\n \n src\/renderers\/shared\/shared\/__tests__\/refs-destruction-test.js\n * should remove refs when destroying the parent\ndiff --git a\/src\/renderers\/art\/ReactARTFiber.js b\/src\/renderers\/art\/ReactARTFiber.js\nindex ffa9cc419c..ede8d96b8c 100644\n--- a\/src\/renderers\/art\/ReactARTFiber.js\n+++ b\/src\/renderers\/art\/ReactARTFiber.js\n@@ -356,7 +356,11 @@ class Surface extends Component {\n }\n \n componentWillUnmount() {\n- ARTRenderer.unmountContainer(this._mountNode);\n+ ARTRenderer.updateContainer(\n+ null,\n+ this._mountNode,\n+ this,\n+ );\n }\n \n render() {\ndiff --git a\/src\/renderers\/dom\/fiber\/ReactDOMFiber.js b\/src\/renderers\/dom\/fiber\/ReactDOMFiber.js\nindex 19c29643f3..fccc2e0655 100644\n--- a\/src\/renderers\/dom\/fiber\/ReactDOMFiber.js\n+++ b\/src\/renderers\/dom\/fiber\/ReactDOMFiber.js\n@@ -191,7 +191,7 @@ function warnAboutUnstableUse() {\n warned = true;\n }\n \n-function renderSubtreeIntoContainer(parentComponent : ?ReactComponent, element : ReactElement, containerNode : DOMContainerElement | Document, callback: ?Function) {\n+function renderSubtreeIntoContainer(parentComponent : ?ReactComponent, children : ReactNodeList, containerNode : DOMContainerElement | Document, callback: ?Function) {\n let container : DOMContainerElement =\n containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any);\n let root;\n@@ -200,9 +200,9 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent {\n container._reactRootContainer = null;\n- DOMRenderer.unmountContainer(root);\n- }\n+ });\n },\n \n findDOMNode: findDOMNode,\ndiff --git a\/src\/renderers\/native\/ReactNativeFiber.js b\/src\/renderers\/native\/ReactNativeFiber.js\nindex d11438a114..7595fa6b74 100644\n--- a\/src\/renderers\/native\/ReactNativeFiber.js\n+++ b\/src\/renderers\/native\/ReactNativeFiber.js\n@@ -326,7 +326,7 @@ const NativeRenderer = ReactFiberReconciler({\n \/\/ But creates an additional child Fiber for raw text children.\n \/\/ No additional native views are created though.\n \/\/ It's not clear to me which is better so I'm deferring for now.\n- \/\/ More context @ github.com\/facebook\/react\/pull\/8560#discussion_r92111303 \n+ \/\/ More context @ github.com\/facebook\/react\/pull\/8560#discussion_r92111303\n return false;\n },\n \n@@ -372,8 +372,9 @@ const ReactNative = {\n const root = roots.get(containerTag);\n if (root) {\n \/\/ TODO: Is it safe to reset this now or should I wait since this unmount could be deferred?\n- roots.delete(containerTag);\n- NativeRenderer.unmountContainer(root);\n+ NativeRenderer.updateContainer(null, root, null, () => {\n+ roots.delete(containerTag);\n+ });\n }\n },\n \ndiff --git a\/src\/renderers\/noop\/ReactNoop.js b\/src\/renderers\/noop\/ReactNoop.js\nindex 56f9208130..6d887d89c8 100644\n--- a\/src\/renderers\/noop\/ReactNoop.js\n+++ b\/src\/renderers\/noop\/ReactNoop.js\n@@ -188,10 +188,11 @@ var ReactNoop = {\n \n unmountRootWithID(rootID : string) {\n const root = roots.get(rootID);\n- roots.delete(rootID);\n- rootContainers.delete(rootID);\n if (root) {\n- NoopRenderer.unmountContainer(root);\n+ NoopRenderer.updateContainer(null, root, null, () => {\n+ roots.delete(rootID);\n+ rootContainers.delete(rootID);\n+ });\n }\n },\n \ndiff --git a\/src\/renderers\/shared\/fiber\/ReactFiberReconciler.js b\/src\/renderers\/shared\/fiber\/ReactFiberReconciler.js\nindex 8505ba1f53..ac34ef9a95 100644\n--- a\/src\/renderers\/shared\/fiber\/ReactFiberReconciler.js\n+++ b\/src\/renderers\/shared\/fiber\/ReactFiberReconciler.js\n@@ -15,6 +15,7 @@\n import type { Fiber } from 'ReactFiber';\n import type { FiberRoot } from 'ReactFiberRoot';\n import type { PriorityLevel } from 'ReactPriorityLevel';\n+import type { ReactNodeList } from 'ReactTypes';\n \n var {\n findCurrentUnmaskedContext,\n@@ -69,9 +70,8 @@ export type HostConfig = {\n };\n \n export type Reconciler = {\n- mountContainer(element : ReactElement, containerInfo : C, parentComponent : ?ReactComponent) : OpaqueNode,\n- updateContainer(element : ReactElement, container : OpaqueNode, parentComponent : ?ReactComponent) : void,\n- unmountContainer(container : OpaqueNode) : void,\n+ mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent) : OpaqueNode,\n+ updateContainer(element : ReactNodeList, container : OpaqueNode, parentComponent : ?ReactComponent) : void,\n performWithPriority(priorityLevel : PriorityLevel, fn : Function) : void,\n \/* eslint-disable no-undef *\/\n \/\/ FIXME: ESLint complains about type parameter\n@@ -97,7 +97,7 @@ getContextForSubtree._injectFiber(function(fiber : Fiber) {\n module.exports = function(config : HostConfig) : Reconciler {\n \n var {\n- scheduleSetState,\n+ scheduleTopLevelSetState,\n scheduleUpdateCallback,\n performWithPriority,\n batchedUpdates,\n@@ -107,12 +107,12 @@ module.exports = function(config : HostConfig, containerInfo : C, parentComponent : ?ReactComponent, callback: ?Function) : OpaqueNode {\n+ mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent, callback: ?Function) : OpaqueNode {\n const context = getContextForSubtree(parentComponent);\n const root = createFiberRoot(containerInfo, context);\n const current = root.current;\n \n- scheduleSetState(current, { element });\n+ scheduleTopLevelSetState(current, { element });\n if (callback) {\n scheduleUpdateCallback(current, callback);\n }\n@@ -127,32 +127,27 @@ module.exports = function(config : HostConfig, container : OpaqueNode, parentComponent : ?ReactComponent, callback: ?Function) : void {\n+ updateContainer(element : ReactNodeList, container : OpaqueNode, parentComponent : ?ReactComponent, callback: ?Function) : void {\n \/\/ TODO: If this is a nested container, this won't be the root.\n const root : FiberRoot = (container.stateNode : any);\n const current = root.current;\n \n root.pendingContext = getContextForSubtree(parentComponent);\n \n- scheduleSetState(current, { element });\n+ scheduleTopLevelSetState(current, { element });\n if (callback) {\n scheduleUpdateCallback(current, callback);\n }\n \n- if (__DEV__ && ReactFiberInstrumentation.debugTool) {\n- ReactFiberInstrumentation.debugTool.onUpdateContainer(root);\n- }\n- },\n-\n- unmountContainer(container : OpaqueNode) : void {\n- \/\/ TODO: If this is a nested container, this won't be the root.\n- const root : FiberRoot = (container.stateNode : any);\n- const current = root.current;\n-\n- scheduleSetState(current, { element: [] });\n-\n- if (__DEV__ && ReactFiberInstrumentation.debugTool) {\n- ReactFiberInstrumentation.debugTool.onUnmountContainer(root);\n+ if (__DEV__) {\n+ if (ReactFiberInstrumentation.debugTool) {\n+ if (element === null) {\n+ ReactFiberInstrumentation.debugTool.onUpdateContainer(root);\n+ } else {\n+ \/\/ This is an unmount\n+ ReactFiberInstrumentation.debugTool.onUnmountContainer(root);\n+ }\n+ }\n }\n },\n \ndiff --git a\/src\/renderers\/shared\/fiber\/ReactFiberScheduler.js b\/src\/renderers\/shared\/fiber\/ReactFiberScheduler.js\nindex 03ce6685dd..356c25422a 100644\n--- a\/src\/renderers\/shared\/fiber\/ReactFiberScheduler.js\n+++ b\/src\/renderers\/shared\/fiber\/ReactFiberScheduler.js\n@@ -59,6 +59,7 @@ var {\n addReplaceUpdate,\n addForceUpdate,\n addCallback,\n+ addTopLevelUpdate,\n } = require('ReactFiberUpdateQueue');\n \n var {\n@@ -1058,6 +1059,12 @@ module.exports = function(config : HostConfig(config : HostConfig,\n+ priorityLevel : PriorityLevel\n+) : void {\n+ const isTopLevelUnmount = partialState === null;\n+\n+ const update = {\n+ priorityLevel,\n+ partialState,\n+ callback: null,\n+ isReplace: false,\n+ isForced: false,\n+ isTopLevelUnmount,\n+ next: null,\n+ };\n+ const update2 = insertUpdate(fiber, update);\n+\n+ if (isTopLevelUnmount) {\n+ \/\/ Drop all updates that are lower-priority, so that the tree is not\n+ \/\/ remounted. We need to do this for both queues.\n+ const queue1 = fiber.updateQueue;\n+ const queue2 = fiber.alternate && fiber.alternate.updateQueue;\n+\n+ if (queue1 && update.next) {\n+ update.next = null;\n+ queue1.last = update;\n+ }\n+ if (queue2 && update2 && update2.next) {\n+ update2.next = null;\n+ queue2.last = update;\n+ }\n+ }\n+}\n+exports.addTopLevelUpdate = addTopLevelUpdate;\n+\n function getStateFromUpdate(update, instance, prevState, props) {\n const partialState = update.partialState;\n if (typeof partialState === 'function') {\n@@ -412,12 +459,14 @@ function beginUpdateQueue(\n if (update.isForced) {\n queue.hasForceUpdate = true;\n }\n- if (update.callback) {\n+ \/\/ Second condition ignores top-level unmount callbacks if they are not the\n+ \/\/ last update in the queue, since a subsequent update will cause a remount.\n+ if (update.callback && !(update.isTopLevelUnmount && update.next)) {\n+ const callbackUpdate = cloneUpdate(update);\n if (callbackList && callbackList.last) {\n- callbackList.last.next = update;\n- callbackList.last = update;\n+ callbackList.last.next = callbackUpdate;\n+ callbackList.last = callbackUpdate;\n } else {\n- const callbackUpdate = cloneUpdate(update);\n callbackList = {\n first: callbackUpdate,\n last: callbackUpdate,\ndiff --git a\/src\/renderers\/shared\/fiber\/__tests__\/ReactIncrementalErrorHandling-test.js b\/src\/renderers\/shared\/fiber\/__tests__\/ReactIncrementalErrorHandling-test.js\nindex 3ee13eaa63..94a163a80d 100644\n--- a\/src\/renderers\/shared\/fiber\/__tests__\/ReactIncrementalErrorHandling-test.js\n+++ b\/src\/renderers\/shared\/fiber\/__tests__\/ReactIncrementalErrorHandling-test.js\n@@ -654,6 +654,7 @@ describe('ReactIncrementalErrorHandling', () => {\n ReactNoop.unmountRootWithID('d');\n ReactNoop.unmountRootWithID('e');\n ReactNoop.unmountRootWithID('f');\n+ ReactNoop.flush();\n expect(ReactNoop.getChildren('a')).toEqual(null);\n expect(ReactNoop.getChildren('b')).toEqual(null);\n expect(ReactNoop.getChildren('c')).toEqual(null);\n","files":[{"status":"M","path":"scripts\/fiber\/tests-failing.txt"},{"status":"M","path":"scripts\/fiber\/tests-passing.txt"},{"status":"M","path":"src\/renderers\/art\/ReactARTFiber.js"},{"status":"M","path":"src\/renderers\/dom\/fiber\/ReactDOMFiber.js"},{"status":"M","path":"src\/renderers\/native\/ReactNativeFiber.js"},{"status":"M","path":"src\/renderers\/noop\/ReactNoop.js"},{"status":"M","path":"src\/renderers\/shared\/fiber\/ReactFiberReconciler.js"},{"status":"M","path":"src\/renderers\/shared\/fiber\/ReactFiberScheduler.js"},{"status":"M","path":"src\/renderers\/shared\/fiber\/ReactFiberUpdateQueue.js"},{"status":"M","path":"src\/renderers\/shared\/fiber\/__tests__\/ReactIncrementalErrorHandling-test.js"}],"body":"Instead, renderers should pass null to updateContainer.","parents":["d8ade83eb2019bffe19d71a9b1a03ad2e84b3150"]}

dillon1000/react

Files changed