From d76eab188e51f7efac087f60940f27ddcca186fb Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 15:56:13 -0700 Subject: [PATCH 01/22] Update build size --- scripts/rollup/results.json | 96 ++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/scripts/rollup/results.json b/scripts/rollup/results.json index 0a5ce382a14..dc49cb86b8f 100644 --- a/scripts/rollup/results.json +++ b/scripts/rollup/results.json @@ -25,28 +25,28 @@ "gzip": 6709 }, "react-dom.development.js (UMD_DEV)": { - "size": 622438, - "gzip": 143476 + "size": 622611, + "gzip": 143520 }, "react-dom.production.min.js (UMD_PROD)": { - "size": 100793, - "gzip": 31792 + "size": 100874, + "gzip": 31801 }, "react-dom.development.js (NODE_DEV)": { - "size": 584733, - "gzip": 134780 + "size": 584906, + "gzip": 134811 }, "react-dom.production.min.js (NODE_PROD)": { - "size": 105201, - "gzip": 33053 + "size": 105280, + "gzip": 33069 }, "ReactDOMFiber-dev.js (FB_DEV)": { - "size": 581810, - "gzip": 134178 + "size": 581983, + "gzip": 134206 }, "ReactDOMFiber-prod.js (FB_PROD)": { - "size": 413106, - "gzip": 92244 + "size": 413270, + "gzip": 92266 }, "react-dom-test-utils.development.js (NODE_DEV)": { "size": 41688, @@ -113,44 +113,44 @@ "gzip": 6214 }, "react-art.development.js (UMD_DEV)": { - "size": 368779, - "gzip": 81552 + "size": 368952, + "gzip": 81593 }, "react-art.production.min.js (UMD_PROD)": { - "size": 82740, - "gzip": 25652 + "size": 82819, + "gzip": 25682 }, "react-art.development.js (NODE_DEV)": { - "size": 293112, - "gzip": 62358 + "size": 293285, + "gzip": 62391 }, "react-art.production.min.js (NODE_PROD)": { - "size": 52107, - "gzip": 16388 + "size": 52186, + "gzip": 16406 }, "ReactARTFiber-dev.js (FB_DEV)": { - "size": 291853, - "gzip": 62333 + "size": 292026, + "gzip": 62361 }, "ReactARTFiber-prod.js (FB_PROD)": { - "size": 217374, - "gzip": 45136 + "size": 217538, + "gzip": 45158 }, "ReactNativeFiber-dev.js (RN_DEV)": { - "size": 278956, - "gzip": 48430 + "size": 279140, + "gzip": 48459 }, "ReactNativeFiber-prod.js (RN_PROD)": { - "size": 217084, - "gzip": 37619 + "size": 217248, + "gzip": 37638 }, "react-test-renderer.development.js (NODE_DEV)": { - "size": 296835, - "gzip": 62765 + "size": 297008, + "gzip": 62795 }, "ReactTestRendererFiber-dev.js (FB_DEV)": { - "size": 295566, - "gzip": 62732 + "size": 295739, + "gzip": 62764 }, "react-test-renderer-shallow.development.js (NODE_DEV)": { "size": 9370, @@ -161,8 +161,8 @@ "gzip": 2253 }, "react-noop-renderer.development.js (NODE_DEV)": { - "size": 284424, - "gzip": 59683 + "size": 284597, + "gzip": 59713 }, "react-dom-server.development.js (UMD_DEV)": { "size": 120897, @@ -189,16 +189,16 @@ "gzip": 7520 }, "ReactNativeRTFiber-dev.js (RN_DEV)": { - "size": 210860, - "gzip": 35891 + "size": 211043, + "gzip": 35922 }, "ReactNativeRTFiber-prod.js (RN_PROD)": { - "size": 158763, - "gzip": 26631 + "size": 158926, + "gzip": 26650 }, "react-test-renderer.production.min.js (NODE_PROD)": { - "size": 53651, - "gzip": 16653 + "size": 53730, + "gzip": 16673 }, "react-test-renderer-shallow.production.min.js (NODE_PROD)": { "size": 4630, @@ -209,20 +209,20 @@ "gzip": 4241 }, "react-reconciler.development.js (NODE_DEV)": { - "size": 271721, - "gzip": 56840 + "size": 271894, + "gzip": 56870 }, "react-reconciler.production.min.js (NODE_PROD)": { - "size": 37669, - "gzip": 11733 + "size": 37748, + "gzip": 11753 }, "ReactNativeCSFiber-dev.js (RN_DEV)": { - "size": 203248, - "gzip": 34141 + "size": 203431, + "gzip": 34170 }, "ReactNativeCSFiber-prod.js (RN_PROD)": { - "size": 153658, - "gzip": 25401 + "size": 153821, + "gzip": 25424 } } } \ No newline at end of file From 9d0a1f0a2e92881d5c767fb7e927497d727a4b59 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 00:07:56 -0700 Subject: [PATCH 02/22] [CS] Clone container instead of new root concept The extra "root" concept is kind of unnecessary. Instead of having a mutable container even in the persistent mode, I'll instead make the container be immutable too and be cloned. Then the "commit" just becomes swapping the previous container for the new one. --- .../src/ReactNativeCSFiberEntry.js | 24 ++++++++++++++----- .../src/ReactFiberReconciler.js | 12 ++++++---- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js index dde53ac20f9..de5692c5ec6 100644 --- a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js +++ b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js @@ -175,13 +175,25 @@ const ReactNativeCSFiberRenderer = ReactFiberReconciler({ return 0; }, - createRootInstance( - rootContainerInstance: Container, - hostContext: {}, - ): Instance { - return 123; + cloneContainer(container: Container, keepChildren: boolean): Container { + return 0; }, - commitRootInstance(rootInstance: Instance): void {}, + tryToReuseContainer( + container: Container, + keepChildren: boolean, + ): Container { + return 0; + }, + + appendInititalChildToContainer( + container: Container, + child: Instance | TextInstance, + ): void {}, + + completeContainer( + oldContainer: Container, + newContainer: Container, + ): void {}, }, }); diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 7f6a7c3017c..3837bdcab23 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -100,7 +100,7 @@ export type HostConfig = { +hydration?: HydrationHostConfig, +mutation?: MutableUpdatesHostConfig, - +persistence?: PersistentUpdatesHostConfig, + +persistence?: PersistentUpdatesHostConfig, }; type MutableUpdatesHostConfig = { @@ -132,7 +132,7 @@ type MutableUpdatesHostConfig = { removeChildFromContainer(container: C, child: I | TI): void, }; -type PersistentUpdatesHostConfig = { +type PersistentUpdatesHostConfig = { cloneInstance( instance: I, updatePayload: PL, @@ -152,8 +152,12 @@ type PersistentUpdatesHostConfig = { keepChildren: boolean, ): I, - createRootInstance(rootContainerInstance: C, hostContext: CX): I, - commitRootInstance(rootInstance: I): void, + cloneContainer(container: C, keepChildren: boolean): C, + tryToReuseContainer(container: C, keepChildren: boolean): C, + + appendInititalChildToContainer(container: C, child: I | TI): void, + + completeContainer(oldContainer: C, newContainer: C): void, }; type HydrationHostConfig = { From eba046a3b5d7f36a07e7830ff337d18179102786 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 11:10:07 -0700 Subject: [PATCH 03/22] Change the signature or persistence again We may need to clone without any updates, e.g. when the children are changed. Passing in the previous node is not enough to recycle since it won't have the up-to-date props and children. It's really only useful to for allocation pooling. --- .../src/ReactNativeCSFiberEntry.js | 10 ++++++---- .../react-reconciler/src/ReactFiberReconciler.js | 13 +++++++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js index de5692c5ec6..76831cd1941 100644 --- a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js +++ b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js @@ -154,7 +154,7 @@ const ReactNativeCSFiberRenderer = ReactFiberReconciler({ persistence: { cloneInstance( instance: Instance, - updatePayload: Object, + updatePayload: null | Object, type: string, oldProps: Props, newProps: Props, @@ -163,14 +163,15 @@ const ReactNativeCSFiberRenderer = ReactFiberReconciler({ ): Instance { return 0; }, - tryToReuseInstance( + cloneInstanceOrRecycle( instance: Instance, - updatePayload: Object, + updatePayload: null | Object, type: string, oldProps: Props, newProps: Props, internalInstanceHandle: Object, keepChildren: boolean, + recyclableInstance: null | Instance, ): Instance { return 0; }, @@ -178,9 +179,10 @@ const ReactNativeCSFiberRenderer = ReactFiberReconciler({ cloneContainer(container: Container, keepChildren: boolean): Container { return 0; }, - tryToReuseContainer( + cloneContainerOrRecycle( container: Container, keepChildren: boolean, + recyclableContainer: Container, ): Container { return 0; }, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 3837bdcab23..ca17e866d74 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -135,25 +135,30 @@ type MutableUpdatesHostConfig = { type PersistentUpdatesHostConfig = { cloneInstance( instance: I, - updatePayload: PL, + updatePayload: null | PL, type: T, oldProps: P, newProps: P, internalInstanceHandle: OpaqueHandle, keepChildren: boolean, ): I, - tryToReuseInstance( + cloneInstanceOrRecycle( instance: I, - updatePayload: PL, + updatePayload: null | PL, type: T, oldProps: P, newProps: P, internalInstanceHandle: OpaqueHandle, keepChildren: boolean, + recyclableInstance: I, ): I, cloneContainer(container: C, keepChildren: boolean): C, - tryToReuseContainer(container: C, keepChildren: boolean): C, + cloneContainerOrRecycle( + container: C, + keepChildren: boolean, + recyclableContainer: C, + ): C, appendInititalChildToContainer(container: C, child: I | TI): void, From 8a21e261ae65654da5995a568132cec0d95aca25 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 11:12:43 -0700 Subject: [PATCH 04/22] Implement persistent updates This forks the update path for host fibers. For mutation mode we mark them as having an effect. For persistence mode, we clone the stateNode with new props/children. Next I'll do HostRoot and HostPortal. --- .../src/ReactFiberCompleteWork.js | 174 +++++++++++++++++- 1 file changed, 164 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index afc3b5d52b8..43756105dbe 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -55,6 +55,8 @@ module.exports = function( appendInitialChild, finalizeInitialChildren, prepareUpdate, + mutation, + persistence, } = config; const { @@ -177,6 +179,157 @@ module.exports = function( } } + let updateHostComponent; + let updateHostText; + if (mutation) { + // Mutation mode + updateHostComponent = function( + current: Fiber, + workInProgress: Fiber, + updatePayload: null | PL, + type: T, + oldProps: P, + newProps: P, + rootContainerInstance: C, + ) { + // TODO: Type this specific to this type of component. + workInProgress.updateQueue = (updatePayload: any); + // If the update payload indicates that there is a change or if there + // is a new ref we mark this as an update. All the work is done in commitWork. + if (updatePayload) { + markUpdate(workInProgress); + } + }; + updateHostText = function( + current: Fiber, + workInProgress: Fiber, + oldText: string, + newText: string, + ) { + // If the text differs, mark it as an update. All the work in done in commitWork. + if (oldText !== newText) { + markUpdate(workInProgress); + } + }; + } else if (persistence) { + // Persistent host tree mode + const { + cloneInstance, + cloneInstanceOrRecycle, + cloneContainer, + cloneContainerOrRecycle, + appendInititalChildToContainer, + completeContainer, + } = persistence; + updateHostComponent = function( + current: Fiber, + workInProgress: Fiber, + updatePayload: null | PL, + type: T, + oldProps: P, + newProps: P, + rootContainerInstance: C, + ) { + // If there are no effects associated with this node, then none of our children had any updates. + // This guarantees that we can reuse all of them. + const childrenUnchanged = workInProgress.firstEffect === null; + const currentInstance = current.stateNode; + if (childrenUnchanged && updatePayload === null) { + // No changes, just reuse the existing instance. + // Note that this might release a previous clone. + workInProgress.stateNode = currentInstance; + } else { + let recyclableInstance = workInProgress.stateNode; + let newInstance; + if (currentInstance === recyclableInstance) { + // We can't recycle the current, we'll need to clone. + newInstance = cloneInstance( + currentInstance, + updatePayload, + type, + oldProps, + newProps, + workInProgress, + childrenUnchanged, + ); + } else { + newInstance = cloneInstanceOrRecycle( + currentInstance, + updatePayload, + type, + oldProps, + newProps, + workInProgress, + childrenUnchanged, + recyclableInstance, + ); + } + if ( + finalizeInitialChildren( + newInstance, + type, + newProps, + rootContainerInstance, + ) + ) { + markUpdate(workInProgress); + } + workInProgress.stateNode = newInstance; + if (childrenUnchanged) { + // If there are no other effects in this tree, we need to flag this node as having one. + // Even though we're not going to use it for anything. + // Otherwise parents won't know that there are new children to propagate upwards. + markUpdate(workInProgress); + } else { + // If children might have changed, we have to add them all to the set. + appendAllChildren(newInstance, workInProgress); + } + } + }; + updateHostText = function( + current: Fiber, + workInProgress: Fiber, + oldText: string, + newText: string, + ) { + if (oldText !== newText) { + // If the text content differs, we'll create a new text instance for it. + const rootContainerInstance = getRootHostContainer(); + const currentHostContext = getHostContext(); + workInProgress.stateNode = createTextInstance( + newText, + rootContainerInstance, + currentHostContext, + workInProgress, + ); + // We'll have to mark it as having an effect, even though we won't use the effect for anything. + // This lets the parents know that at least one of their children has changed. + markUpdate(workInProgress); + } + }; + } else { + // No host operations + updateHostComponent = function( + current: Fiber, + workInProgress: Fiber, + updatePayload: null | PL, + type: T, + oldProps: P, + newProps: P, + rootContainerInstance: C, + ) { + // Noop + }; + updateHostText = function( + current: Fiber, + workInProgress: Fiber, + oldText: string, + newText: string, + ) { + // Noop + }; + } + function completeWork( current: Fiber | null, workInProgress: Fiber, @@ -244,13 +397,16 @@ module.exports = function( currentHostContext, ); - // TODO: Type this specific to this type of component. - workInProgress.updateQueue = (updatePayload: any); - // If the update payload indicates that there is a change or if there - // is a new ref we mark this as an update. - if (updatePayload) { - markUpdate(workInProgress); - } + updateHostComponent( + current, + workInProgress, + updatePayload, + type, + oldProps, + newProps, + rootContainerInstance, + ); + if (current.ref !== workInProgress.ref) { markRef(workInProgress); } @@ -325,9 +481,7 @@ module.exports = function( const oldText = current.memoizedProps; // If we have an alternate, that means this is an update and we need // to schedule a side-effect to do the updates. - if (oldText !== newText) { - markUpdate(workInProgress); - } + updateHostText(current, workInProgress, oldText, newText); } else { if (typeof newText !== 'string') { invariant( From 33163d5e910bd39f4e1f658152b446594084c034 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 13:57:03 -0700 Subject: [PATCH 05/22] Refine protocol into a complete and commit phase finalizeContainerChildren will get called at the complete phase. replaceContainer will get called at commit. Also, drop the keepChildren flag. We'll never keep children as we'll never update a container if none of the children has changed. --- .../react-cs-renderer/src/ReactNativeCSFiberEntry.js | 10 ++++------ .../react-reconciler/src/ReactFiberCompleteWork.js | 2 +- packages/react-reconciler/src/ReactFiberReconciler.js | 11 ++++------- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js index 76831cd1941..06a928b98ef 100644 --- a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js +++ b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js @@ -176,12 +176,11 @@ const ReactNativeCSFiberRenderer = ReactFiberReconciler({ return 0; }, - cloneContainer(container: Container, keepChildren: boolean): Container { + cloneContainer(container: Container): Container { return 0; }, cloneContainerOrRecycle( container: Container, - keepChildren: boolean, recyclableContainer: Container, ): Container { return 0; @@ -192,10 +191,9 @@ const ReactNativeCSFiberRenderer = ReactFiberReconciler({ child: Instance | TextInstance, ): void {}, - completeContainer( - oldContainer: Container, - newContainer: Container, - ): void {}, + finalizeContainerChildren(container: Container): void {}, + + replaceContainer(oldContainer: Container, newContainer: Container): void {}, }, }); diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 43756105dbe..96d39aa9563 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -219,7 +219,7 @@ module.exports = function( cloneContainer, cloneContainerOrRecycle, appendInititalChildToContainer, - completeContainer, + finalizeContainerChildren, } = persistence; updateHostComponent = function( current: Fiber, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index ca17e866d74..a4cb0502c88 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -153,16 +153,13 @@ type PersistentUpdatesHostConfig = { recyclableInstance: I, ): I, - cloneContainer(container: C, keepChildren: boolean): C, - cloneContainerOrRecycle( - container: C, - keepChildren: boolean, - recyclableContainer: C, - ): C, + cloneContainer(container: C): C, + cloneContainerOrRecycle(container: C, recyclableContainer: C): C, appendInititalChildToContainer(container: C, child: I | TI): void, + finalizeContainerChildren(container: C): void, - completeContainer(oldContainer: C, newContainer: C): void, + replaceContainer(oldContainer: C, newContainer: C): void, }; type HydrationHostConfig = { From 1bde3dd3b15f2f3fba90bc14d234d90adfefb6b8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 14:37:44 -0700 Subject: [PATCH 06/22] Implement persistent updates of roots and portals These are both "containers". Normally we rely on placement/deletion effects to deal with insertions into the containers. In the persistent mode we need to clone the container and append all the changed children to it. I needed somewhere to store these new containers before they're committed so I added another field. --- packages/react-reconciler/src/ReactFiber.js | 1 + .../src/ReactFiberCompleteWork.js | 73 +++++++++++++++++++ .../react-reconciler/src/ReactFiberRoot.js | 3 + 3 files changed, 77 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 633911db341..6c08388990a 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -445,6 +445,7 @@ exports.createFiberFromPortal = function( fiber.expirationTime = expirationTime; fiber.stateNode = { containerInfo: portal.containerInfo, + pendingContainerInfo: portal.containerInfo, // Used by persistent updates implementation: portal.implementation, }; return fiber; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 96d39aa9563..a5d1c658de8 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -179,10 +179,14 @@ module.exports = function( } } + let updateHostContainer; let updateHostComponent; let updateHostText; if (mutation) { // Mutation mode + updateHostContainer = function(workInProgress: Fiber) { + // Noop + }; updateHostComponent = function( current: Fiber, workInProgress: Fiber, @@ -221,6 +225,70 @@ module.exports = function( appendInititalChildToContainer, finalizeContainerChildren, } = persistence; + + // An unfortunate fork of appendAllChildren because we have two different parent types. + const appendAllChildrenToContainer = function( + container: C, + workInProgress: Fiber, + ) { + // We only have the top Fiber that was created but we need recurse down its + // children to find all the terminal nodes. + let node = workInProgress.child; + while (node !== null) { + if (node.tag === HostComponent || node.tag === HostText) { + appendInititalChildToContainer(container, node.stateNode); + } else if (node.tag === HostPortal) { + // If we have a portal child, then we don't want to traverse + // down its children. Instead, we'll get insertions from each child in + // the portal directly. + } else if (node.child !== null) { + node = node.child; + continue; + } + if (node === workInProgress) { + return; + } + while (node.sibling === null) { + if (node.return === null || node.return === workInProgress) { + return; + } + node = node.return; + } + node = node.sibling; + } + }; + updateHostContainer = function(workInProgress: Fiber) { + const portalOrRoot: {containerInfo: C, pendingContainerInfo: C} = + workInProgress.stateNode; + const currentContainer = portalOrRoot.containerInfo; + const recyclableContainer = portalOrRoot.pendingContainerInfo; + + const childrenUnchanged = workInProgress.firstEffect === null; + if (childrenUnchanged) { + // No changes, just reuse the existing instance. + // Note that this might release a previous clone. + portalOrRoot.pendingContainerInfo = currentContainer; + } else { + let newContainer; + if (currentContainer === recyclableContainer) { + // We can't recycle the current, we'll need to clone. + newContainer = cloneContainer(currentContainer); + } else { + newContainer = cloneContainerOrRecycle( + currentContainer, + recyclableContainer, + ); + } + if (finalizeContainerChildren(newContainer)) { + markUpdate(workInProgress); + } + portalOrRoot.pendingContainerInfo = newContainer; + // If children might have changed, we have to add them all to the set. + appendAllChildrenToContainer(newContainer, workInProgress); + // Schedule an update on the container to swap out the container. + markUpdate(workInProgress); + } + }; updateHostComponent = function( current: Fiber, workInProgress: Fiber, @@ -309,6 +377,9 @@ module.exports = function( }; } else { // No host operations + updateHostContainer = function(workInProgress: Fiber) { + // Noop + }; updateHostComponent = function( current: Fiber, workInProgress: Fiber, @@ -372,6 +443,7 @@ module.exports = function( // TODO: Delete this when we delete isMounted and findDOMNode. workInProgress.effectTag &= ~Placement; } + updateHostContainer(workInProgress); return null; } case HostComponent: { @@ -527,6 +599,7 @@ module.exports = function( return null; case HostPortal: popHostContainer(workInProgress); + updateHostContainer(workInProgress); return null; // Error cases case IndeterminateComponent: diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 419e842e08b..704cd60b928 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -17,6 +17,8 @@ const {createHostRootFiber} = require('ReactFiber'); export type FiberRoot = { // Any additional information from the host associated with this root. containerInfo: any, + // Used only by persistent updates. + pendingContainerInfo: any, // The currently active root fiber. This is the mutable root of the tree. current: Fiber, // Determines if this root has already been added to the schedule for work. @@ -40,6 +42,7 @@ exports.createFiberRoot = function( const root = { current: uninitializedFiber, containerInfo: containerInfo, + pendingContainerInfo: containerInfo, isScheduled: false, nextScheduledRoot: null, context: null, From bb4ccc4abca1db19c227e34bbb1dbb2c7cf052b8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 14:58:01 -0700 Subject: [PATCH 07/22] Commit persistent work at the end by swapping out the container --- .../src/ReactFiberCommitWork.js | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 1cf90d93e30..4afbc2ee425 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -273,6 +273,43 @@ module.exports = function( } if (!config.mutation) { + let commitContainer; + if (!config.persistence) { + commitContainer = function(finishedWork: Fiber) { + // Noop + }; + } else { + const {replaceContainer} = config.persistence; + commitContainer = function(finishedWork: Fiber) { + switch (finishedWork.tag) { + case ClassComponent: { + return; + } + case HostComponent: { + return; + } + case HostRoot: + case HostPortal: { + const portalOrRoot: {containerInfo: C, pendingContainerInfo: C} = + finishedWork.stateNode; + const {containerInfo, pendingContainerInfo} = portalOrRoot; + replaceContainer(containerInfo, pendingContainerInfo); + // Swap out the current container. + portalOrRoot.containerInfo = pendingContainerInfo; + // The old one is now free to be recycled. + portalOrRoot.pendingContainerInfo = containerInfo; + return; + } + default: { + invariant( + false, + 'This unit of work tag should not have side-effects. This error is ' + + 'likely caused by a bug in React. Please file an issue.', + ); + } + } + }; + } return { commitResetTextContent(finishedWork: Fiber) {}, commitPlacement(finishedWork: Fiber) {}, @@ -281,7 +318,9 @@ module.exports = function( commitNestedUnmounts(current); detachFiber(current); }, - commitWork(current: Fiber | null, finishedWork: Fiber) {}, + commitWork(current: Fiber | null, finishedWork: Fiber) { + commitContainer(finishedWork); + }, commitLifeCycles, commitAttachRef, commitDetachRef, From cf738c536fe77e406be20ba409f7d62e5cb4a95f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 15:06:17 -0700 Subject: [PATCH 08/22] Unify cloneOrRecycle Originally I tried to make the recyclable instance nullable but Flow didn't like that and it's kind of sketchy since the instance type might not be nullable. However, the real difference which one we call is depending on whether they are equal. We can just offload that to the renderer. Most of them won't need to know about this at all since they'll always clone or just create new. The ones that do know now have to be careful to compare them so they don't reuse an existing instance but that's probably fine to simplify the implementation and API. --- .../src/ReactNativeCSFiberEntry.js | 16 +----- .../src/ReactFiberCompleteWork.js | 50 ++++++------------- .../src/ReactFiberReconciler.js | 12 +---- 3 files changed, 16 insertions(+), 62 deletions(-) diff --git a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js index 06a928b98ef..fbbfb4548f0 100644 --- a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js +++ b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js @@ -160,26 +160,12 @@ const ReactNativeCSFiberRenderer = ReactFiberReconciler({ newProps: Props, internalInstanceHandle: Object, keepChildren: boolean, - ): Instance { - return 0; - }, - cloneInstanceOrRecycle( - instance: Instance, - updatePayload: null | Object, - type: string, - oldProps: Props, - newProps: Props, - internalInstanceHandle: Object, - keepChildren: boolean, recyclableInstance: null | Instance, ): Instance { return 0; }, - cloneContainer(container: Container): Container { - return 0; - }, - cloneContainerOrRecycle( + cloneContainer( container: Container, recyclableContainer: Container, ): Container { diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index a5d1c658de8..520e4c291f7 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -219,9 +219,7 @@ module.exports = function( // Persistent host tree mode const { cloneInstance, - cloneInstanceOrRecycle, cloneContainer, - cloneContainerOrRecycle, appendInititalChildToContainer, finalizeContainerChildren, } = persistence; @@ -269,16 +267,10 @@ module.exports = function( // Note that this might release a previous clone. portalOrRoot.pendingContainerInfo = currentContainer; } else { - let newContainer; - if (currentContainer === recyclableContainer) { - // We can't recycle the current, we'll need to clone. - newContainer = cloneContainer(currentContainer); - } else { - newContainer = cloneContainerOrRecycle( - currentContainer, - recyclableContainer, - ); - } + let newContainer = cloneContainer( + currentContainer, + recyclableContainer, + ); if (finalizeContainerChildren(newContainer)) { markUpdate(workInProgress); } @@ -308,30 +300,16 @@ module.exports = function( workInProgress.stateNode = currentInstance; } else { let recyclableInstance = workInProgress.stateNode; - let newInstance; - if (currentInstance === recyclableInstance) { - // We can't recycle the current, we'll need to clone. - newInstance = cloneInstance( - currentInstance, - updatePayload, - type, - oldProps, - newProps, - workInProgress, - childrenUnchanged, - ); - } else { - newInstance = cloneInstanceOrRecycle( - currentInstance, - updatePayload, - type, - oldProps, - newProps, - workInProgress, - childrenUnchanged, - recyclableInstance, - ); - } + let newInstance = cloneInstance( + currentInstance, + updatePayload, + type, + oldProps, + newProps, + workInProgress, + childrenUnchanged, + recyclableInstance, + ); if ( finalizeInitialChildren( newInstance, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index a4cb0502c88..2fcff4aaa7c 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -141,20 +141,10 @@ type PersistentUpdatesHostConfig = { newProps: P, internalInstanceHandle: OpaqueHandle, keepChildren: boolean, - ): I, - cloneInstanceOrRecycle( - instance: I, - updatePayload: null | PL, - type: T, - oldProps: P, - newProps: P, - internalInstanceHandle: OpaqueHandle, - keepChildren: boolean, recyclableInstance: I, ): I, - cloneContainer(container: C): C, - cloneContainerOrRecycle(container: C, recyclableContainer: C): C, + cloneContainer(container: C, recyclableContainer: C): C, appendInititalChildToContainer(container: C, child: I | TI): void, finalizeContainerChildren(container: C): void, From 8bb653870bbd1651867fb62e5ce0de83e1d98ac1 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 15:30:57 -0700 Subject: [PATCH 09/22] Add persistent noop renderer for testing --- .../react-noop-renderer/src/ReactNoopEntry.js | 64 ++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/packages/react-noop-renderer/src/ReactNoopEntry.js b/packages/react-noop-renderer/src/ReactNoopEntry.js index 89bfc33c2d7..5b828362166 100644 --- a/packages/react-noop-renderer/src/ReactNoopEntry.js +++ b/packages/react-noop-renderer/src/ReactNoopEntry.js @@ -85,7 +85,7 @@ function removeChild( let elapsedTimeInMs = 0; -var NoopRenderer = ReactFiberReconciler({ +var SharedHostConfig = { getRootHostContext() { if (failInBeginPhase) { throw new Error('Error in host config.'); @@ -176,7 +176,10 @@ var NoopRenderer = ReactFiberReconciler({ now(): number { return elapsedTimeInMs; }, +}; +var NoopRenderer = ReactFiberReconciler({ + ...SharedHostConfig, mutation: { commitMount(instance: Instance, type: string, newProps: Props): void { // Noop @@ -211,8 +214,52 @@ var NoopRenderer = ReactFiberReconciler({ }, }); +var PersistentNoopRenderer = ReactFiberReconciler({ + ...SharedHostConfig, + persistence: { + cloneInstance( + instance: Instance, + updatePayload: null | Object, + type: string, + oldProps: Props, + newProps: Props, + internalInstanceHandle: Object, + keepChildren: boolean, + recyclableInstance: null | Instance, + ): Instance { + const clone = { + id: instance.id, + type: type, + children: keepChildren ? instance.children : [], + prop: newProps.prop, + }; + Object.defineProperty(clone, 'id', { + value: clone.id, + enumerable: false, + }); + return clone; + }, + + cloneContainer( + container: Container, + recyclableContainer: Container, + ): Container { + return {rootID: container.rootID, children: []}; + }, + + appendInititalChildToContainer: appendChild, + + finalizeContainerChildren(container: Container): void {}, + + replaceContainer(oldContainer: Container, newContainer: Container): void { + rootContainers.set(oldContainer.rootID, newContainer); + }, + }, +}); + var rootContainers = new Map(); var roots = new Map(); +var persistentRoots = new Map(); var DEFAULT_ROOT_ID = ''; let yieldedValues = null; @@ -275,6 +322,21 @@ var ReactNoop = { NoopRenderer.updateContainer(element, root, null, callback); }, + renderToPersistentRootWithID( + element: React$Element, + rootID: string, + callback: ?Function, + ) { + let root = persistentRoots.get(rootID); + if (!root) { + const container = {rootID: rootID, children: []}; + rootContainers.set(rootID, container); + root = PersistentNoopRenderer.createContainer(container, false); + persistentRoots.set(rootID, root); + } + PersistentNoopRenderer.updateContainer(element, root, null, callback); + }, + unmountRootWithID(rootID: string) { const root = roots.get(rootID); if (root) { From 9b9344cd264ffc03ced77f32c01e96736eee33b7 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 22:03:43 -0700 Subject: [PATCH 10/22] Add basic persistent tree test --- .../fiber/__tests__/ReactPersistent-test.js | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 src/renderers/shared/fiber/__tests__/ReactPersistent-test.js diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js new file mode 100644 index 00000000000..1ccab9571f0 --- /dev/null +++ b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js @@ -0,0 +1,67 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +var React; +var ReactNoop; + +describe('ReactPersistent', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactNoop = require('react-noop-renderer'); + }); + + const DEFAULT_ROOT_ID = 'persistent-test'; + + function render(element) { + ReactNoop.renderToPersistentRootWithID(element, DEFAULT_ROOT_ID); + } + + function div(...children) { + children = children.map(c => (typeof c === 'string' ? {text: c} : c)); + return {type: 'div', children, prop: undefined}; + } + + function span(prop) { + return {type: 'span', children: [], prop}; + } + + function getChildren() { + return ReactNoop.getChildren(DEFAULT_ROOT_ID); + } + + it('can update child nodes of a host instance', () => { + function Bar(props) { + return {props.text}; + } + + function Foo(props) { + return ( +
+ + {props.text === 'World' ? : null} +
+ ); + } + + render(); + ReactNoop.flush(); + var originalChildren = getChildren(); + expect(originalChildren).toEqual([div(span())]); + + render(); + ReactNoop.flush(); + var newChildren = getChildren(); + expect(newChildren).toEqual([div(span(), span())]); + + expect(originalChildren).toEqual([div(span())]); + }); +}); From a1a8249ef0ab9d1a101613c14a53aac1fe62be80 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 22:51:15 -0700 Subject: [PATCH 11/22] Test bail out This adds a test for bailouts. This revealed a subtle bug. We don't set the return pointer when stepping into newly created fibers because there can only be one. However, since I'm reusing this mechanism for persistent updates, I'll need to set the return pointer because a bailed out tree won't have the right return pointer. --- .../react-noop-renderer/src/ReactNoopEntry.js | 7 +++- .../src/ReactFiberCompleteWork.js | 4 ++ .../fiber/__tests__/ReactPersistent-test.js | 37 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/react-noop-renderer/src/ReactNoopEntry.js b/packages/react-noop-renderer/src/ReactNoopEntry.js index 5b828362166..cba0c28e0ae 100644 --- a/packages/react-noop-renderer/src/ReactNoopEntry.js +++ b/packages/react-noop-renderer/src/ReactNoopEntry.js @@ -247,7 +247,12 @@ var PersistentNoopRenderer = ReactFiberReconciler({ return {rootID: container.rootID, children: []}; }, - appendInititalChildToContainer: appendChild, + appendInititalChildToContainer( + parentInstance: Container, + child: Instance | TextInstance, + ) { + parentInstance.children.push(child); + }, finalizeContainerChildren(container: Container): void {}, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 520e4c291f7..05e6437b162 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -163,6 +163,7 @@ module.exports = function( // down its children. Instead, we'll get insertions from each child in // the portal directly. } else if (node.child !== null) { + node.child.return = node; node = node.child; continue; } @@ -175,6 +176,7 @@ module.exports = function( } node = node.return; } + node.sibling.return = node.return; node = node.sibling; } } @@ -241,6 +243,7 @@ module.exports = function( // the portal directly. } else if (node.child !== null) { node = node.child; + node.child.return = node; continue; } if (node === workInProgress) { @@ -252,6 +255,7 @@ module.exports = function( } node = node.return; } + node.sibling.return = node.return; node = node.sibling; } }; diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js index 1ccab9571f0..01316a97919 100644 --- a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js @@ -64,4 +64,41 @@ describe('ReactPersistent', () => { expect(originalChildren).toEqual([div(span())]); }); + + it('can reuse child nodes between updates', () => { + function Baz(props) { + return ; + } + class Bar extends React.Component { + shouldComponentUpdate(newProps) { + return false; + } + render() { + return ; + } + } + function Foo(props) { + return ( +
+ + {props.text === 'World' ? : null} +
+ ); + } + + render(); + ReactNoop.flush(); + var originalChildren = getChildren(); + expect(originalChildren).toEqual([div(span('Hello'))]); + + render(); + ReactNoop.flush(); + var newChildren = getChildren(); + expect(newChildren).toEqual([div(span('Hello'), span('World'))]); + + expect(originalChildren).toEqual([div(span('Hello'))]); + + // Reused node should have reference equality + expect(newChildren[0].children[0]).toBe(originalChildren[0].children[0]); + }); }); From 9b90f549928d6cc79b45078c4b1c4d9b7d5c03eb Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 17 Oct 2017 23:35:49 -0700 Subject: [PATCH 12/22] Test persistent text nodes Found another bug. --- .../src/ReactFiberCommitWork.js | 3 +++ .../fiber/__tests__/ReactPersistent-test.js | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 4afbc2ee425..72fa5cbf2e6 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -288,6 +288,9 @@ module.exports = function( case HostComponent: { return; } + case HostText: { + return; + } case HostRoot: case HostPortal: { const portalOrRoot: {containerInfo: C, pendingContainerInfo: C} = diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js index 01316a97919..a8b5f8a5c73 100644 --- a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js @@ -101,4 +101,27 @@ describe('ReactPersistent', () => { // Reused node should have reference equality expect(newChildren[0].children[0]).toBe(originalChildren[0].children[0]); }); + + it('can update child text nodes', () => { + function Foo(props) { + return ( +
+ {props.text} + +
+ ); + } + + render(); + ReactNoop.flush(); + var originalChildren = getChildren(); + expect(originalChildren).toEqual([div('Hello', span())]); + + render(); + ReactNoop.flush(); + var newChildren = getChildren(); + expect(newChildren).toEqual([div('World', span())]); + + expect(originalChildren).toEqual([div('Hello', span())]); + }); }); From fc963ad3f35b68385f35e96b4c01285d5ac17c53 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 00:51:13 -0700 Subject: [PATCH 13/22] Add persistent portal test This creates a bit of an unfortunate feature testing in the unmount branch. That's because we want to trigger nested host deletions in portals in the mutation mode. --- .../src/ReactFiberCommitWork.js | 14 +++++- .../src/ReactFiberCompleteWork.js | 2 +- .../fiber/__tests__/ReactPersistent-test.js | 46 +++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 72fa5cbf2e6..8bafd5d9d5f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -162,6 +162,10 @@ module.exports = function( // We have no life-cycles associated with text. return; } + case HostPortal: { + // We have no life-cycles associated with portals. + return; + } default: { invariant( false, @@ -222,7 +226,9 @@ module.exports = function( // TODO: this is recursive. // We are also not using this parent because // the portal will get pushed immediately. - unmountHostComponents(current); + if (config.mutation) { + unmountHostComponents(current); + } return; } } @@ -239,7 +245,11 @@ module.exports = function( commitUnmount(node); // Visit children because they may contain more composite or host nodes. // Skip portals because commitUnmount() currently visits them recursively. - if (node.child !== null && node.tag !== HostPortal) { + if ( + node.child !== null && + // Drill down into portals only if we use mutation since that branch is recursive + (!config.mutation || node.tag !== HostPortal) + ) { node.child.return = node; node = node.child; continue; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 05e6437b162..a5029be9945 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -242,8 +242,8 @@ module.exports = function( // down its children. Instead, we'll get insertions from each child in // the portal directly. } else if (node.child !== null) { - node = node.child; node.child.return = node; + node = node.child; continue; } if (node === workInProgress) { diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js index a8b5f8a5c73..e7b6c6bc0b9 100644 --- a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js @@ -11,12 +11,14 @@ var React; var ReactNoop; +let ReactPortal; describe('ReactPersistent', () => { beforeEach(() => { jest.resetModules(); React = require('react'); ReactNoop = require('react-noop-renderer'); + ReactPortal = require('ReactPortal'); }); const DEFAULT_ROOT_ID = 'persistent-test'; @@ -124,4 +126,48 @@ describe('ReactPersistent', () => { expect(originalChildren).toEqual([div('Hello', span())]); }); + + it('supports portals', () => { + function Parent(props) { + return
{props.children}
; + } + + function Child(props) { + return
{props.children}
; + } + const portalID = 'persistent-portal-test'; + const portalContainer = {rootID: portalID, children: []}; + render( + + {ReactPortal.createPortal(, portalContainer, null)} + , + ); + ReactNoop.flush(); + + expect(portalContainer.children).toEqual([]); + + var originalChildren = getChildren(); + expect(originalChildren).toEqual([div()]); + var originalPortalChildren = ReactNoop.getChildren(portalID); + expect(originalPortalChildren).toEqual([div()]); + + render( + + {ReactPortal.createPortal( + Hello {'World'}, + portalContainer, + null, + )} + , + ); + ReactNoop.flush(); + + var newChildren = getChildren(); + expect(newChildren).toEqual([div()]); + var newPortalChildren = ReactNoop.getChildren(portalID); + expect(newPortalChildren).toEqual([div('Hello ', 'World')]); + + expect(originalChildren).toEqual([div()]); + expect(originalPortalChildren).toEqual([div()]); + }); }); From c3d6ee9455a7a1e9ffa655de3a15d24f7455fc28 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 01:53:34 -0700 Subject: [PATCH 14/22] Don't consider container when determining portal identity Basically, we can't use the container to determine if we should keep identity and update an existing portal instead of recreate it. Because for persistent containers, there is no permanent identity. This makes it kind of strange to even use portals in this mode. It's probably more ideal to have another concept that has permanent identity rather than trying to swap out containers. --- .../react-reconciler/src/ReactChildFiber.js | 6 +++-- .../fiber/__tests__/ReactPersistent-test.js | 26 ++++++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 935b897b3ca..b31a28d3a2e 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -458,7 +458,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if ( current === null || current.tag !== HostPortal || - current.stateNode.containerInfo !== portal.containerInfo || + // Persistent containers don't have permanent identity. + // current.stateNode.containerInfo !== portal.containerInfo || current.stateNode.implementation !== portal.implementation ) { // Insert @@ -1315,7 +1316,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (child.key === key) { if ( child.tag === HostPortal && - child.stateNode.containerInfo === portal.containerInfo && + // Persistent containers don't have permanent identity. + // child.stateNode.containerInfo === portal.containerInfo && child.stateNode.implementation === portal.implementation ) { deleteRemainingChildren(returnFiber, child.sibling); diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js index e7b6c6bc0b9..e910e7364e6 100644 --- a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js @@ -132,8 +132,21 @@ describe('ReactPersistent', () => { return
{props.children}
; } + function BailoutSpan() { + return ; + } + + class BailoutTest extends React.Component { + shouldComponentUpdate() { + return false; + } + render() { + return ; + } + } + function Child(props) { - return
{props.children}
; + return
{props.children}
; } const portalID = 'persistent-portal-test'; const portalContainer = {rootID: portalID, children: []}; @@ -149,7 +162,7 @@ describe('ReactPersistent', () => { var originalChildren = getChildren(); expect(originalChildren).toEqual([div()]); var originalPortalChildren = ReactNoop.getChildren(portalID); - expect(originalPortalChildren).toEqual([div()]); + expect(originalPortalChildren).toEqual([div(span())]); render( @@ -165,9 +178,14 @@ describe('ReactPersistent', () => { var newChildren = getChildren(); expect(newChildren).toEqual([div()]); var newPortalChildren = ReactNoop.getChildren(portalID); - expect(newPortalChildren).toEqual([div('Hello ', 'World')]); + expect(newPortalChildren).toEqual([div(span(), 'Hello ', 'World')]); expect(originalChildren).toEqual([div()]); - expect(originalPortalChildren).toEqual([div()]); + expect(originalPortalChildren).toEqual([div(span())]); + + // Reused portal children should have reference equality + expect(newPortalChildren[0].children[0]).toBe( + originalPortalChildren[0].children[0], + ); }); }); From a6b2638a8bd97b487a00986a846c1b17f9fe5251 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 02:03:29 -0700 Subject: [PATCH 15/22] Clear portals when the portal is deleted When a portal gets deleted we need to create a new empty container and replace the current one with the empty one. --- .../react-reconciler/src/ReactFiberCommitWork.js | 14 +++++++++++++- .../shared/fiber/__tests__/ReactPersistent-test.js | 10 ++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 8bafd5d9d5f..67036109246 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -228,6 +228,8 @@ module.exports = function( // the portal will get pushed immediately. if (config.mutation) { unmountHostComponents(current); + } else if (config.persistence) { + emptyPortalContainer(current); } return; } @@ -289,7 +291,17 @@ module.exports = function( // Noop }; } else { - const {replaceContainer} = config.persistence; + const {replaceContainer, cloneContainer} = config.persistence; + var emptyPortalContainer = function(current: Fiber) { + const portal: {containerInfo: C, pendingContainerInfo: C} = + current.stateNode; + const {containerInfo, pendingContainerInfo} = portal; + const emptyContainer = cloneContainer( + containerInfo, + pendingContainerInfo, + ); + replaceContainer(containerInfo, emptyContainer); + }; commitContainer = function(finishedWork: Fiber) { switch (finishedWork.tag) { case ClassComponent: { diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js index e910e7364e6..ba11d649a2c 100644 --- a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js @@ -187,5 +187,15 @@ describe('ReactPersistent', () => { expect(newPortalChildren[0].children[0]).toBe( originalPortalChildren[0].children[0], ); + + // Deleting the Portal, should clear its children + render(); + ReactNoop.flush(); + + var clearedPortalChildren = ReactNoop.getChildren(portalID); + expect(clearedPortalChildren).toEqual([]); + + // The original is unchanged. + expect(newPortalChildren).toEqual([div(span(), 'Hello ', 'World')]); }); }); From 5d461e00a9a400e0d28b558e4b6c3c614f0c249b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 18 Oct 2017 21:45:21 +0100 Subject: [PATCH 16/22] Add renderer mode flags for dead code elimination --- .../src/__tests__/ReactNativeCS-test.js | 2 + .../react-noop-renderer/src/ReactNoopEntry.js | 196 +++++---- .../src/ReactFiberCommitWork.js | 90 +++-- .../src/ReactFiberCompleteWork.js | 377 +++++++++--------- .../shared/src/utils/ReactFeatureFlags.js | 9 + scripts/rollup/bundles.js | 1 + .../native-cs/ReactNativeCSFeatureFlags.js | 24 ++ .../fiber/__tests__/ReactPersistent-test.js | 6 + 8 files changed, 404 insertions(+), 301 deletions(-) create mode 100644 src/renderers/native-cs/ReactNativeCSFeatureFlags.js diff --git a/packages/react-cs-renderer/src/__tests__/ReactNativeCS-test.js b/packages/react-cs-renderer/src/__tests__/ReactNativeCS-test.js index f1ad224d6e5..3fa811fb255 100644 --- a/packages/react-cs-renderer/src/__tests__/ReactNativeCS-test.js +++ b/packages/react-cs-renderer/src/__tests__/ReactNativeCS-test.js @@ -12,6 +12,8 @@ var React; var ReactNativeCS; +jest.mock('ReactFeatureFlags', () => require('ReactNativeCSFeatureFlags')); + describe('ReactNativeCS', () => { beforeEach(() => { jest.resetModules(); diff --git a/packages/react-noop-renderer/src/ReactNoopEntry.js b/packages/react-noop-renderer/src/ReactNoopEntry.js index cba0c28e0ae..7e67fcccb25 100644 --- a/packages/react-noop-renderer/src/ReactNoopEntry.js +++ b/packages/react-noop-renderer/src/ReactNoopEntry.js @@ -178,89 +178,105 @@ var SharedHostConfig = { }, }; -var NoopRenderer = ReactFiberReconciler({ - ...SharedHostConfig, - mutation: { - commitMount(instance: Instance, type: string, newProps: Props): void { - // Noop - }, - - commitUpdate( - instance: Instance, - updatePayload: Object, - type: string, - oldProps: Props, - newProps: Props, - ): void { - instance.prop = newProps.prop; - }, - - commitTextUpdate( - textInstance: TextInstance, - oldText: string, - newText: string, - ): void { - textInstance.text = newText; - }, - - appendChild: appendChild, - appendChildToContainer: appendChild, - insertBefore: insertBefore, - insertInContainerBefore: insertBefore, - removeChild: removeChild, - removeChildFromContainer: removeChild, - - resetTextContent(instance: Instance): void {}, - }, -}); - -var PersistentNoopRenderer = ReactFiberReconciler({ - ...SharedHostConfig, - persistence: { - cloneInstance( - instance: Instance, - updatePayload: null | Object, - type: string, - oldProps: Props, - newProps: Props, - internalInstanceHandle: Object, - keepChildren: boolean, - recyclableInstance: null | Instance, - ): Instance { - const clone = { - id: instance.id, - type: type, - children: keepChildren ? instance.children : [], - prop: newProps.prop, - }; - Object.defineProperty(clone, 'id', { - value: clone.id, - enumerable: false, - }); - return clone; - }, +var MutationHostConfig = { + commitMount(instance: Instance, type: string, newProps: Props): void { + // Noop + }, + + commitUpdate( + instance: Instance, + updatePayload: Object, + type: string, + oldProps: Props, + newProps: Props, + ): void { + instance.prop = newProps.prop; + }, + + commitTextUpdate( + textInstance: TextInstance, + oldText: string, + newText: string, + ): void { + textInstance.text = newText; + }, + + appendChild: appendChild, + appendChildToContainer: appendChild, + insertBefore: insertBefore, + insertInContainerBefore: insertBefore, + removeChild: removeChild, + removeChildFromContainer: removeChild, + + resetTextContent(instance: Instance): void {}, +}; + +var PersistenceHostConfig = { + cloneInstance( + instance: Instance, + updatePayload: null | Object, + type: string, + oldProps: Props, + newProps: Props, + internalInstanceHandle: Object, + keepChildren: boolean, + recyclableInstance: null | Instance, + ): Instance { + const clone = { + id: instance.id, + type: type, + children: keepChildren ? instance.children : [], + prop: newProps.prop, + }; + Object.defineProperty(clone, 'id', { + value: clone.id, + enumerable: false, + }); + return clone; + }, - cloneContainer( - container: Container, - recyclableContainer: Container, - ): Container { - return {rootID: container.rootID, children: []}; - }, + cloneContainer( + container: Container, + recyclableContainer: Container, + ): Container { + return {rootID: container.rootID, children: []}; + }, - appendInititalChildToContainer( - parentInstance: Container, - child: Instance | TextInstance, - ) { - parentInstance.children.push(child); - }, + appendInititalChildToContainer( + parentInstance: Container, + child: Instance | TextInstance, + ) { + parentInstance.children.push(child); + }, - finalizeContainerChildren(container: Container): void {}, + finalizeContainerChildren(container: Container): void {}, - replaceContainer(oldContainer: Container, newContainer: Container): void { - rootContainers.set(oldContainer.rootID, newContainer); - }, + replaceContainer(oldContainer: Container, newContainer: Container): void { + rootContainers.set(oldContainer.rootID, newContainer); }, -}); +}; + +// They are created lazily because only one can be created per test file. +var renderer = null; +var persistentRenderer = null; +function getRenderer() { + return ( + renderer || + (renderer = ReactFiberReconciler({ + ...SharedHostConfig, + mutation: MutationHostConfig, + })) + ); +} +function getPersistentRenderer() { + return ( + persistentRenderer || + (persistentRenderer = ReactFiberReconciler({ + ...SharedHostConfig, + persistence: PersistenceHostConfig, + })) + ); +} var rootContainers = new Map(); var roots = new Map(); @@ -317,6 +333,7 @@ var ReactNoop = { rootID: string, callback: ?Function, ) { + const NoopRenderer = getRenderer(); let root = roots.get(rootID); if (!root) { const container = {rootID: rootID, children: []}; @@ -332,6 +349,7 @@ var ReactNoop = { rootID: string, callback: ?Function, ) { + const PersistentNoopRenderer = getPersistentRenderer(); let root = persistentRoots.get(rootID); if (!root) { const container = {rootID: rootID, children: []}; @@ -343,6 +361,7 @@ var ReactNoop = { }, unmountRootWithID(rootID: string) { + const NoopRenderer = getRenderer(); const root = roots.get(rootID); if (root) { NoopRenderer.updateContainer(null, root, null, () => { @@ -355,6 +374,7 @@ var ReactNoop = { findInstance( componentOrElement: Element | ?React$Component, ): null | Instance | TextInstance { + const NoopRenderer = getRenderer(); if (componentOrElement == null) { return null; } @@ -431,13 +451,25 @@ var ReactNoop = { return !!scheduledCallback; }, - batchedUpdates: NoopRenderer.batchedUpdates, + batchedUpdates(...args: Array) { + const NoopRenderer = getRenderer(); + return NoopRenderer.batchedUpdates(...args); + }, - deferredUpdates: NoopRenderer.deferredUpdates, + deferredUpdates(...args: Array) { + const NoopRenderer = getRenderer(); + return NoopRenderer.deferredUpdates(...args); + }, - unbatchedUpdates: NoopRenderer.unbatchedUpdates, + unbatchedUpdates(...args: Array) { + const NoopRenderer = getRenderer(); + return NoopRenderer.unbatchedUpdates(...args); + }, - flushSync: NoopRenderer.flushSync, + flushSync(...args: Array) { + const NoopRenderer = getRenderer(); + return NoopRenderer.flushSync(...args); + }, // Logs the current state of the tree. dumpTree(rootID: string = DEFAULT_ROOT_ID) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 67036109246..7a9b8d96a9b 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -13,6 +13,7 @@ import type {Fiber} from 'ReactFiber'; import type {HostConfig} from 'ReactFiberReconciler'; +var ReactFeatureFlags = require('ReactFeatureFlags'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { ClassComponent, @@ -42,7 +43,7 @@ module.exports = function( config: HostConfig, captureError: (failedFiber: Fiber, error: mixed) => Fiber | null, ) { - const {getPublicInstance} = config; + const {getPublicInstance, mutation, persistence} = config; if (__DEV__) { var callComponentWillUnmountWithTimerInDev = function(current, instance) { @@ -226,9 +227,12 @@ module.exports = function( // TODO: this is recursive. // We are also not using this parent because // the portal will get pushed immediately. - if (config.mutation) { + if (ReactFeatureFlags.enableMutatingReconciler && mutation) { unmountHostComponents(current); - } else if (config.persistence) { + } else if ( + ReactFeatureFlags.enablePersistentReconciler && + persistence + ) { emptyPortalContainer(current); } return; @@ -250,7 +254,7 @@ module.exports = function( if ( node.child !== null && // Drill down into portals only if we use mutation since that branch is recursive - (!config.mutation || node.tag !== HostPortal) + (!mutation || node.tag !== HostPortal) ) { node.child.return = node; node = node.child; @@ -284,14 +288,10 @@ module.exports = function( } } - if (!config.mutation) { + if (!mutation) { let commitContainer; - if (!config.persistence) { - commitContainer = function(finishedWork: Fiber) { - // Noop - }; - } else { - const {replaceContainer, cloneContainer} = config.persistence; + if (persistence) { + const {replaceContainer, cloneContainer} = persistence; var emptyPortalContainer = function(current: Fiber) { const portal: {containerInfo: C, pendingContainerInfo: C} = current.stateNode; @@ -334,24 +334,36 @@ module.exports = function( } } }; + } else { + commitContainer = function(finishedWork: Fiber) { + // Noop + }; + } + if ( + ReactFeatureFlags.enablePersistentReconciler || + ReactFeatureFlags.enableNoopReconciler + ) { + return { + commitResetTextContent(finishedWork: Fiber) {}, + commitPlacement(finishedWork: Fiber) {}, + commitDeletion(current: Fiber) { + // Detach refs and call componentWillUnmount() on the whole subtree. + commitNestedUnmounts(current); + detachFiber(current); + }, + commitWork(current: Fiber | null, finishedWork: Fiber) { + commitContainer(finishedWork); + }, + commitLifeCycles, + commitAttachRef, + commitDetachRef, + }; + } else if (persistence) { + invariant(false, 'Persistent reconciler is disabled.'); + } else { + invariant(false, 'Noop reconciler is disabled.'); } - return { - commitResetTextContent(finishedWork: Fiber) {}, - commitPlacement(finishedWork: Fiber) {}, - commitDeletion(current: Fiber) { - // Detach refs and call componentWillUnmount() on the whole subtree. - commitNestedUnmounts(current); - detachFiber(current); - }, - commitWork(current: Fiber | null, finishedWork: Fiber) { - commitContainer(finishedWork); - }, - commitLifeCycles, - commitAttachRef, - commitDetachRef, - }; } - const { commitMount, commitUpdate, @@ -363,7 +375,7 @@ module.exports = function( insertInContainerBefore, removeChild, removeChildFromContainer, - } = config.mutation; + } = mutation; function getHostParentFiber(fiber: Fiber): Fiber { let parent = fiber.return; @@ -663,13 +675,17 @@ module.exports = function( resetTextContent(current.stateNode); } - return { - commitResetTextContent, - commitPlacement, - commitDeletion, - commitWork, - commitLifeCycles, - commitAttachRef, - commitDetachRef, - }; + if (ReactFeatureFlags.enableMutatingReconciler) { + return { + commitResetTextContent, + commitPlacement, + commitDeletion, + commitWork, + commitLifeCycles, + commitAttachRef, + commitDetachRef, + }; + } else { + invariant(false, 'Mutating reconciler is disabled.'); + } }; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index a5029be9945..b39d8594d5f 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -23,6 +23,7 @@ var { popContextProvider, popTopLevelContextObject, } = require('ReactFiberContext'); +var ReactFeatureFlags = require('ReactFeatureFlags'); var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); var ReactFiberExpirationTime = require('ReactFiberExpirationTime'); @@ -185,202 +186,214 @@ module.exports = function( let updateHostComponent; let updateHostText; if (mutation) { - // Mutation mode - updateHostContainer = function(workInProgress: Fiber) { - // Noop - }; - updateHostComponent = function( - current: Fiber, - workInProgress: Fiber, - updatePayload: null | PL, - type: T, - oldProps: P, - newProps: P, - rootContainerInstance: C, - ) { - // TODO: Type this specific to this type of component. - workInProgress.updateQueue = (updatePayload: any); - // If the update payload indicates that there is a change or if there - // is a new ref we mark this as an update. All the work is done in commitWork. - if (updatePayload) { - markUpdate(workInProgress); - } - }; - updateHostText = function( - current: Fiber, - workInProgress: Fiber, - oldText: string, - newText: string, - ) { - // If the text differs, mark it as an update. All the work in done in commitWork. - if (oldText !== newText) { - markUpdate(workInProgress); - } - }; - } else if (persistence) { - // Persistent host tree mode - const { - cloneInstance, - cloneContainer, - appendInititalChildToContainer, - finalizeContainerChildren, - } = persistence; - - // An unfortunate fork of appendAllChildren because we have two different parent types. - const appendAllChildrenToContainer = function( - container: C, - workInProgress: Fiber, - ) { - // We only have the top Fiber that was created but we need recurse down its - // children to find all the terminal nodes. - let node = workInProgress.child; - while (node !== null) { - if (node.tag === HostComponent || node.tag === HostText) { - appendInititalChildToContainer(container, node.stateNode); - } else if (node.tag === HostPortal) { - // If we have a portal child, then we don't want to traverse - // down its children. Instead, we'll get insertions from each child in - // the portal directly. - } else if (node.child !== null) { - node.child.return = node; - node = node.child; - continue; + if (ReactFeatureFlags.enableMutatingReconciler) { + // Mutation mode + updateHostContainer = function(workInProgress: Fiber) { + // Noop + }; + updateHostComponent = function( + current: Fiber, + workInProgress: Fiber, + updatePayload: null | PL, + type: T, + oldProps: P, + newProps: P, + rootContainerInstance: C, + ) { + // TODO: Type this specific to this type of component. + workInProgress.updateQueue = (updatePayload: any); + // If the update payload indicates that there is a change or if there + // is a new ref we mark this as an update. All the work is done in commitWork. + if (updatePayload) { + markUpdate(workInProgress); } - if (node === workInProgress) { - return; + }; + updateHostText = function( + current: Fiber, + workInProgress: Fiber, + oldText: string, + newText: string, + ) { + // If the text differs, mark it as an update. All the work in done in commitWork. + if (oldText !== newText) { + markUpdate(workInProgress); } - while (node.sibling === null) { - if (node.return === null || node.return === workInProgress) { + }; + } else { + invariant(false, 'Mutating reconciler is disabled.'); + } + } else if (persistence) { + if (ReactFeatureFlags.enablePersistentReconciler) { + // Persistent host tree mode + const { + cloneInstance, + cloneContainer, + appendInititalChildToContainer, + finalizeContainerChildren, + } = persistence; + + // An unfortunate fork of appendAllChildren because we have two different parent types. + const appendAllChildrenToContainer = function( + container: C, + workInProgress: Fiber, + ) { + // We only have the top Fiber that was created but we need recurse down its + // children to find all the terminal nodes. + let node = workInProgress.child; + while (node !== null) { + if (node.tag === HostComponent || node.tag === HostText) { + appendInititalChildToContainer(container, node.stateNode); + } else if (node.tag === HostPortal) { + // If we have a portal child, then we don't want to traverse + // down its children. Instead, we'll get insertions from each child in + // the portal directly. + } else if (node.child !== null) { + node.child.return = node; + node = node.child; + continue; + } + if (node === workInProgress) { return; } - node = node.return; + while (node.sibling === null) { + if (node.return === null || node.return === workInProgress) { + return; + } + node = node.return; + } + node.sibling.return = node.return; + node = node.sibling; } - node.sibling.return = node.return; - node = node.sibling; - } - }; - updateHostContainer = function(workInProgress: Fiber) { - const portalOrRoot: {containerInfo: C, pendingContainerInfo: C} = - workInProgress.stateNode; - const currentContainer = portalOrRoot.containerInfo; - const recyclableContainer = portalOrRoot.pendingContainerInfo; + }; + updateHostContainer = function(workInProgress: Fiber) { + const portalOrRoot: {containerInfo: C, pendingContainerInfo: C} = + workInProgress.stateNode; + const currentContainer = portalOrRoot.containerInfo; + const recyclableContainer = portalOrRoot.pendingContainerInfo; - const childrenUnchanged = workInProgress.firstEffect === null; - if (childrenUnchanged) { - // No changes, just reuse the existing instance. - // Note that this might release a previous clone. - portalOrRoot.pendingContainerInfo = currentContainer; - } else { - let newContainer = cloneContainer( - currentContainer, - recyclableContainer, - ); - if (finalizeContainerChildren(newContainer)) { + const childrenUnchanged = workInProgress.firstEffect === null; + if (childrenUnchanged) { + // No changes, just reuse the existing instance. + // Note that this might release a previous clone. + portalOrRoot.pendingContainerInfo = currentContainer; + } else { + let newContainer = cloneContainer( + currentContainer, + recyclableContainer, + ); + if (finalizeContainerChildren(newContainer)) { + markUpdate(workInProgress); + } + portalOrRoot.pendingContainerInfo = newContainer; + // If children might have changed, we have to add them all to the set. + appendAllChildrenToContainer(newContainer, workInProgress); + // Schedule an update on the container to swap out the container. markUpdate(workInProgress); } - portalOrRoot.pendingContainerInfo = newContainer; - // If children might have changed, we have to add them all to the set. - appendAllChildrenToContainer(newContainer, workInProgress); - // Schedule an update on the container to swap out the container. - markUpdate(workInProgress); - } - }; - updateHostComponent = function( - current: Fiber, - workInProgress: Fiber, - updatePayload: null | PL, - type: T, - oldProps: P, - newProps: P, - rootContainerInstance: C, - ) { - // If there are no effects associated with this node, then none of our children had any updates. - // This guarantees that we can reuse all of them. - const childrenUnchanged = workInProgress.firstEffect === null; - const currentInstance = current.stateNode; - if (childrenUnchanged && updatePayload === null) { - // No changes, just reuse the existing instance. - // Note that this might release a previous clone. - workInProgress.stateNode = currentInstance; - } else { - let recyclableInstance = workInProgress.stateNode; - let newInstance = cloneInstance( - currentInstance, - updatePayload, - type, - oldProps, - newProps, - workInProgress, - childrenUnchanged, - recyclableInstance, - ); - if ( - finalizeInitialChildren( - newInstance, + }; + updateHostComponent = function( + current: Fiber, + workInProgress: Fiber, + updatePayload: null | PL, + type: T, + oldProps: P, + newProps: P, + rootContainerInstance: C, + ) { + // If there are no effects associated with this node, then none of our children had any updates. + // This guarantees that we can reuse all of them. + const childrenUnchanged = workInProgress.firstEffect === null; + const currentInstance = current.stateNode; + if (childrenUnchanged && updatePayload === null) { + // No changes, just reuse the existing instance. + // Note that this might release a previous clone. + workInProgress.stateNode = currentInstance; + } else { + let recyclableInstance = workInProgress.stateNode; + let newInstance = cloneInstance( + currentInstance, + updatePayload, type, + oldProps, newProps, - rootContainerInstance, - ) - ) { - markUpdate(workInProgress); + workInProgress, + childrenUnchanged, + recyclableInstance, + ); + if ( + finalizeInitialChildren( + newInstance, + type, + newProps, + rootContainerInstance, + ) + ) { + markUpdate(workInProgress); + } + workInProgress.stateNode = newInstance; + if (childrenUnchanged) { + // If there are no other effects in this tree, we need to flag this node as having one. + // Even though we're not going to use it for anything. + // Otherwise parents won't know that there are new children to propagate upwards. + markUpdate(workInProgress); + } else { + // If children might have changed, we have to add them all to the set. + appendAllChildren(newInstance, workInProgress); + } } - workInProgress.stateNode = newInstance; - if (childrenUnchanged) { - // If there are no other effects in this tree, we need to flag this node as having one. - // Even though we're not going to use it for anything. - // Otherwise parents won't know that there are new children to propagate upwards. + }; + updateHostText = function( + current: Fiber, + workInProgress: Fiber, + oldText: string, + newText: string, + ) { + if (oldText !== newText) { + // If the text content differs, we'll create a new text instance for it. + const rootContainerInstance = getRootHostContainer(); + const currentHostContext = getHostContext(); + workInProgress.stateNode = createTextInstance( + newText, + rootContainerInstance, + currentHostContext, + workInProgress, + ); + // We'll have to mark it as having an effect, even though we won't use the effect for anything. + // This lets the parents know that at least one of their children has changed. markUpdate(workInProgress); - } else { - // If children might have changed, we have to add them all to the set. - appendAllChildren(newInstance, workInProgress); } - } - }; - updateHostText = function( - current: Fiber, - workInProgress: Fiber, - oldText: string, - newText: string, - ) { - if (oldText !== newText) { - // If the text content differs, we'll create a new text instance for it. - const rootContainerInstance = getRootHostContainer(); - const currentHostContext = getHostContext(); - workInProgress.stateNode = createTextInstance( - newText, - rootContainerInstance, - currentHostContext, - workInProgress, - ); - // We'll have to mark it as having an effect, even though we won't use the effect for anything. - // This lets the parents know that at least one of their children has changed. - markUpdate(workInProgress); - } - }; + }; + } else { + invariant(false, 'Persistent reconciler is disabled.'); + } } else { - // No host operations - updateHostContainer = function(workInProgress: Fiber) { - // Noop - }; - updateHostComponent = function( - current: Fiber, - workInProgress: Fiber, - updatePayload: null | PL, - type: T, - oldProps: P, - newProps: P, - rootContainerInstance: C, - ) { - // Noop - }; - updateHostText = function( - current: Fiber, - workInProgress: Fiber, - oldText: string, - newText: string, - ) { - // Noop - }; + if (ReactFeatureFlags.enableNoopReconciler) { + // No host operations + updateHostContainer = function(workInProgress: Fiber) { + // Noop + }; + updateHostComponent = function( + current: Fiber, + workInProgress: Fiber, + updatePayload: null | PL, + type: T, + oldProps: P, + newProps: P, + rootContainerInstance: C, + ) { + // Noop + }; + updateHostText = function( + current: Fiber, + workInProgress: Fiber, + oldText: string, + newText: string, + ) { + // Noop + }; + } else { + invariant(false, 'Noop reconciler is disabled.'); + } } function completeWork( diff --git a/packages/shared/src/utils/ReactFeatureFlags.js b/packages/shared/src/utils/ReactFeatureFlags.js index d60ef12f551..6291d1054ab 100644 --- a/packages/shared/src/utils/ReactFeatureFlags.js +++ b/packages/shared/src/utils/ReactFeatureFlags.js @@ -13,11 +13,20 @@ export type FeatureFlags = {| enableAsyncSubtreeAPI: boolean, enableAsyncSchedulingByDefaultInReactDOM: boolean, + enableMutatingReconciler: boolean, + enableNoopReconciler: boolean, + enablePersistentReconciler: boolean, |}; var ReactFeatureFlags: FeatureFlags = { enableAsyncSubtreeAPI: true, enableAsyncSchedulingByDefaultInReactDOM: false, + // Mutating mode (React DOM, React ART, React Native): + enableMutatingReconciler: true, + // Experimental noop mode (currently unused): + enableNoopReconciler: false, + // Experimental persistent mode (CS): + enablePersistentReconciler: false, }; if (__DEV__) { diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index 01581f73abc..f735afc9954 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -323,6 +323,7 @@ const bundles = [ label: 'native-cs-fiber', manglePropertiesOnProd: false, name: 'react-native-cs-renderer', + featureFlags: 'src/renderers/native-cs/ReactNativeCSFeatureFlags', paths: [ 'packages/react-native-renderer/**/*.js', // This is used since we reuse the error dialog code 'packages/react-cs-renderer/**/*.js', diff --git a/src/renderers/native-cs/ReactNativeCSFeatureFlags.js b/src/renderers/native-cs/ReactNativeCSFeatureFlags.js new file mode 100644 index 00000000000..874fdf32560 --- /dev/null +++ b/src/renderers/native-cs/ReactNativeCSFeatureFlags.js @@ -0,0 +1,24 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @providesModule ReactNativeCSFeatureFlags + * @flow + */ + +'use strict'; + +import type {FeatureFlags} from 'ReactFeatureFlags'; + +var ReactNativeCSFeatureFlags: FeatureFlags = { + enableAsyncSubtreeAPI: true, + enableAsyncSchedulingByDefaultInReactDOM: false, + // React Native CS uses persistent reconciler. + enableMutatingReconciler: false, + enableNoopReconciler: false, + enablePersistentReconciler: true, +}; + +module.exports = ReactNativeCSFeatureFlags; diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js index ba11d649a2c..bd2043c7ee3 100644 --- a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js @@ -16,6 +16,12 @@ let ReactPortal; describe('ReactPersistent', () => { beforeEach(() => { jest.resetModules(); + + const ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.enableMutableReconciler = false; + ReactFeatureFlags.enablePersistentReconciler = true; + ReactFeatureFlags.enableNoopReconciler = false; + React = require('react'); ReactNoop = require('react-noop-renderer'); ReactPortal = require('ReactPortal'); From 837a6d97b87f45047be24849d1f069140a2bfecc Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 18 Oct 2017 22:17:13 +0100 Subject: [PATCH 17/22] Simplify ReactNoop fix --- .../react-noop-renderer/src/ReactNoopEntry.js | 215 ++++++++---------- 1 file changed, 97 insertions(+), 118 deletions(-) diff --git a/packages/react-noop-renderer/src/ReactNoopEntry.js b/packages/react-noop-renderer/src/ReactNoopEntry.js index 7e67fcccb25..7d84364a28b 100644 --- a/packages/react-noop-renderer/src/ReactNoopEntry.js +++ b/packages/react-noop-renderer/src/ReactNoopEntry.js @@ -20,6 +20,7 @@ import type {Fiber} from 'ReactFiber'; import type {UpdateQueue} from 'ReactFiberUpdateQueue'; +var ReactFeatureFlags = require('ReactFeatureFlags'); var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); var ReactFiberReconciler = require('ReactFiberReconciler'); var ReactInstanceMap = require('ReactInstanceMap'); @@ -178,105 +179,94 @@ var SharedHostConfig = { }, }; -var MutationHostConfig = { - commitMount(instance: Instance, type: string, newProps: Props): void { - // Noop - }, - - commitUpdate( - instance: Instance, - updatePayload: Object, - type: string, - oldProps: Props, - newProps: Props, - ): void { - instance.prop = newProps.prop; - }, - - commitTextUpdate( - textInstance: TextInstance, - oldText: string, - newText: string, - ): void { - textInstance.text = newText; - }, - - appendChild: appendChild, - appendChildToContainer: appendChild, - insertBefore: insertBefore, - insertInContainerBefore: insertBefore, - removeChild: removeChild, - removeChildFromContainer: removeChild, - - resetTextContent(instance: Instance): void {}, -}; - -var PersistenceHostConfig = { - cloneInstance( - instance: Instance, - updatePayload: null | Object, - type: string, - oldProps: Props, - newProps: Props, - internalInstanceHandle: Object, - keepChildren: boolean, - recyclableInstance: null | Instance, - ): Instance { - const clone = { - id: instance.id, - type: type, - children: keepChildren ? instance.children : [], - prop: newProps.prop, - }; - Object.defineProperty(clone, 'id', { - value: clone.id, - enumerable: false, - }); - return clone; - }, - - cloneContainer( - container: Container, - recyclableContainer: Container, - ): Container { - return {rootID: container.rootID, children: []}; - }, - - appendInititalChildToContainer( - parentInstance: Container, - child: Instance | TextInstance, - ) { - parentInstance.children.push(child); - }, - - finalizeContainerChildren(container: Container): void {}, - - replaceContainer(oldContainer: Container, newContainer: Container): void { - rootContainers.set(oldContainer.rootID, newContainer); - }, -}; - -// They are created lazily because only one can be created per test file. -var renderer = null; -var persistentRenderer = null; -function getRenderer() { - return ( - renderer || - (renderer = ReactFiberReconciler({ +var NoopRenderer = ReactFiberReconciler({ + ...SharedHostConfig, + mutation: { + commitMount(instance: Instance, type: string, newProps: Props): void { + // Noop + }, + + commitUpdate( + instance: Instance, + updatePayload: Object, + type: string, + oldProps: Props, + newProps: Props, + ): void { + instance.prop = newProps.prop; + }, + + commitTextUpdate( + textInstance: TextInstance, + oldText: string, + newText: string, + ): void { + textInstance.text = newText; + }, + + appendChild: appendChild, + appendChildToContainer: appendChild, + insertBefore: insertBefore, + insertInContainerBefore: insertBefore, + removeChild: removeChild, + removeChildFromContainer: removeChild, + + resetTextContent(instance: Instance): void {}, + }, +}); + +var PersistentNoopRenderer = ReactFeatureFlags.enablePersistentReconciler + ? ReactFiberReconciler({ ...SharedHostConfig, - mutation: MutationHostConfig, - })) - ); -} -function getPersistentRenderer() { - return ( - persistentRenderer || - (persistentRenderer = ReactFiberReconciler({ - ...SharedHostConfig, - persistence: PersistenceHostConfig, - })) - ); -} + persistence: { + cloneInstance( + instance: Instance, + updatePayload: null | Object, + type: string, + oldProps: Props, + newProps: Props, + internalInstanceHandle: Object, + keepChildren: boolean, + recyclableInstance: null | Instance, + ): Instance { + const clone = { + id: instance.id, + type: type, + children: keepChildren ? instance.children : [], + prop: newProps.prop, + }; + Object.defineProperty(clone, 'id', { + value: clone.id, + enumerable: false, + }); + return clone; + }, + + cloneContainer( + container: Container, + recyclableContainer: Container, + ): Container { + return {rootID: container.rootID, children: []}; + }, + + appendInititalChildToContainer( + parentInstance: Container, + child: Instance | TextInstance, + ) { + parentInstance.children.push(child); + }, + + finalizeContainerChildren(container: Container): void {}, + + replaceContainer( + oldContainer: Container, + newContainer: Container, + ): void { + rootContainers.set(oldContainer.rootID, newContainer); + }, + }, + }) + : null; var rootContainers = new Map(); var roots = new Map(); @@ -333,7 +323,6 @@ var ReactNoop = { rootID: string, callback: ?Function, ) { - const NoopRenderer = getRenderer(); let root = roots.get(rootID); if (!root) { const container = {rootID: rootID, children: []}; @@ -349,7 +338,11 @@ var ReactNoop = { rootID: string, callback: ?Function, ) { - const PersistentNoopRenderer = getPersistentRenderer(); + if (PersistentNoopRenderer === null) { + throw new Error( + 'Enable ReactFeatureFlags.enablePersistentReconciler to use it in tests.', + ); + } let root = persistentRoots.get(rootID); if (!root) { const container = {rootID: rootID, children: []}; @@ -361,7 +354,6 @@ var ReactNoop = { }, unmountRootWithID(rootID: string) { - const NoopRenderer = getRenderer(); const root = roots.get(rootID); if (root) { NoopRenderer.updateContainer(null, root, null, () => { @@ -374,7 +366,6 @@ var ReactNoop = { findInstance( componentOrElement: Element | ?React$Component, ): null | Instance | TextInstance { - const NoopRenderer = getRenderer(); if (componentOrElement == null) { return null; } @@ -451,25 +442,13 @@ var ReactNoop = { return !!scheduledCallback; }, - batchedUpdates(...args: Array) { - const NoopRenderer = getRenderer(); - return NoopRenderer.batchedUpdates(...args); - }, + batchedUpdates: NoopRenderer.batchedUpdates, - deferredUpdates(...args: Array) { - const NoopRenderer = getRenderer(); - return NoopRenderer.deferredUpdates(...args); - }, + deferredUpdates: NoopRenderer.deferredUpdates, - unbatchedUpdates(...args: Array) { - const NoopRenderer = getRenderer(); - return NoopRenderer.unbatchedUpdates(...args); - }, + unbatchedUpdates: NoopRenderer.unbatchedUpdates, - flushSync(...args: Array) { - const NoopRenderer = getRenderer(); - return NoopRenderer.flushSync(...args); - }, + flushSync: NoopRenderer.flushSync, // Logs the current state of the tree. dumpTree(rootID: string = DEFAULT_ROOT_ID) { From 25638ef21df11b7a8a31ea450183f7923f5f06b0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 14:58:32 -0700 Subject: [PATCH 18/22] Add new type to the host config for persistent configs We need container to stay as the persistent identity of the root atom. So that we can refer to portals over time. Instead, I'll introduce a new type just to temporarily hold the children of a container until they're ready to be committed into the permanent container. Essentially, this is just a fancy array that is not an array so that the host can choose data structure/allocation for it. --- .../src/ReactFiberBeginWork.js | 4 ++-- .../src/ReactFiberCommitWork.js | 4 ++-- .../src/ReactFiberCompleteWork.js | 4 ++-- .../src/ReactFiberHostContext.js | 4 ++-- .../src/ReactFiberHydrationContext.js | 4 ++-- .../src/ReactFiberReconciler.js | 18 +++++++++--------- .../src/ReactFiberScheduler.js | 4 ++-- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 508a41a8622..ff7f992ef60 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -67,8 +67,8 @@ if (__DEV__) { var warnedAboutStatelessRefs = {}; } -module.exports = function( - config: HostConfig, +module.exports = function( + config: HostConfig, hostContext: HostContext, hydrationContext: HydrationContext, scheduleWork: (fiber: Fiber, expirationTime: ExpirationTime) => void, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 7a9b8d96a9b..2caf7b1c3d3 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -39,8 +39,8 @@ if (__DEV__) { var {startPhaseTimer, stopPhaseTimer} = require('ReactDebugFiberPerf'); } -module.exports = function( - config: HostConfig, +module.exports = function( + config: HostConfig, captureError: (failedFiber: Fiber, error: mixed) => Fiber | null, ) { const {getPublicInstance, mutation, persistence} = config; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index b39d8594d5f..22d1c004387 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -45,8 +45,8 @@ var {Never} = ReactFiberExpirationTime; var invariant = require('fbjs/lib/invariant'); -module.exports = function( - config: HostConfig, +module.exports = function( + config: HostConfig, hostContext: HostContext, hydrationContext: HydrationContext, ) { diff --git a/packages/react-reconciler/src/ReactFiberHostContext.js b/packages/react-reconciler/src/ReactFiberHostContext.js index dfd7f0e334d..657f60280ea 100644 --- a/packages/react-reconciler/src/ReactFiberHostContext.js +++ b/packages/react-reconciler/src/ReactFiberHostContext.js @@ -31,8 +31,8 @@ export type HostContext = { resetHostContainer(): void, }; -module.exports = function( - config: HostConfig, +module.exports = function( + config: HostConfig, ): HostContext { const {getChildHostContext, getRootHostContext} = config; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index f6024e39c67..d710db06719 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -33,8 +33,8 @@ export type HydrationContext = { popHydrationState(fiber: Fiber): boolean, }; -module.exports = function( - config: HostConfig, +module.exports = function( + config: HostConfig, ): HydrationContext { const {shouldSetTextContent, hydration} = config; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 2fcff4aaa7c..4999d73905f 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -47,7 +47,7 @@ export type Deadline = { type OpaqueHandle = Fiber; type OpaqueRoot = FiberRoot; -export type HostConfig = { +export type HostConfig = { getRootHostContext(rootContainerInstance: C): CX, getChildHostContext(parentHostContext: CX, type: T, instance: C): CX, getPublicInstance(instance: I | TI): PI, @@ -100,7 +100,7 @@ export type HostConfig = { +hydration?: HydrationHostConfig, +mutation?: MutableUpdatesHostConfig, - +persistence?: PersistentUpdatesHostConfig, + +persistence?: PersistentUpdatesHostConfig, }; type MutableUpdatesHostConfig = { @@ -132,7 +132,7 @@ type MutableUpdatesHostConfig = { removeChildFromContainer(container: C, child: I | TI): void, }; -type PersistentUpdatesHostConfig = { +type PersistentUpdatesHostConfig = { cloneInstance( instance: I, updatePayload: null | PL, @@ -144,12 +144,12 @@ type PersistentUpdatesHostConfig = { recyclableInstance: I, ): I, - cloneContainer(container: C, recyclableContainer: C): C, + createContainerChildSet(container: C): CC, - appendInititalChildToContainer(container: C, child: I | TI): void, - finalizeContainerChildren(container: C): void, + appendChildToContainerChildSet(childSet: CC, child: I | TI): void, + finalizeContainerChildren(container: C, newChildren: CC): void, - replaceContainer(oldContainer: C, newContainer: C): void, + replaceContainerChildren(container: C, newChildren: CC): void, }; type HydrationHostConfig = { @@ -253,8 +253,8 @@ function getContextForSubtree( : parentContext; } -module.exports = function( - config: HostConfig, +module.exports = function( + config: HostConfig, ): Reconciler { var {getPublicInstance} = config; diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 50f16402d06..45e335a72f1 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -153,8 +153,8 @@ if (__DEV__) { var timeHeuristicForUnitOfWork = 1; -module.exports = function( - config: HostConfig, +module.exports = function( + config: HostConfig, ) { const hostContext = ReactFiberHostContext(config); const hydrationContext: HydrationContext = ReactFiberHydrationContext( From c9b7e564a977cc236ce642ff655557d6a066099d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 15:41:36 -0700 Subject: [PATCH 19/22] Implement new hooks Now containers are singletons and instead their children swap. That way portals can use the container as part of their identity again. --- .../src/ReactNativeCSFiberEntry.js | 21 ++++++++------ .../react-noop-renderer/src/ReactNoopEntry.js | 28 ++++++++++--------- .../react-reconciler/src/ReactChildFiber.js | 6 ++-- packages/react-reconciler/src/ReactFiber.js | 2 +- .../src/ReactFiberCommitWork.js | 23 ++++++--------- .../src/ReactFiberCompleteWork.js | 27 +++++++----------- .../react-reconciler/src/ReactFiberRoot.js | 4 +-- .../fiber/__tests__/ReactPersistent-test.js | 12 ++++---- 8 files changed, 57 insertions(+), 66 deletions(-) diff --git a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js index fbbfb4548f0..b0cc3bda50c 100644 --- a/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js +++ b/packages/react-cs-renderer/src/ReactNativeCSFiberEntry.js @@ -165,21 +165,26 @@ const ReactNativeCSFiberRenderer = ReactFiberReconciler({ return 0; }, - cloneContainer( + createContainerChildSet( container: Container, - recyclableContainer: Container, - ): Container { - return 0; + ): Array { + return []; }, - appendInititalChildToContainer( - container: Container, + appendChildToContainerChildSet( + childSet: Array, child: Instance | TextInstance, ): void {}, - finalizeContainerChildren(container: Container): void {}, + finalizeContainerChildren( + container: Container, + newChildren: Array, + ): void {}, - replaceContainer(oldContainer: Container, newContainer: Container): void {}, + replaceContainerChildren( + container: Container, + newChildren: Array, + ): void {}, }, }); diff --git a/packages/react-noop-renderer/src/ReactNoopEntry.js b/packages/react-noop-renderer/src/ReactNoopEntry.js index 7d84364a28b..f3c7696bdf7 100644 --- a/packages/react-noop-renderer/src/ReactNoopEntry.js +++ b/packages/react-noop-renderer/src/ReactNoopEntry.js @@ -242,27 +242,29 @@ var PersistentNoopRenderer = ReactFeatureFlags.enablePersistentReconciler return clone; }, - cloneContainer( + createContainerChildSet( container: Container, - recyclableContainer: Container, - ): Container { - return {rootID: container.rootID, children: []}; + ): Array { + return []; }, - appendInititalChildToContainer( - parentInstance: Container, + appendChildToContainerChildSet( + childSet: Array, child: Instance | TextInstance, - ) { - parentInstance.children.push(child); + ): void { + childSet.push(child); }, - finalizeContainerChildren(container: Container): void {}, + finalizeContainerChildren( + container: Container, + newChildren: Array, + ): void {}, - replaceContainer( - oldContainer: Container, - newContainer: Container, + replaceContainerChildren( + container: Container, + newChildren: Array, ): void { - rootContainers.set(oldContainer.rootID, newContainer); + container.children = newChildren; }, }, }) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index b31a28d3a2e..935b897b3ca 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -458,8 +458,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if ( current === null || current.tag !== HostPortal || - // Persistent containers don't have permanent identity. - // current.stateNode.containerInfo !== portal.containerInfo || + current.stateNode.containerInfo !== portal.containerInfo || current.stateNode.implementation !== portal.implementation ) { // Insert @@ -1316,8 +1315,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (child.key === key) { if ( child.tag === HostPortal && - // Persistent containers don't have permanent identity. - // child.stateNode.containerInfo === portal.containerInfo && + child.stateNode.containerInfo === portal.containerInfo && child.stateNode.implementation === portal.implementation ) { deleteRemainingChildren(returnFiber, child.sibling); diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 6c08388990a..18e4a984a3f 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -445,7 +445,7 @@ exports.createFiberFromPortal = function( fiber.expirationTime = expirationTime; fiber.stateNode = { containerInfo: portal.containerInfo, - pendingContainerInfo: portal.containerInfo, // Used by persistent updates + pendingChildren: null, // Used by persistent updates implementation: portal.implementation, }; return fiber; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 2caf7b1c3d3..0f76e74e715 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -291,16 +291,13 @@ module.exports = function( if (!mutation) { let commitContainer; if (persistence) { - const {replaceContainer, cloneContainer} = persistence; + const {replaceContainerChildren, createContainerChildSet} = persistence; var emptyPortalContainer = function(current: Fiber) { - const portal: {containerInfo: C, pendingContainerInfo: C} = + const portal: {containerInfo: C, pendingChildren: CC} = current.stateNode; - const {containerInfo, pendingContainerInfo} = portal; - const emptyContainer = cloneContainer( - containerInfo, - pendingContainerInfo, - ); - replaceContainer(containerInfo, emptyContainer); + const {containerInfo} = portal; + const emptyChildSet = createContainerChildSet(containerInfo); + replaceContainerChildren(containerInfo, emptyChildSet); }; commitContainer = function(finishedWork: Fiber) { switch (finishedWork.tag) { @@ -315,14 +312,10 @@ module.exports = function( } case HostRoot: case HostPortal: { - const portalOrRoot: {containerInfo: C, pendingContainerInfo: C} = + const portalOrRoot: {containerInfo: C, pendingChildren: CC} = finishedWork.stateNode; - const {containerInfo, pendingContainerInfo} = portalOrRoot; - replaceContainer(containerInfo, pendingContainerInfo); - // Swap out the current container. - portalOrRoot.containerInfo = pendingContainerInfo; - // The old one is now free to be recycled. - portalOrRoot.pendingContainerInfo = containerInfo; + const {containerInfo, pendingChildren} = portalOrRoot; + replaceContainerChildren(containerInfo, pendingChildren); return; } default: { diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 22d1c004387..5aff0892c76 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -227,14 +227,14 @@ module.exports = function( // Persistent host tree mode const { cloneInstance, - cloneContainer, - appendInititalChildToContainer, + createContainerChildSet, + appendChildToContainerChildSet, finalizeContainerChildren, } = persistence; // An unfortunate fork of appendAllChildren because we have two different parent types. const appendAllChildrenToContainer = function( - container: C, + containerChildSet: CC, workInProgress: Fiber, ) { // We only have the top Fiber that was created but we need recurse down its @@ -242,7 +242,7 @@ module.exports = function( let node = workInProgress.child; while (node !== null) { if (node.tag === HostComponent || node.tag === HostText) { - appendInititalChildToContainer(container, node.stateNode); + appendChildToContainerChildSet(containerChildSet, node.stateNode); } else if (node.tag === HostPortal) { // If we have a portal child, then we don't want to traverse // down its children. Instead, we'll get insertions from each child in @@ -266,27 +266,20 @@ module.exports = function( } }; updateHostContainer = function(workInProgress: Fiber) { - const portalOrRoot: {containerInfo: C, pendingContainerInfo: C} = + const portalOrRoot: {containerInfo: C, pendingChildren: CC} = workInProgress.stateNode; - const currentContainer = portalOrRoot.containerInfo; - const recyclableContainer = portalOrRoot.pendingContainerInfo; - const childrenUnchanged = workInProgress.firstEffect === null; if (childrenUnchanged) { // No changes, just reuse the existing instance. - // Note that this might release a previous clone. - portalOrRoot.pendingContainerInfo = currentContainer; } else { - let newContainer = cloneContainer( - currentContainer, - recyclableContainer, - ); - if (finalizeContainerChildren(newContainer)) { + const container = portalOrRoot.containerInfo; + let newChildSet = createContainerChildSet(container); + if (finalizeContainerChildren(container, newChildSet)) { markUpdate(workInProgress); } - portalOrRoot.pendingContainerInfo = newContainer; + portalOrRoot.pendingChildren = newChildSet; // If children might have changed, we have to add them all to the set. - appendAllChildrenToContainer(newContainer, workInProgress); + appendAllChildrenToContainer(newChildSet, workInProgress); // Schedule an update on the container to swap out the container. markUpdate(workInProgress); } diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 704cd60b928..1288fee9bb8 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -18,7 +18,7 @@ export type FiberRoot = { // Any additional information from the host associated with this root. containerInfo: any, // Used only by persistent updates. - pendingContainerInfo: any, + pendingChildren: any, // The currently active root fiber. This is the mutable root of the tree. current: Fiber, // Determines if this root has already been added to the schedule for work. @@ -42,7 +42,7 @@ exports.createFiberRoot = function( const root = { current: uninitializedFiber, containerInfo: containerInfo, - pendingContainerInfo: containerInfo, + pendingChildren: null, isScheduled: false, nextScheduledRoot: null, context: null, diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js index bd2043c7ee3..023050b5197 100644 --- a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js @@ -154,8 +154,8 @@ describe('ReactPersistent', () => { function Child(props) { return
{props.children}
; } - const portalID = 'persistent-portal-test'; - const portalContainer = {rootID: portalID, children: []}; + const portalContainer = {rootID: 'persistent-portal-test', children: []}; + const emptyPortalChildSet = portalContainer.children; render( {ReactPortal.createPortal(, portalContainer, null)} @@ -163,11 +163,11 @@ describe('ReactPersistent', () => { ); ReactNoop.flush(); - expect(portalContainer.children).toEqual([]); + expect(emptyPortalChildSet).toEqual([]); var originalChildren = getChildren(); expect(originalChildren).toEqual([div()]); - var originalPortalChildren = ReactNoop.getChildren(portalID); + var originalPortalChildren = portalContainer.children; expect(originalPortalChildren).toEqual([div(span())]); render( @@ -183,7 +183,7 @@ describe('ReactPersistent', () => { var newChildren = getChildren(); expect(newChildren).toEqual([div()]); - var newPortalChildren = ReactNoop.getChildren(portalID); + var newPortalChildren = portalContainer.children; expect(newPortalChildren).toEqual([div(span(), 'Hello ', 'World')]); expect(originalChildren).toEqual([div()]); @@ -198,7 +198,7 @@ describe('ReactPersistent', () => { render(); ReactNoop.flush(); - var clearedPortalChildren = ReactNoop.getChildren(portalID); + var clearedPortalChildren = portalContainer.children; expect(clearedPortalChildren).toEqual([]); // The original is unchanged. From c6ff79c44bee76a925dcd2552e7e11b511c14a3d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 16:00:52 -0700 Subject: [PATCH 20/22] Update build size and error codes --- scripts/error-codes/codes.json | 9 +++- scripts/rollup/results.json | 96 +++++++++++++++++----------------- 2 files changed, 56 insertions(+), 49 deletions(-) diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index cd9a6c83dc8..b4045c0640e 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -233,5 +233,12 @@ "231": "Expected `%s` listener to be a function, instead got a value of `%s` type.", "232": "_processChildContext is not available in React 16+. This likely means you have multiple copies of React and are attempting to nest a React 15 tree inside a React 16 tree using unstable_renderSubtreeIntoContainer, which isn't supported. Try to make sure you have only one copy of React (and ideally, switch to ReactDOM.createPortal).", "233": "Unsupported top level event type \"%s\" dispatched", - "234": "Event cannot be both direct and bubbling: %s" + "234": "Event cannot be both direct and bubbling: %s", + "235": "Persistent reconciler is disabled.", + "236": "Noop reconciler is disabled.", + "237": "Mutating reconciler is disabled.", + "238": "Task updates can only be scheduled as a nested update or inside batchedUpdates. This error is likely caused by a bug in React. Please file an issue.", + "239": "Measure not implemented yet", + "240": "Text components are not supported for now.", + "241": "Text components are not yet supported." } diff --git a/scripts/rollup/results.json b/scripts/rollup/results.json index dc49cb86b8f..6dd455e46c9 100644 --- a/scripts/rollup/results.json +++ b/scripts/rollup/results.json @@ -25,28 +25,28 @@ "gzip": 6709 }, "react-dom.development.js (UMD_DEV)": { - "size": 622611, - "gzip": 143520 + "size": 631933, + "gzip": 144937 }, "react-dom.production.min.js (UMD_PROD)": { - "size": 100874, - "gzip": 31801 + "size": 100474, + "gzip": 31680 }, "react-dom.development.js (NODE_DEV)": { - "size": 584906, - "gzip": 134811 + "size": 594214, + "gzip": 136223 }, "react-dom.production.min.js (NODE_PROD)": { - "size": 105280, - "gzip": 33069 + "size": 107016, + "gzip": 33539 }, "ReactDOMFiber-dev.js (FB_DEV)": { - "size": 581983, - "gzip": 134206 + "size": 591153, + "gzip": 135569 }, "ReactDOMFiber-prod.js (FB_PROD)": { - "size": 413270, - "gzip": 92266 + "size": 421158, + "gzip": 93551 }, "react-dom-test-utils.development.js (NODE_DEV)": { "size": 41688, @@ -113,44 +113,44 @@ "gzip": 6214 }, "react-art.development.js (UMD_DEV)": { - "size": 368952, - "gzip": 81593 + "size": 378274, + "gzip": 83009 }, "react-art.production.min.js (UMD_PROD)": { - "size": 82819, - "gzip": 25682 + "size": 82432, + "gzip": 25592 }, "react-art.development.js (NODE_DEV)": { - "size": 293285, - "gzip": 62391 + "size": 302629, + "gzip": 63820 }, "react-art.production.min.js (NODE_PROD)": { - "size": 52186, - "gzip": 16406 + "size": 53897, + "gzip": 16869 }, "ReactARTFiber-dev.js (FB_DEV)": { - "size": 292026, - "gzip": 62361 + "size": 301232, + "gzip": 63727 }, "ReactARTFiber-prod.js (FB_PROD)": { - "size": 217538, - "gzip": 45158 + "size": 225462, + "gzip": 46449 }, "ReactNativeFiber-dev.js (RN_DEV)": { - "size": 279140, - "gzip": 48459 + "size": 285864, + "gzip": 49285 }, "ReactNativeFiber-prod.js (RN_PROD)": { - "size": 217248, - "gzip": 37638 + "size": 223648, + "gzip": 38454 }, "react-test-renderer.development.js (NODE_DEV)": { - "size": 297008, - "gzip": 62795 + "size": 306317, + "gzip": 64218 }, "ReactTestRendererFiber-dev.js (FB_DEV)": { - "size": 295739, - "gzip": 62764 + "size": 304910, + "gzip": 64134 }, "react-test-renderer-shallow.development.js (NODE_DEV)": { "size": 9370, @@ -161,8 +161,8 @@ "gzip": 2253 }, "react-noop-renderer.development.js (NODE_DEV)": { - "size": 284597, - "gzip": 59713 + "size": 295525, + "gzip": 61405 }, "react-dom-server.development.js (UMD_DEV)": { "size": 120897, @@ -189,16 +189,16 @@ "gzip": 7520 }, "ReactNativeRTFiber-dev.js (RN_DEV)": { - "size": 211043, - "gzip": 35922 + "size": 217767, + "gzip": 36738 }, "ReactNativeRTFiber-prod.js (RN_PROD)": { - "size": 158926, - "gzip": 26650 + "size": 165326, + "gzip": 27464 }, "react-test-renderer.production.min.js (NODE_PROD)": { - "size": 53730, - "gzip": 16673 + "size": 55449, + "gzip": 17169 }, "react-test-renderer-shallow.production.min.js (NODE_PROD)": { "size": 4630, @@ -209,20 +209,20 @@ "gzip": 4241 }, "react-reconciler.development.js (NODE_DEV)": { - "size": 271894, - "gzip": 56870 + "size": 281202, + "gzip": 58283 }, "react-reconciler.production.min.js (NODE_PROD)": { - "size": 37748, - "gzip": 11753 + "size": 37658, + "gzip": 11762 }, "ReactNativeCSFiber-dev.js (RN_DEV)": { - "size": 203431, - "gzip": 34170 + "size": 210188, + "gzip": 34969 }, "ReactNativeCSFiber-prod.js (RN_PROD)": { - "size": 153821, - "gzip": 25424 + "size": 160321, + "gzip": 26276 } } } \ No newline at end of file From 2f8d55b919f765abc6f948a0bb797d26f4d81927 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 16:15:59 -0700 Subject: [PATCH 21/22] Address comment --- packages/react-reconciler/src/ReactFiberCommitWork.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 0f76e74e715..8d661e63dc1 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -253,7 +253,8 @@ module.exports = function( // Skip portals because commitUnmount() currently visits them recursively. if ( node.child !== null && - // Drill down into portals only if we use mutation since that branch is recursive + // If we use mutation we drill down into portals using commitUnmount above. + // If we don't use mutation we drill down into portals here instead. (!mutation || node.tag !== HostPortal) ) { node.child.return = node; From bf043ddbaf5f1166f15465344c93836fd3bdb4d2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 18 Oct 2017 16:43:54 -0700 Subject: [PATCH 22/22] Move new files to new location --- .../src}/ReactNativeCSFeatureFlags.js | 0 .../src}/__tests__/ReactPersistent-test.js | 0 scripts/rollup/bundles.js | 2 +- scripts/rollup/results.json | 26 +++++++++---------- 4 files changed, 14 insertions(+), 14 deletions(-) rename {src/renderers/native-cs => packages/react-cs-renderer/src}/ReactNativeCSFeatureFlags.js (100%) rename {src/renderers/shared/fiber => packages/react-reconciler/src}/__tests__/ReactPersistent-test.js (100%) diff --git a/src/renderers/native-cs/ReactNativeCSFeatureFlags.js b/packages/react-cs-renderer/src/ReactNativeCSFeatureFlags.js similarity index 100% rename from src/renderers/native-cs/ReactNativeCSFeatureFlags.js rename to packages/react-cs-renderer/src/ReactNativeCSFeatureFlags.js diff --git a/src/renderers/shared/fiber/__tests__/ReactPersistent-test.js b/packages/react-reconciler/src/__tests__/ReactPersistent-test.js similarity index 100% rename from src/renderers/shared/fiber/__tests__/ReactPersistent-test.js rename to packages/react-reconciler/src/__tests__/ReactPersistent-test.js diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index f735afc9954..7868ce0f01b 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -323,7 +323,7 @@ const bundles = [ label: 'native-cs-fiber', manglePropertiesOnProd: false, name: 'react-native-cs-renderer', - featureFlags: 'src/renderers/native-cs/ReactNativeCSFeatureFlags', + featureFlags: 'packages/react-cs-renderer/src/ReactNativeCSFeatureFlags', paths: [ 'packages/react-native-renderer/**/*.js', // This is used since we reuse the error dialog code 'packages/react-cs-renderer/**/*.js', diff --git a/scripts/rollup/results.json b/scripts/rollup/results.json index 6dd455e46c9..018169ca02a 100644 --- a/scripts/rollup/results.json +++ b/scripts/rollup/results.json @@ -25,7 +25,7 @@ "gzip": 6709 }, "react-dom.development.js (UMD_DEV)": { - "size": 631933, + "size": 632002, "gzip": 144937 }, "react-dom.production.min.js (UMD_PROD)": { @@ -33,7 +33,7 @@ "gzip": 31680 }, "react-dom.development.js (NODE_DEV)": { - "size": 594214, + "size": 594283, "gzip": 136223 }, "react-dom.production.min.js (NODE_PROD)": { @@ -41,7 +41,7 @@ "gzip": 33539 }, "ReactDOMFiber-dev.js (FB_DEV)": { - "size": 591153, + "size": 591222, "gzip": 135569 }, "ReactDOMFiber-prod.js (FB_PROD)": { @@ -113,7 +113,7 @@ "gzip": 6214 }, "react-art.development.js (UMD_DEV)": { - "size": 378274, + "size": 378343, "gzip": 83009 }, "react-art.production.min.js (UMD_PROD)": { @@ -121,16 +121,16 @@ "gzip": 25592 }, "react-art.development.js (NODE_DEV)": { - "size": 302629, - "gzip": 63820 + "size": 302698, + "gzip": 63826 }, "react-art.production.min.js (NODE_PROD)": { "size": 53897, "gzip": 16869 }, "ReactARTFiber-dev.js (FB_DEV)": { - "size": 301232, - "gzip": 63727 + "size": 301301, + "gzip": 63733 }, "ReactARTFiber-prod.js (FB_PROD)": { "size": 225462, @@ -145,12 +145,12 @@ "gzip": 38454 }, "react-test-renderer.development.js (NODE_DEV)": { - "size": 306317, + "size": 306386, "gzip": 64218 }, "ReactTestRendererFiber-dev.js (FB_DEV)": { - "size": 304910, - "gzip": 64134 + "size": 304979, + "gzip": 64135 }, "react-test-renderer-shallow.development.js (NODE_DEV)": { "size": 9370, @@ -161,7 +161,7 @@ "gzip": 2253 }, "react-noop-renderer.development.js (NODE_DEV)": { - "size": 295525, + "size": 295594, "gzip": 61405 }, "react-dom-server.development.js (UMD_DEV)": { @@ -209,7 +209,7 @@ "gzip": 4241 }, "react-reconciler.development.js (NODE_DEV)": { - "size": 281202, + "size": 281271, "gzip": 58283 }, "react-reconciler.production.min.js (NODE_PROD)": {