From 28dc560d0f2455f62e62847a62573b2508e55e4c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 6 Jul 2016 13:46:43 -0700 Subject: [PATCH 01/13] [Fiber] unmountComponentAtNode Add unmounting hook. --- src/renderers/dom/fiber/ReactDOMFiber.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 7be5e5cc8787f..175b274549974 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -16,7 +16,7 @@ import type { HostChildren } from 'ReactFiberReconciler'; var ReactFiberReconciler = require('ReactFiberReconciler'); -type DOMContainerElement = Element & { _reactRootContainer: Object }; +type DOMContainerElement = Element & { _reactRootContainer: ?Object }; type Container = Element; type Props = { }; @@ -92,6 +92,16 @@ var ReactDOM = { } }, + unmountComponentAtNode(container : DOMContainerElement) { + const root = container._reactRootContainer; + if (root) { + // TODO: Is it safe to reset this now or should I wait since this + // unmount could be deferred? + container._reactRootContainer = null; + DOMRenderer.unmountContainer(root); + } + }, + }; module.exports = ReactDOM; From 00084f55038d95365c383c32c994c38b413c4a3c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 6 Jul 2016 13:47:43 -0700 Subject: [PATCH 02/13] [Fiber] Rudimentary class support Mostly just to get started with unit testing. --- .../shared/fiber/ReactFiberBeginWork.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 0b28cd98cfa9f..5d8b0868e061b 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -55,6 +55,22 @@ module.exports = function(config : HostConfig) { workInProgress.pendingWorkPriority = NoWork; } + function updateClassComponent(current, workInProgress) { + var props = workInProgress.pendingProps; + if (!workInProgress.stateNode) { + var ctor = workInProgress.type; + console.log('mount class:', workInProgress.type.name); + workInProgress.stateNode = new ctor(props); + } else { + console.log('update class:', workInProgress.type.name); + } + var instance = workInProgress.stateNode; + instance.props = props; + var nextChildren = instance.render(); + reconcileChildren(current, workInProgress, nextChildren); + workInProgress.pendingWorkPriority = NoWork; + } + function updateHostComponent(current, workInProgress) { console.log( 'host component', workInProgress.type, @@ -99,6 +115,7 @@ module.exports = function(config : HostConfig) { if (workInProgress.alternate) { workInProgress.alternate.tag = ClassComponent; } + value = value.render(); } else { console.log('performed work on fn:', fn.name); // Proceed under the assumption that this is a functional component @@ -199,7 +216,7 @@ module.exports = function(config : HostConfig) { updateFunctionalComponent(current, workInProgress); return workInProgress.child; case ClassComponent: - console.log('class component', workInProgress.pendingProps.type.name); + updateClassComponent(current, workInProgress); return workInProgress.child; case HostContainer: reconcileChildren(current, workInProgress, workInProgress.pendingProps); From e0e4954061a88f65c3b9ad6041a9b0a1d3881454 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 6 Jul 2016 13:53:35 -0700 Subject: [PATCH 03/13] Remove console.logs from Fiber The code is sufficiently complex now that this is more noise than helpful. Will just temporary explicit logs in the future. --- src/renderers/noop/ReactNoop.js | 5 ----- src/renderers/shared/fiber/ReactChildFiber.js | 4 ++-- src/renderers/shared/fiber/ReactFiberBeginWork.js | 12 ------------ src/renderers/shared/fiber/ReactFiberCompleteWork.js | 4 ---- src/renderers/shared/fiber/ReactFiberScheduler.js | 1 - .../shared/fiber/__tests__/ReactCoroutine-test.js | 1 - .../shared/fiber/__tests__/ReactIncremental-test.js | 1 - .../__tests__/ReactIncrementalSideEffects-test.js | 1 - 8 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 8404cf159b460..044b593a4e5c1 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -58,12 +58,10 @@ function flattenChildren(children : HostChildren) { var NoopRenderer = ReactFiberReconciler({ updateContainer(containerInfo : Container, children : HostChildren) : void { - console.log('Update container #' + containerInfo.rootID); containerInfo.children = flattenChildren(children); }, createInstance(type : string, props : Props, children : HostChildren) : Instance { - console.log('Create instance #' + instanceCounter); const inst = { tag: TERMINAL_TAG, id: instanceCounter++, @@ -77,17 +75,14 @@ var NoopRenderer = ReactFiberReconciler({ }, prepareUpdate(instance : Instance, oldProps : Props, newProps : Props, children : HostChildren) : boolean { - console.log('Prepare for update on #' + instance.id); return true; }, commitUpdate(instance : Instance, oldProps : Props, newProps : Props, children : HostChildren) : void { - console.log('Commit update on #' + instance.id); instance.children = flattenChildren(children); }, deleteInstance(instance : Instance) : void { - console.log('Delete #' + instance.id); }, scheduleHighPriCallback(callback) { diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index ddf4725a02cc2..225fe0db6a711 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -94,7 +94,7 @@ function createSubsequentChild( } return prev; } else { - console.log('Unknown child', newChildren); + // TODO: Throw for unknown children. return previousSibling; } } @@ -162,7 +162,7 @@ function createFirstChild(returnFiber, existingChild, newChildren, priority) { } return first; } else { - console.log('Unknown child', newChildren); + // TODO: Throw for unknown children. return null; } } diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 5d8b0868e061b..c0c1b67a4a7a8 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -49,7 +49,6 @@ module.exports = function(config : HostConfig) { function updateFunctionalComponent(current, workInProgress) { var fn = workInProgress.type; var props = workInProgress.pendingProps; - console.log('update fn:', fn.name); var nextChildren = fn(props); reconcileChildren(current, workInProgress, nextChildren); workInProgress.pendingWorkPriority = NoWork; @@ -59,10 +58,7 @@ module.exports = function(config : HostConfig) { var props = workInProgress.pendingProps; if (!workInProgress.stateNode) { var ctor = workInProgress.type; - console.log('mount class:', workInProgress.type.name); workInProgress.stateNode = new ctor(props); - } else { - console.log('update class:', workInProgress.type.name); } var instance = workInProgress.stateNode; instance.props = props; @@ -72,11 +68,6 @@ module.exports = function(config : HostConfig) { } function updateHostComponent(current, workInProgress) { - console.log( - 'host component', workInProgress.type, - typeof workInProgress.pendingProps.children === 'string' ? workInProgress.pendingProps.children : '' - ); - var nextChildren = workInProgress.pendingProps.children; let priority = workInProgress.pendingWorkPriority; @@ -109,7 +100,6 @@ module.exports = function(config : HostConfig) { var props = workInProgress.pendingProps; var value = fn(props); if (typeof value === 'object' && value && typeof value.render === 'function') { - console.log('performed work on class:', fn.name); // Proceed under the assumption that this is a class instance workInProgress.tag = ClassComponent; if (workInProgress.alternate) { @@ -117,7 +107,6 @@ module.exports = function(config : HostConfig) { } value = value.render(); } else { - console.log('performed work on fn:', fn.name); // Proceed under the assumption that this is a functional component workInProgress.tag = FunctionalComponent; if (workInProgress.alternate) { @@ -133,7 +122,6 @@ module.exports = function(config : HostConfig) { if (!coroutine) { throw new Error('Should be resolved by now'); } - console.log('begin coroutine', workInProgress.type.name); reconcileChildren(current, workInProgress, coroutine.children); workInProgress.pendingWorkPriority = NoWork; } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 9900a5b9fb9d0..f435778759a45 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -125,11 +125,9 @@ module.exports = function(config : HostConfig) { function completeWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { switch (workInProgress.tag) { case FunctionalComponent: - console.log('/functional component', workInProgress.type.name); transferOutput(workInProgress.child, workInProgress); return null; case ClassComponent: - console.log('/class component', workInProgress.type.name); transferOutput(workInProgress.child, workInProgress); return null; case HostContainer: @@ -142,7 +140,6 @@ module.exports = function(config : HostConfig) { markForPreEffect(workInProgress); return null; case HostComponent: - console.log('/host component', workInProgress.type); const child = workInProgress.child; const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child; const newProps = workInProgress.pendingProps; @@ -165,7 +162,6 @@ module.exports = function(config : HostConfig) { } return null; case CoroutineComponent: - console.log('/coroutine component', workInProgress.pendingProps.handler.name); return moveCoroutineToHandlerPhase(current, workInProgress); case CoroutineHandlerPhase: transferOutput(workInProgress.stateNode, workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 6df6198f9fe6f..dfbaa4000efee 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -156,7 +156,6 @@ module.exports = function(config : HostConfig) { // "next" scheduled work since we've already scanned passed. That // also ensures that work scheduled during reconciliation gets deferred. // const hasMoreWork = workInProgress.pendingWorkPriority !== NoWork; - console.log('----- COMPLETED with remaining work:', workInProgress.pendingWorkPriority); commitAllWork(workInProgress); const nextWork = findNextUnitOfWork(); // if (!nextWork && hasMoreWork) { diff --git a/src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js b/src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js index cd5423625894a..553085c35068b 100644 --- a/src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js @@ -20,7 +20,6 @@ describe('ReactCoroutine', function() { React = require('React'); ReactNoop = require('ReactNoop'); ReactCoroutine = require('ReactCoroutine'); - spyOn(console, 'log'); }); it('should render a coroutine', function() { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 1fd0ac9fe9d5e..4c5b734d13d6c 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -18,7 +18,6 @@ describe('ReactIncremental', function() { beforeEach(function() { React = require('React'); ReactNoop = require('ReactNoop'); - spyOn(console, 'log'); }); it('should render a simple component', function() { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index c59c12b97d31a..789f4d8f2bd09 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -18,7 +18,6 @@ describe('ReactIncremental', function() { beforeEach(function() { React = require('React'); ReactNoop = require('ReactNoop'); - spyOn(console, 'log'); }); function div(...children) { From de4a7b972bad8c0fdc4256577c54eb3e1b8a7b27 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 6 Jul 2016 14:12:42 -0700 Subject: [PATCH 04/13] Move all imports to closures in Fiber I'm paranoid about inline-ability so I use this pattern of adding a constant to the closure everywhere. ES6 modules help avoid that but we can't use that consistently because of the dependency injection so instead I opt for making this explicit everywhere. Grep: \b[a-zA-Z_$\d]+\.[a-zA-Z_$\d]+\( --- src/renderers/shared/fiber/ReactChildFiber.js | 51 ++++++++++++------- src/renderers/shared/fiber/ReactFiber.js | 6 ++- .../shared/fiber/ReactFiberBeginWork.js | 8 +-- .../shared/fiber/ReactFiberCompleteWork.js | 4 +- .../shared/fiber/ReactFiberPendingWork.js | 6 +-- .../shared/fiber/ReactReifiedYield.js | 4 +- 6 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 225fe0db6a711..c5ca161a01188 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -29,11 +29,24 @@ var { var ReactFiber = require('ReactFiber'); var ReactReifiedYield = require('ReactReifiedYield'); +const { + cloneFiber, + createFiberFromElement, + createFiberFromCoroutine, + createFiberFromYield, +} = ReactFiber; + +const { + createReifiedYield, +} = ReactReifiedYield; + +const isArray = Array.isArray; + function createSubsequentChild( - returnFiber : Fiber, - existingChild : ?Fiber, - previousSibling : Fiber, - newChildren, + returnFiber : Fiber, + existingChild : ?Fiber, + previousSibling : Fiber, + newChildren, priority : PriorityLevel ) : Fiber { if (typeof newChildren !== 'object' || newChildren === null) { @@ -48,7 +61,7 @@ function createSubsequentChild( element.key === existingChild.key) { // TODO: This is not sufficient since previous siblings could be new. // Will fix reconciliation properly later. - const clone = ReactFiber.cloneFiber(existingChild, priority); + const clone = cloneFiber(existingChild, priority); clone.pendingProps = element.props; clone.child = existingChild.child; clone.sibling = null; @@ -56,7 +69,7 @@ function createSubsequentChild( previousSibling.sibling = clone; return clone; } - const child = ReactFiber.createFiberFromElement(element, priority); + const child = createFiberFromElement(element, priority); previousSibling.sibling = child; child.return = returnFiber; return child; @@ -64,7 +77,7 @@ function createSubsequentChild( case REACT_COROUTINE_TYPE: { const coroutine = (newChildren : ReactCoroutine); - const child = ReactFiber.createFiberFromCoroutine(coroutine, priority); + const child = createFiberFromCoroutine(coroutine, priority); previousSibling.sibling = child; child.return = returnFiber; return child; @@ -72,8 +85,8 @@ function createSubsequentChild( case REACT_YIELD_TYPE: { const yieldNode = (newChildren : ReactYield); - const reifiedYield = ReactReifiedYield.createReifiedYield(yieldNode); - const child = ReactFiber.createFiberFromYield(yieldNode, priority); + const reifiedYield = createReifiedYield(yieldNode); + const child = createFiberFromYield(yieldNode, priority); child.output = reifiedYield; previousSibling.sibling = child; child.return = returnFiber; @@ -81,7 +94,7 @@ function createSubsequentChild( } } - if (Array.isArray(newChildren)) { + if (isArray(newChildren)) { let prev : Fiber = previousSibling; let existing : ?Fiber = existingChild; for (var i = 0; i < newChildren.length; i++) { @@ -111,21 +124,21 @@ function createFirstChild(returnFiber, existingChild, newChildren, priority) { element.type === existingChild.type && element.key === existingChild.key) { // Get the clone of the existing fiber. - const clone = ReactFiber.cloneFiber(existingChild, priority); + const clone = cloneFiber(existingChild, priority); clone.pendingProps = element.props; clone.child = existingChild.child; clone.sibling = null; clone.return = returnFiber; return clone; } - const child = ReactFiber.createFiberFromElement(element, priority); + const child = createFiberFromElement(element, priority); child.return = returnFiber; return child; } case REACT_COROUTINE_TYPE: { const coroutine = (newChildren : ReactCoroutine); - const child = ReactFiber.createFiberFromCoroutine(coroutine, priority); + const child = createFiberFromCoroutine(coroutine, priority); child.return = returnFiber; return child; } @@ -135,15 +148,15 @@ function createFirstChild(returnFiber, existingChild, newChildren, priority) { // TODO: When there is only a single child, we can optimize this to avoid // the fragment. const yieldNode = (newChildren : ReactYield); - const reifiedYield = ReactReifiedYield.createReifiedYield(yieldNode); - const child = ReactFiber.createFiberFromYield(yieldNode, priority); + const reifiedYield = createReifiedYield(yieldNode); + const child = createFiberFromYield(yieldNode, priority); child.output = reifiedYield; child.return = returnFiber; return child; } } - if (Array.isArray(newChildren)) { + if (isArray(newChildren)) { var first : ?Fiber = null; var prev : ?Fiber = null; var existing : ?Fiber = existingChild; @@ -170,9 +183,9 @@ function createFirstChild(returnFiber, existingChild, newChildren, priority) { // TODO: This API won't work because we'll need to transfer the side-effects of // unmounting children to the returnFiber. exports.reconcileChildFibers = function( - returnFiber : Fiber, - currentFirstChild : ?Fiber, - newChildren : ReactNodeList, + returnFiber : Fiber, + currentFirstChild : ?Fiber, + newChildren : ReactNodeList, priority : PriorityLevel ) : ?Fiber { return createFirstChild(returnFiber, currentFirstChild, newChildren, priority); diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 39683f001e55f..991fb5f6d7c83 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -201,13 +201,13 @@ exports.createHostContainerFiber = function() { }; exports.createFiberFromElement = function(element : ReactElement, priorityLevel : PriorityLevel) { - const fiber = exports.createFiberFromElementType(element.type, element.key); + const fiber = createFiberFromElementType(element.type, element.key); fiber.pendingProps = element.props; fiber.pendingWorkPriority = priorityLevel; return fiber; }; -exports.createFiberFromElementType = function(type : mixed, key : null | string) { +function createFiberFromElementType(type : mixed, key : null | string) { let fiber; if (typeof type === 'function') { fiber = shouldConstruct(type) ? @@ -226,6 +226,8 @@ exports.createFiberFromElementType = function(type : mixed, key : null | string) return fiber; }; +exports.createFiberFromElementType = createFiberFromElementType; + exports.createFiberFromCoroutine = function(coroutine : ReactCoroutine, priorityLevel : PriorityLevel) { const fiber = createFiber(CoroutineComponent, coroutine.key); fiber.type = coroutine.handler; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index c0c1b67a4a7a8..86f96db2f7d1d 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -16,7 +16,7 @@ import type { ReactCoroutine } from 'ReactCoroutine'; import type { Fiber } from 'ReactFiber'; import type { HostConfig } from 'ReactFiberReconciler'; -var ReactChildFiber = require('ReactChildFiber'); +var { reconcileChildFibers } = require('ReactChildFiber'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { IndeterminateComponent, @@ -38,7 +38,7 @@ module.exports = function(config : HostConfig) { function reconcileChildren(current, workInProgress, nextChildren) { const priority = workInProgress.pendingWorkPriority; - workInProgress.child = ReactChildFiber.reconcileChildFibers( + workInProgress.child = reconcileChildFibers( workInProgress, current ? current.child : null, nextChildren, @@ -75,7 +75,7 @@ module.exports = function(config : HostConfig) { // If this host component is hidden, we can reconcile its children at // the lowest priority and bail out from this particular pass. Unless, we're // currently reconciling the lowest priority. - workInProgress.child = ReactChildFiber.reconcileChildFibers( + workInProgress.child = reconcileChildFibers( workInProgress, current ? current.child : null, nextChildren, @@ -84,7 +84,7 @@ module.exports = function(config : HostConfig) { workInProgress.pendingWorkPriority = OffscreenPriority; return null; } else { - workInProgress.child = ReactChildFiber.reconcileChildFibers( + workInProgress.child = reconcileChildFibers( workInProgress, current ? current.child : null, nextChildren, diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index f435778759a45..df1df562c96d3 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -17,7 +17,7 @@ import type { Fiber } from 'ReactFiber'; import type { HostConfig } from 'ReactFiberReconciler'; import type { ReifiedYield } from 'ReactReifiedYield'; -var ReactChildFiber = require('ReactChildFiber'); +var { reconcileChildFibers } = require('ReactChildFiber'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { IndeterminateComponent, @@ -113,7 +113,7 @@ module.exports = function(config : HostConfig) { var currentFirstChild = current ? current.stateNode : null; // Inherit the priority of the returnFiber. const priority = workInProgress.pendingWorkPriority; - workInProgress.stateNode = ReactChildFiber.reconcileChildFibers( + workInProgress.stateNode = reconcileChildFibers( workInProgress, currentFirstChild, nextChildren, diff --git a/src/renderers/shared/fiber/ReactFiberPendingWork.js b/src/renderers/shared/fiber/ReactFiberPendingWork.js index 15d1a10134901..8505473de3d6e 100644 --- a/src/renderers/shared/fiber/ReactFiberPendingWork.js +++ b/src/renderers/shared/fiber/ReactFiberPendingWork.js @@ -15,7 +15,7 @@ import type { Fiber } from 'ReactFiber'; import type { PriorityLevel } from 'ReactPriorityLevel'; -var ReactFiber = require('ReactFiber'); +var { cloneFiber } = require('ReactFiber'); var { NoWork, @@ -24,7 +24,7 @@ var { function cloneSiblings(current : Fiber, workInProgress : Fiber, returnFiber : Fiber) { while (current.sibling) { current = current.sibling; - workInProgress = workInProgress.sibling = ReactFiber.cloneFiber( + workInProgress = workInProgress.sibling = cloneFiber( current, current.pendingWorkPriority ); @@ -62,7 +62,7 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev throw new Error('Should have wip now'); } workInProgress.pendingWorkPriority = current.pendingWorkPriority; - workInProgress.child = ReactFiber.cloneFiber(currentChild, NoWork); + workInProgress.child = cloneFiber(currentChild, NoWork); workInProgress.child.return = workInProgress; cloneSiblings(currentChild, workInProgress.child, workInProgress); current = current.child; diff --git a/src/renderers/shared/fiber/ReactReifiedYield.js b/src/renderers/shared/fiber/ReactReifiedYield.js index 3f0e8a62e5262..41ef6b9218973 100644 --- a/src/renderers/shared/fiber/ReactReifiedYield.js +++ b/src/renderers/shared/fiber/ReactReifiedYield.js @@ -15,12 +15,12 @@ import type { ReactYield } from 'ReactCoroutine'; import type { Fiber } from 'ReactFiber'; -var ReactFiber = require('ReactFiber'); +var { createFiberFromElementType } = require('ReactFiber'); export type ReifiedYield = { continuation: Fiber, props: Object }; exports.createReifiedYield = function(yieldNode : ReactYield) : ReifiedYield { - var fiber = ReactFiber.createFiberFromElementType( + var fiber = createFiberFromElementType( yieldNode.continuation, yieldNode.key ); From 7e9b95866231e99342217593a8e3bfbae05280ef Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 6 Jul 2016 20:24:43 -0700 Subject: [PATCH 05/13] Reuse old props for the update if there are no new props --- src/renderers/noop/ReactNoop.js | 6 ++-- .../shared/fiber/ReactFiberCompleteWork.js | 14 ++++++++-- .../ReactIncrementalSideEffects-test.js | 28 +++++++++++++++---- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 044b593a4e5c1..70598bc5b7f69 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -30,8 +30,8 @@ var scheduledLowPriCallback = null; const TERMINAL_TAG = 99; type Container = { rootID: number, children: Array }; -type Props = { }; -type Instance = { tag: 99, type: string, id: number, children: Array }; +type Props = { prop: any }; +type Instance = { tag: 99, type: string, id: number, children: Array, prop: any }; var instanceCounter = 0; @@ -67,6 +67,7 @@ var NoopRenderer = ReactFiberReconciler({ id: instanceCounter++, type: type, children: flattenChildren(children), + prop: props.prop, }; // Hide from unit tests Object.defineProperty(inst, 'tag', { value: inst.tag, enumerable: false }); @@ -80,6 +81,7 @@ var NoopRenderer = ReactFiberReconciler({ commitUpdate(instance : Instance, oldProps : Props, newProps : Props, children : HostChildren) : void { instance.children = flattenChildren(children); + instance.prop = newProps.prop; }, deleteInstance(instance : Instance) : void { diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index df1df562c96d3..f75ca6b0f25b2 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -142,12 +142,18 @@ module.exports = function(config : HostConfig) { case HostComponent: const child = workInProgress.child; const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child; - const newProps = workInProgress.pendingProps; - workInProgress.memoizedProps = newProps; + let newProps = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to // schedule a side-effect to do the updates. const oldProps = current.memoizedProps; + // If we get updated because one of our children updated, we don't + // have newProps so we'll have to reuse them. + // TODO: Split the update API as separate for the props vs. children. + // Even better would be if children weren't special cased at all tho. + if (!newProps) { + newProps = oldProps; + } const instance : I = workInProgress.stateNode; if (prepareUpdate(instance, oldProps, newProps, children)) { // This returns true if there was something to update. @@ -155,11 +161,15 @@ module.exports = function(config : HostConfig) { } workInProgress.output = instance; } else { + if (!newProps) { + throw new Error('We must have new props for new mounts.'); + } const instance = createInstance(workInProgress.type, newProps, children); // TODO: This seems like unnecessary duplication. workInProgress.stateNode = instance; workInProgress.output = instance; } + workInProgress.memoizedProps = newProps; return null; case CoroutineComponent: return moveCoroutineToHandlerPhase(current, workInProgress); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 789f4d8f2bd09..d4974fc3e0718 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -21,11 +21,11 @@ describe('ReactIncremental', function() { }); function div(...children) { - return { type: 'div', children }; + return { type: 'div', children, prop: undefined }; } - function span(...children) { - return { type: 'span', children }; + function span(prop) { + return { type: 'span', children: [], prop }; } it('can update child nodes of a host instance', function() { @@ -60,7 +60,7 @@ describe('ReactIncremental', function() { it('does not update child nodes if a flush is aborted', function() { function Bar(props) { - return {props.text}; + return ; } function Foo(props) { @@ -78,17 +78,33 @@ describe('ReactIncremental', function() { ReactNoop.render(); ReactNoop.flush(); expect(ReactNoop.root.children).toEqual([ - div(div(span(), span()), span()), + div(div(span('Hello'), span('Hello')), span('Yo')), ]); ReactNoop.render(); ReactNoop.flushLowPri(35); expect(ReactNoop.root.children).toEqual([ - div(div(span(), span()), span()), + div(div(span('Hello'), span('Hello')), span('Yo')), ]); }); + it('updates a child even though the old props is empty', function() { + function Foo(props) { + return ( + + ); + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.root.children).toEqual([ + div(span(1)), + ]); + }); + // TODO: Test that side-effects are not cut off when a work in progress node // moves to "current" without flushing due to having lower priority. Does this // even happen? Maybe a child doesn't get processed because it is lower prio? From 5c094fb5d1467e7b1fde823363fa4d1b9d8bdd11 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 6 Jul 2016 21:08:43 -0700 Subject: [PATCH 06/13] Don't reset next work pointer for lower priority work If the current work is higher priority than the new work, then don't bother resetting the unit of work pointer since it won't affect the execution order. --- src/renderers/shared/fiber/ReactFiberReconciler.js | 6 +++--- src/renderers/shared/fiber/ReactFiberScheduler.js | 13 +++++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index cb22e88421dca..4ece60a0f2e9c 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -73,7 +73,7 @@ module.exports = function(config : HostConfig) : Reconci container.pendingProps = element; container.pendingWorkPriority = LowPriority; - scheduleLowPriWork(root); + scheduleLowPriWork(root, LowPriority); // It may seem strange that we don't return the root here, but that will // allow us to have containers that are in the middle of the tree instead @@ -88,7 +88,7 @@ module.exports = function(config : HostConfig) : Reconci root.current.pendingProps = element; root.current.pendingWorkPriority = LowPriority; - scheduleLowPriWork(root); + scheduleLowPriWork(root, LowPriority); }, unmountContainer(container : OpaqueNode) : void { @@ -98,7 +98,7 @@ module.exports = function(config : HostConfig) : Reconci root.current.pendingProps = []; root.current.pendingWorkPriority = LowPriority; - scheduleLowPriWork(root); + scheduleLowPriWork(root, LowPriority); }, getPublicRootInstance(container : OpaqueNode) : (C | null) { diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index dfbaa4000efee..1bf5150ac8919 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -15,6 +15,7 @@ import type { Fiber } from 'ReactFiber'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; +import type { PriorityLevel } from 'ReactPriorityLevel'; var ReactFiberBeginWork = require('ReactFiberBeginWork'); var ReactFiberCompleteWork = require('ReactFiberCompleteWork'); @@ -43,6 +44,7 @@ module.exports = function(config : HostConfig) { // The next work in progress fiber that we're currently working on. let nextUnitOfWork : ?Fiber = null; + let nextPriorityLevel : PriorityLevel = NoWork; // Linked list of roots with scheduled work on them. let nextScheduledRoot : ?FiberRoot = null; @@ -55,6 +57,7 @@ module.exports = function(config : HostConfig) { if (nextScheduledRoot === lastScheduledRoot) { nextScheduledRoot = null; lastScheduledRoot = null; + nextPriorityLevel = NoWork; return null; } nextScheduledRoot = nextScheduledRoot.nextScheduledRoot; @@ -68,19 +71,23 @@ module.exports = function(config : HostConfig) { // too many just means flushing changes too often. let work = findNextUnitOfWorkAtPriority(root.current, HighPriority); if (work) { + nextPriorityLevel = HighPriority; return work; } work = findNextUnitOfWorkAtPriority(root.current, LowPriority); if (work) { + nextPriorityLevel = LowPriority; return work; } work = findNextUnitOfWorkAtPriority(root.current, OffscreenPriority); if (work) { + nextPriorityLevel = OffscreenPriority; return work; } // We didn't find anything to do in this root, so let's try the next one. root = root.nextScheduledRoot; } + nextPriorityLevel = NoWork; return null; } @@ -208,11 +215,13 @@ module.exports = function(config : HostConfig) { } } - function scheduleLowPriWork(root : FiberRoot) { + function scheduleLowPriWork(root : FiberRoot, priority : PriorityLevel) { // We must reset the current unit of work pointer so that we restart the // search from the root during the next tick, in case there is now higher // priority work somewhere earlier than before. - nextUnitOfWork = null; + if (priority <= nextPriorityLevel) { + nextUnitOfWork = null; + } if (root.isScheduled) { // If we're already scheduled, we can bail out. From d0838797a31d396337df97170eb89b6c9c277067 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 7 Jul 2016 14:00:28 -0700 Subject: [PATCH 07/13] Set pendingProps in cloneFiber for symmetry This might become confusing later but is unreachable today. That's because existing pendingProps only matter when a clone is updated, but this path can only matter when it is created. --- src/renderers/shared/fiber/ReactFiber.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 991fb5f6d7c83..a813dcc9fab10 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -188,6 +188,8 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.child = fiber.child; alt.sibling = fiber.sibling; alt.ref = alt.ref; + // pendingProps is here for symmetry but is unnecessary in practice for now. + alt.pendingProps = fiber.pendingProps; alt.pendingWorkPriority = priorityLevel; alt.alternate = fiber; From 5ac87e0d0fe3227ec64bd1fec55aa6bb5918026d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 18 Jul 2016 16:22:07 -0700 Subject: [PATCH 08/13] When a reconciliation gets down prioritized, reuse children When we downprioritize children we need to remember to reuse the old children in the update side-effect. This whole set up is very unfortunate since we can have children in our active tree that never actually finished rendering. This strategy might be fundamentally flawed, not sure. --- src/renderers/shared/fiber/ReactFiber.js | 6 +++ .../shared/fiber/ReactFiberBeginWork.js | 2 + .../shared/fiber/ReactFiberCommitWork.js | 9 ++++- .../shared/fiber/ReactFiberCompleteWork.js | 11 +++++- .../ReactIncrementalSideEffects-test.js | 39 +++++++++++++++++++ 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index a813dcc9fab10..02bb854c228c0 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -106,6 +106,11 @@ export type Fiber = Instance & { // I'm not really sure so I'll use a flag for now. // TODO: Find another way to infer this flag. hasWorkInProgress: bool, + // Keeps track if we've deprioritized the children of this node so we know to + // reuse the existing children. + // TODO: Find another way to infer this flag or separate children updates from + // property updates so that we simply don't try to update children here. + wasDeprioritized: bool, // Conceptual aliases // workInProgress : Fiber -> alternate The alternate used for reuse happens @@ -146,6 +151,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { pendingWorkPriority: NoWork, hasWorkInProgress: false, + wasDeprioritized: false, alternate: null, diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 86f96db2f7d1d..49888b7337f75 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -82,6 +82,7 @@ module.exports = function(config : HostConfig) { OffscreenPriority ); workInProgress.pendingWorkPriority = OffscreenPriority; + workInProgress.wasDeprioritized = true; return null; } else { workInProgress.child = reconcileChildFibers( @@ -91,6 +92,7 @@ module.exports = function(config : HostConfig) { priority ); workInProgress.pendingWorkPriority = NoWork; + workInProgress.wasDeprioritized = false; return workInProgress.child; } } diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 7792be2102bb1..5a5a6ae74dcd8 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -47,7 +47,14 @@ module.exports = function(config : HostConfig) { throw new Error('This should only be done during updates.'); } // Commit the work prepared earlier. - const child = (finishedWork.child : ?Fiber); + let child; + if (finishedWork.wasDeprioritized) { + // If this was a down priority, we need to preserve the old child in + // the output. + child = finishedWork.alternate ? finishedWork.alternate.child : null; + } else { + child = finishedWork.child; + } const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child; const newProps = finishedWork.memoizedProps; const current = finishedWork.alternate; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index f75ca6b0f25b2..dd27062f6f4b5 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -140,9 +140,16 @@ module.exports = function(config : HostConfig) { markForPreEffect(workInProgress); return null; case HostComponent: - const child = workInProgress.child; - const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child; + let child; let newProps = workInProgress.pendingProps; + if (workInProgress.wasDeprioritized) { + // If this was a down priority, we need to preserve the old child in + // the output. + child = current ? current.child : null; + } else { + child = workInProgress.child; + } + const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to // schedule a side-effect to do the updates. diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index d4974fc3e0718..f5ffa84a45947 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -89,6 +89,45 @@ describe('ReactIncremental', function() { }); + it('preserves a previously rendered node when deprioritized', function() { + + function Middle(props) { + return ; + } + + function Foo(props) { + return ( +
+ +
+ ); + } + + ReactNoop.render(); + ReactNoop.flush(); + + expect(ReactNoop.root.children).toEqual([ + div(div(span('foo'))), + ]); + + ReactNoop.render(); + ReactNoop.flushLowPri(20); + + expect(ReactNoop.root.children).toEqual([ + div(div(span('foo'))), + ]); + + ReactNoop.flush(); + + expect(ReactNoop.root.children).toEqual([ + div(div(span('bar'))), + ]); + + }); + + it('updates a child even though the old props is empty', function() { function Foo(props) { return ( From 48b7b2c67c96aec21b6c4150c6c3f80d481f709b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 18 Jul 2016 17:41:58 -0700 Subject: [PATCH 09/13] Add note about potential future bugs --- src/renderers/shared/fiber/ReactFiberCompleteWork.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index dd27062f6f4b5..fd0cb31f2da21 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -47,6 +47,9 @@ module.exports = function(config : HostConfig) { } /* + // TODO: It's possible this will create layout thrash issues because mutations + // of the DOM and life-cycles are interleaved. E.g. if a componentDidMount + // of a sibling reads, then the next sibling updates and reads etc. function markForPostEffect(workInProgress : Fiber) { // Schedule a side-effect on this fiber, AFTER the children's side-effects. if (workInProgress.lastEffect) { From 9fca81213967210ea3f74daab7a0f64a454bd3b6 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 18 Jul 2016 18:11:28 -0700 Subject: [PATCH 10/13] Fix NoWork bug Thanks @acdlite. Add comment about future unit test coverage. This was actually hiding the fact that we are only able to reuse existing work if it was marked as completely finished which it won't be if we reuse its pending priority. However, we should be able to bail out even if there is work remaining in a subtree. --- .../shared/fiber/ReactFiberBeginWork.js | 22 ++++++++++++++----- .../shared/fiber/ReactFiberPendingWork.js | 13 +++++++++-- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 49888b7337f75..984c09de9810f 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -169,7 +169,7 @@ module.exports = function(config : HostConfig) { // it until the next tick. workInProgress.child = current.child; reuseChildren(workInProgress, workInProgress.child); - if (workInProgress.pendingWorkPriority <= priorityLevel) { + if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) { // TODO: This passes the current node and reads the priority level and // pending props from that. We want it to read our priority level and // pending props from the work in progress. Needs restructuring. @@ -184,15 +184,25 @@ module.exports = function(config : HostConfig) { } if (!workInProgress.hasWorkInProgress && - workInProgress.pendingProps === workInProgress.memoizedProps && - workInProgress.pendingWorkPriority === NoWork) { + workInProgress.pendingProps === workInProgress.memoizedProps) { // If we started this work before, and finished it, or if we're in a // ping-pong update scenario, this version could already be what we're // looking for. In that case, we should be able to just bail out. + const priorityLevel = workInProgress.pendingWorkPriority; workInProgress.pendingProps = null; - // TODO: We should be able to bail out if there is remaining work at a lower - // priority too. However, I don't know if that is safe or even better since - // the other tree could've potentially finished that work. + workInProgress.pendingWorkPriority = NoWork; + if (workInProgress.child) { + // If we bail out but still has work with the current priority in this + // subtree, we need to go find it right now. If we don't, we won't flush + // it until the next tick. + reuseChildren(workInProgress, workInProgress.child); + if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) { + // TODO: This passes the current node and reads the priority level and + // pending props from that. We want it to read our priority level and + // pending props from the work in progress. Needs restructuring. + return findNextUnitOfWorkAtPriority(workInProgress, priorityLevel); + } + } return null; } diff --git a/src/renderers/shared/fiber/ReactFiberPendingWork.js b/src/renderers/shared/fiber/ReactFiberPendingWork.js index 8505473de3d6e..bf837c0426ca8 100644 --- a/src/renderers/shared/fiber/ReactFiberPendingWork.js +++ b/src/renderers/shared/fiber/ReactFiberPendingWork.js @@ -22,6 +22,7 @@ var { } = require('ReactPriorityLevel'); function cloneSiblings(current : Fiber, workInProgress : Fiber, returnFiber : Fiber) { + workInProgress.return = returnFiber; while (current.sibling) { current = current.sibling; workInProgress = workInProgress.sibling = cloneFiber( @@ -49,6 +50,7 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev workInProgress.pendingProps = current.pendingProps; return workInProgress; } + // If we have a child let's see if any of our children has work to do. // Only bother doing this at all if the current priority level matches // because it is the highest priority for the whole subtree. @@ -62,8 +64,15 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev throw new Error('Should have wip now'); } workInProgress.pendingWorkPriority = current.pendingWorkPriority; - workInProgress.child = cloneFiber(currentChild, NoWork); - workInProgress.child.return = workInProgress; + // TODO: The below priority used to be set to NoWork which would've + // dropped work. This is currently unobservable but will become + // observable when the first sibling has lower priority work remaining + // than the next sibling. At that point we should add tests that catches + // this. + workInProgress.child = cloneFiber( + currentChild, + currentChild.pendingWorkPriority + ); cloneSiblings(currentChild, workInProgress.child, workInProgress); current = current.child; continue; From de6069e5504e5b4d71dada5da9a771f11f00c6cc Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 18 Jul 2016 20:00:17 -0700 Subject: [PATCH 11/13] Reuse the correct child and side-effects when reusing partial work We need to use the *other* child because we reset it to the current one on the way up. We also need to reset the first/last side-effects to that of the children so that we're committing the right thing. --- .../shared/fiber/ReactFiberBeginWork.js | 29 ++++++++- .../fiber/__tests__/ReactIncremental-test.js | 2 - .../ReactIncrementalSideEffects-test.js | 62 ++++++++++++++++++- 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 984c09de9810f..b6e38d77ccafe 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -148,6 +148,23 @@ module.exports = function(config : HostConfig) { } while (child = child.sibling); } + function reuseChildrenEffects(returnFiber : Fiber, firstChild : Fiber) { + let child = firstChild; + do { + // Ensure that the first and last effect of the parent corresponds + // to the children's first and last effect. + if (!returnFiber.firstEffect) { + returnFiber.firstEffect = child.firstEffect; + } + if (child.lastEffect) { + if (returnFiber.lastEffect) { + returnFiber.lastEffect.nextEffect = child.firstEffect; + } + returnFiber.lastEffect = child.lastEffect; + } + } while (child = child.sibling); + } + function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here @@ -191,7 +208,17 @@ module.exports = function(config : HostConfig) { const priorityLevel = workInProgress.pendingWorkPriority; workInProgress.pendingProps = null; workInProgress.pendingWorkPriority = NoWork; - if (workInProgress.child) { + + workInProgress.firstEffect = null; + workInProgress.nextEffect = null; + workInProgress.lastEffect = null; + + if (workInProgress.child && workInProgress.child.alternate) { + // On the way up here, we reset the child node to be the current one. + // Therefore we have to reuse the alternate. This is super weird. + workInProgress.child = workInProgress.child.alternate; + // Ensure that the effects of reused work are preserved. + reuseChildrenEffects(workInProgress, workInProgress.child); // If we bail out but still has work with the current priority in this // subtree, we need to go find it right now. If we don't, we won't flush // it until the next tick. diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 4c5b734d13d6c..4158724be28b7 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -394,8 +394,6 @@ describe('ReactIncremental', function() { ); } - // Start rendering an update - // Init ReactNoop.render(); ReactNoop.flush(); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index f5ffa84a45947..68360c62885a8 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -14,7 +14,7 @@ var React; var ReactNoop; -describe('ReactIncremental', function() { +describe('ReactIncrementalSideEffects', function() { beforeEach(function() { React = require('React'); ReactNoop = require('ReactNoop'); @@ -127,6 +127,66 @@ describe('ReactIncremental', function() { }); + it('can reuse side-effects after being preempted', function() { + + function Bar(props) { + return ; + } + + var middleContent = ( +
+ Hello + World +
+ ); + + function Foo(props) { + return ( + + ); + } + + // Init + ReactNoop.render(); + ReactNoop.flush(); + + expect(ReactNoop.root.children).toEqual([ + div(div(span('Hi'), span('foo'))), + ]); + + // Make a quick update which will schedule low priority work to + // update the middle content. + ReactNoop.render(); + ReactNoop.flushLowPri(30); + + // The tree remains unchanged. + expect(ReactNoop.root.children).toEqual([ + div(div(span('Hi'), span('foo'))), + ]); + + // The first Bar has already completed its update but we'll interupt it to + // render some higher priority work. The middle content will bailout so + // it remains untouched which means that it should reuse it next time. + ReactNoop.render(); + ReactNoop.flush(30); + + // Since we did nothing to the middle subtree during the interuption, + // we should be able to reuse the reconciliation work that we already did + // without restarting. The side-effects should still be replayed. + + expect(ReactNoop.root.children).toEqual([ + div(div(span('Hello'), span('World'))), + ]); + }); it('updates a child even though the old props is empty', function() { function Foo(props) { From 94ed00740b191617420f3736dba8df3c0bbeac96 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 20 Jul 2016 19:34:50 -0700 Subject: [PATCH 12/13] Introduce shouldComponentUpdate in Fiber It is important to be able to use this since it avoids starvation problems if you can reuse partially completed work. --- .../shared/fiber/ReactFiberBeginWork.js | 164 +++++++++++------- .../fiber/__tests__/ReactIncremental-test.js | 137 +++++++++++++-- .../ReactIncrementalSideEffects-test.js | 66 +++++++ 3 files changed, 286 insertions(+), 81 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index b6e38d77ccafe..6d841b075b509 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -54,17 +54,35 @@ module.exports = function(config : HostConfig) { workInProgress.pendingWorkPriority = NoWork; } - function updateClassComponent(current, workInProgress) { + function updateClassComponent(current : ?Fiber, workInProgress : Fiber) { var props = workInProgress.pendingProps; - if (!workInProgress.stateNode) { + var instance = workInProgress.stateNode; + if (!instance) { var ctor = workInProgress.type; - workInProgress.stateNode = new ctor(props); + workInProgress.stateNode = instance = new ctor(props); + } else if (typeof instance.shouldComponentUpdate === 'function') { + if (current && current.memoizedProps) { + // Revert to the last flushed props, incase we aborted an update. + instance.props = current.memoizedProps; + if (!instance.shouldComponentUpdate(props)) { + return bailoutOnCurrent(current, workInProgress); + } + } + if (!workInProgress.hasWorkInProgress && workInProgress.memoizedProps) { + // Reset the props, in case this is a ping-pong case rather than a + // completed update case. For the completed update case, the instance + // props will already be the memoizedProps. + instance.props = workInProgress.memoizedProps; + if (!instance.shouldComponentUpdate(props)) { + return bailoutOnAlreadyFinishedWork(workInProgress); + } + } } - var instance = workInProgress.stateNode; instance.props = props; var nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); workInProgress.pendingWorkPriority = NoWork; + return workInProgress.child; } function updateHostComponent(current, workInProgress) { @@ -165,90 +183,103 @@ module.exports = function(config : HostConfig) { } while (child = child.sibling); } + function bailoutOnCurrent(current : Fiber, workInProgress : Fiber) : ?Fiber { + // The most likely scenario is that the previous copy of the tree contains + // the same props as the new one. In that case, we can just copy the output + // and children from that node. + workInProgress.memoizedProps = workInProgress.pendingProps; + workInProgress.output = current.output; + const priorityLevel = workInProgress.pendingWorkPriority; + workInProgress.pendingProps = null; + workInProgress.pendingWorkPriority = NoWork; + workInProgress.stateNode = current.stateNode; + if (current.child) { + // If we bail out but still has work with the current priority in this + // subtree, we need to go find it right now. If we don't, we won't flush + // it until the next tick. + workInProgress.child = current.child; + reuseChildren(workInProgress, workInProgress.child); + if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) { + // TODO: This passes the current node and reads the priority level and + // pending props from that. We want it to read our priority level and + // pending props from the work in progress. Needs restructuring. + return findNextUnitOfWorkAtPriority(current, priorityLevel); + } else { + return null; + } + } else { + workInProgress.child = null; + return null; + } + } + + function bailoutOnAlreadyFinishedWork(workInProgress : Fiber) : ?Fiber { + // If we started this work before, and finished it, or if we're in a + // ping-pong update scenario, this version could already be what we're + // looking for. In that case, we should be able to just bail out. + const priorityLevel = workInProgress.pendingWorkPriority; + workInProgress.pendingProps = null; + workInProgress.pendingWorkPriority = NoWork; + + workInProgress.firstEffect = null; + workInProgress.nextEffect = null; + workInProgress.lastEffect = null; + + if (workInProgress.child && workInProgress.child.alternate) { + // On the way up here, we reset the child node to be the current one. + // Therefore we have to reuse the alternate. This is super weird. + let child = workInProgress.child.alternate; + workInProgress.child = child; + // Ensure that the effects of reused work are preserved. + reuseChildrenEffects(workInProgress, child); + // If we bail out but still has work with the current priority in this + // subtree, we need to go find it right now. If we don't, we won't flush + // it until the next tick. + reuseChildren(workInProgress, child); + if (workInProgress.pendingWorkPriority !== NoWork && + workInProgress.pendingWorkPriority <= priorityLevel) { + // TODO: This passes the current node and reads the priority level and + // pending props from that. We want it to read our priority level and + // pending props from the work in progress. Needs restructuring. + return findNextUnitOfWorkAtPriority(workInProgress, priorityLevel); + } + } + return null; + } + function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here // means that we don't need an additional field on the work in // progress. if (current && workInProgress.pendingProps === current.memoizedProps) { - // The most likely scenario is that the previous copy of the tree contains - // the same props as the new one. In that case, we can just copy the output - // and children from that node. - workInProgress.memoizedProps = workInProgress.pendingProps; - workInProgress.output = current.output; - const priorityLevel = workInProgress.pendingWorkPriority; - workInProgress.pendingProps = null; - workInProgress.pendingWorkPriority = NoWork; - workInProgress.stateNode = current.stateNode; - if (current.child) { - // If we bail out but still has work with the current priority in this - // subtree, we need to go find it right now. If we don't, we won't flush - // it until the next tick. - workInProgress.child = current.child; - reuseChildren(workInProgress, workInProgress.child); - if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) { - // TODO: This passes the current node and reads the priority level and - // pending props from that. We want it to read our priority level and - // pending props from the work in progress. Needs restructuring. - return findNextUnitOfWorkAtPriority(workInProgress.alternate, priorityLevel); - } else { - return null; - } - } else { - workInProgress.child = null; - return null; - } + return bailoutOnCurrent(current, workInProgress); } if (!workInProgress.hasWorkInProgress && workInProgress.pendingProps === workInProgress.memoizedProps) { - // If we started this work before, and finished it, or if we're in a - // ping-pong update scenario, this version could already be what we're - // looking for. In that case, we should be able to just bail out. - const priorityLevel = workInProgress.pendingWorkPriority; - workInProgress.pendingProps = null; - workInProgress.pendingWorkPriority = NoWork; - - workInProgress.firstEffect = null; - workInProgress.nextEffect = null; - workInProgress.lastEffect = null; - - if (workInProgress.child && workInProgress.child.alternate) { - // On the way up here, we reset the child node to be the current one. - // Therefore we have to reuse the alternate. This is super weird. - workInProgress.child = workInProgress.child.alternate; - // Ensure that the effects of reused work are preserved. - reuseChildrenEffects(workInProgress, workInProgress.child); - // If we bail out but still has work with the current priority in this - // subtree, we need to go find it right now. If we don't, we won't flush - // it until the next tick. - reuseChildren(workInProgress, workInProgress.child); - if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) { - // TODO: This passes the current node and reads the priority level and - // pending props from that. We want it to read our priority level and - // pending props from the work in progress. Needs restructuring. - return findNextUnitOfWorkAtPriority(workInProgress, priorityLevel); - } - } - return null; + return bailoutOnAlreadyFinishedWork(workInProgress); } - workInProgress.hasWorkInProgress = true; - + let nextWork; switch (workInProgress.tag) { case IndeterminateComponent: mountIndeterminateComponent(current, workInProgress); + workInProgress.hasWorkInProgress = true; return workInProgress.child; case FunctionalComponent: updateFunctionalComponent(current, workInProgress); + workInProgress.hasWorkInProgress = true; return workInProgress.child; case ClassComponent: - updateClassComponent(current, workInProgress); - return workInProgress.child; + nextWork = updateClassComponent(current, workInProgress); + workInProgress.hasWorkInProgress = true; + return nextWork; case HostContainer: reconcileChildren(current, workInProgress, workInProgress.pendingProps); // A yield component is just a placeholder, we can just run through the // next one immediately. + workInProgress.hasWorkInProgress = true; workInProgress.pendingWorkPriority = NoWork; if (workInProgress.child) { return beginWork( @@ -258,13 +289,16 @@ module.exports = function(config : HostConfig) { } return null; case HostComponent: - return updateHostComponent(current, workInProgress); + nextWork = updateHostComponent(current, workInProgress); + workInProgress.hasWorkInProgress = true; + return nextWork; case CoroutineHandlerPhase: // This is a restart. Reset the tag to the initial phase. workInProgress.tag = CoroutineComponent; // Intentionally fall through since this is now the same. case CoroutineComponent: updateCoroutineComponent(current, workInProgress); + workInProgress.hasWorkInProgress = true; // This doesn't take arbitrary time so we could synchronously just begin // eagerly do the work of workInProgress.child as an optimization. if (workInProgress.child) { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 4158724be28b7..29c1f0968d86d 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -291,11 +291,16 @@ describe('ReactIncremental', function() { return
{props.children}
; } - function Tester() { - // This component is just here to ensure that the bail out is - // in fact in effect in the expected place for this test. - ops.push('Tester'); - return
; + class Tester extends React.Component { + shouldComponentUpdate() { + return false; + } + render() { + // This component is just here to ensure that the bail out is + // in fact in effect in the expected place for this test. + ops.push('Tester'); + return
; + } } function Middle(props) { @@ -303,23 +308,28 @@ describe('ReactIncremental', function() { return {props.children}; } - var middleContent = ( - - - - - ); + class Content extends React.Component { + shouldComponentUpdate() { + return false; + } + render() { + return [ + , + , + ]; + } + } function Foo(props) { ops.push('Foo'); return ( ); @@ -434,4 +444,99 @@ describe('ReactIncremental', function() { }); + it('can reuse work if shouldComponentUpdate is false, after being preempted', function() { + + var ops = []; + + function Bar(props) { + ops.push('Bar'); + return
{props.children}
; + } + + class Middle extends React.Component { + shouldComponentUpdate(nextProps) { + return this.props.children !== nextProps.children; + } + render() { + ops.push('Middle'); + return {this.props.children}; + } + } + + class Content extends React.Component { + shouldComponentUpdate(nextProps) { + return this.props.step !== nextProps.step; + } + render() { + ops.push('Content'); + return ( +
+ {this.props.step === 0 ? 'Hi' : 'Hello'} + {this.props.step === 0 ? this.props.text : '-'} + {this.props.step === 0 ? 'There' : 'World'} +
+ ); + } + } + + function Foo(props) { + ops.push('Foo'); + return ( +
+ {props.text} + +
+ ); + } + + // Init + ReactNoop.render(); + ReactNoop.flush(); + + expect(ops).toEqual(['Foo', 'Bar', 'Content', 'Middle', 'Bar', 'Middle']); + + ops = []; + + // Make a quick update which will schedule low priority work to + // update the middle content. + ReactNoop.render(); + ReactNoop.flushLowPri(30); + + expect(ops).toEqual(['Foo', 'Bar']); + + ops = []; + + // The middle content is now pending rendering... + ReactNoop.flushLowPri(30); + expect(ops).toEqual(['Content', 'Middle', 'Bar']); // One more Middle left. + + ops = []; + + // but we'll interupt it to render some higher priority work. + // The middle content will bailout so it remains untouched. + ReactNoop.render(); + ReactNoop.flushLowPri(30); + + expect(ops).toEqual(['Foo', 'Bar']); + + ops = []; + + // Since we did nothing to the middle subtree during the interuption, + // we should be able to reuse the reconciliation work that we already did + // without restarting. + ReactNoop.flush(); + // TODO: Content never fully completed its render so can't completely bail + // out on the entire subtree. However, we could do a shallow bail out and + // not rerender Content, but keep going down the incomplete tree. + // Normally shouldComponentUpdate->false is not enough to determine that we + // can safely reuse the old props, but I think in this case it would be ok, + // since it is a resume of already started work. + // Because of the above we can also not reuse the work of Bar because the + // rerender of Content will generate a new element which will mean we don't + // auto-bail out from Bar. + expect(ops).toEqual(['Content', 'Bar', 'Middle']); + + }); }); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 68360c62885a8..c6068855610eb 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -188,6 +188,72 @@ describe('ReactIncrementalSideEffects', function() { ]); }); + it('can reuse side-effects after being preempted, if shouldComponentUpdate is false', function() { + + class Bar extends React.Component { + shouldComponentUpdate(nextProps) { + return this.props.children !== nextProps.children; + } + render() { + return ; + } + } + + class Content extends React.Component { + shouldComponentUpdate(nextProps) { + return this.props.step !== nextProps.step; + } + render() { + return ( +
+ {this.props.step === 0 ? 'Hi' : 'Hello'} + {this.props.step === 0 ? this.props.text : 'World'} +
+ ); + } + } + + function Foo(props) { + return ( + + ); + } + + // Init + ReactNoop.render(); + ReactNoop.flush(); + + expect(ReactNoop.root.children).toEqual([ + div(div(span('Hi'), span('foo'))), + ]); + + // Make a quick update which will schedule low priority work to + // update the middle content. + ReactNoop.render(); + ReactNoop.flushLowPri(35); + + // The tree remains unchanged. + expect(ReactNoop.root.children).toEqual([ + div(div(span('Hi'), span('foo'))), + ]); + + // The first Bar has already completed its update but we'll interupt it to + // render some higher priority work. The middle content will bailout so + // it remains untouched which means that it should reuse it next time. + ReactNoop.render(); + ReactNoop.flush(30); + + // Since we did nothing to the middle subtree during the interuption, + // we should be able to reuse the reconciliation work that we already did + // without restarting. The side-effects should still be replayed. + + expect(ReactNoop.root.children).toEqual([ + div(div(span('Hello'), span('World'))), + ]); + }); + it('updates a child even though the old props is empty', function() { function Foo(props) { return ( From af7dd8e0a13600870a18b244608cfa37da5ae21c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 5 Aug 2016 14:33:48 -0700 Subject: [PATCH 13/13] Move wip fibers to childInProgress This took a while to figure out, but we need to be able to store children that are currently being worked on separately from the current children. We always need a canonical "current" children so that we can update them. However, we also need a different set that we're currently working on so that we have a way to get to already progressed work. This solve the starvation problem in the first render because now we can reach children that were never rendered and have a place to store their progressed work on. The unit test changes tests this. This lets us get rid of the hasWorkInProgress flag. When we reconcile new children we need to reconcile them against progressed work so that we can reuse it. The progressed work is "work in progress" nodes. So in that case we need to mutate instead of clone, to preserve the invariant that only two versions exist at any given point. This effectively forks the ReactChildFiber implementation. --- src/renderers/noop/ReactNoop.js | 14 +- src/renderers/shared/fiber/ReactChildFiber.js | 271 ++++++++++-------- src/renderers/shared/fiber/ReactFiber.js | 21 +- .../shared/fiber/ReactFiberBeginWork.js | 120 ++++---- .../shared/fiber/ReactFiberCommitWork.js | 9 +- .../shared/fiber/ReactFiberCompleteWork.js | 9 +- .../shared/fiber/ReactFiberPendingWork.js | 30 +- .../shared/fiber/ReactFiberScheduler.js | 19 +- .../fiber/__tests__/ReactIncremental-test.js | 41 ++- 9 files changed, 306 insertions(+), 228 deletions(-) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 70598bc5b7f69..82e64b53579b4 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -167,9 +167,21 @@ var ReactNoop = { function logFiber(fiber : Fiber, depth) { console.log( - ' '.repeat(depth) + '- ' + (fiber.type ? fiber.type.name || fiber.type : '[root]'), + ' '.repeat(depth) + '- ' + (fiber.type ? fiber.type.name || fiber.type : '[root]'), '[' + fiber.pendingWorkPriority + (fiber.pendingProps ? '*' : '') + ']' ); + const childInProgress = fiber.childInProgress; + if (childInProgress) { + if (childInProgress === fiber.child) { + console.log(' '.repeat(depth + 1) + 'ERROR: IN PROGRESS == CURRENT'); + } else { + console.log(' '.repeat(depth + 1) + 'IN PROGRESS'); + logFiber(childInProgress, depth + 1); + if (fiber.child) { + console.log(' '.repeat(depth + 1) + 'CURRENT'); + } + } + } if (fiber.child) { logFiber(fiber.child, depth + 1); } diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index c5ca161a01188..c3621c30ad2a2 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -42,151 +42,168 @@ const { const isArray = Array.isArray; -function createSubsequentChild( - returnFiber : Fiber, - existingChild : ?Fiber, - previousSibling : Fiber, - newChildren, - priority : PriorityLevel -) : Fiber { - if (typeof newChildren !== 'object' || newChildren === null) { - return previousSibling; - } +function ChildReconciler(shouldClone) { + + function createSubsequentChild( + returnFiber : Fiber, + existingChild : ?Fiber, + previousSibling : Fiber, + newChildren, + priority : PriorityLevel + ) : Fiber { + if (typeof newChildren !== 'object' || newChildren === null) { + return previousSibling; + } - switch (newChildren.$$typeof) { - case REACT_ELEMENT_TYPE: { - const element = (newChildren : ReactElement); - if (existingChild && - element.type === existingChild.type && - element.key === existingChild.key) { - // TODO: This is not sufficient since previous siblings could be new. - // Will fix reconciliation properly later. - const clone = cloneFiber(existingChild, priority); - clone.pendingProps = element.props; - clone.child = existingChild.child; - clone.sibling = null; - clone.return = returnFiber; - previousSibling.sibling = clone; - return clone; + switch (newChildren.$$typeof) { + case REACT_ELEMENT_TYPE: { + const element = (newChildren : ReactElement); + if (existingChild && + element.type === existingChild.type && + element.key === existingChild.key) { + // TODO: This is not sufficient since previous siblings could be new. + // Will fix reconciliation properly later. + const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild; + if (!shouldClone) { + clone.pendingWorkPriority = priority; + } + clone.pendingProps = element.props; + clone.child = existingChild.child; + clone.sibling = null; + clone.return = returnFiber; + previousSibling.sibling = clone; + return clone; + } + const child = createFiberFromElement(element, priority); + previousSibling.sibling = child; + child.return = returnFiber; + return child; } - const child = createFiberFromElement(element, priority); - previousSibling.sibling = child; - child.return = returnFiber; - return child; - } - case REACT_COROUTINE_TYPE: { - const coroutine = (newChildren : ReactCoroutine); - const child = createFiberFromCoroutine(coroutine, priority); - previousSibling.sibling = child; - child.return = returnFiber; - return child; - } + case REACT_COROUTINE_TYPE: { + const coroutine = (newChildren : ReactCoroutine); + const child = createFiberFromCoroutine(coroutine, priority); + previousSibling.sibling = child; + child.return = returnFiber; + return child; + } - case REACT_YIELD_TYPE: { - const yieldNode = (newChildren : ReactYield); - const reifiedYield = createReifiedYield(yieldNode); - const child = createFiberFromYield(yieldNode, priority); - child.output = reifiedYield; - previousSibling.sibling = child; - child.return = returnFiber; - return child; + case REACT_YIELD_TYPE: { + const yieldNode = (newChildren : ReactYield); + const reifiedYield = createReifiedYield(yieldNode); + const child = createFiberFromYield(yieldNode, priority); + child.output = reifiedYield; + previousSibling.sibling = child; + child.return = returnFiber; + return child; + } } - } - if (isArray(newChildren)) { - let prev : Fiber = previousSibling; - let existing : ?Fiber = existingChild; - for (var i = 0; i < newChildren.length; i++) { - prev = createSubsequentChild(returnFiber, existing, prev, newChildren[i], priority); - if (prev && existing) { - // TODO: This is not correct because there could've been more - // than one sibling consumed but I don't want to return a tuple. - existing = existing.sibling; + if (isArray(newChildren)) { + let prev : Fiber = previousSibling; + let existing : ?Fiber = existingChild; + for (var i = 0; i < newChildren.length; i++) { + var nextExisting = existing && existing.sibling; + prev = createSubsequentChild(returnFiber, existing, prev, newChildren[i], priority); + if (prev && existing) { + // TODO: This is not correct because there could've been more + // than one sibling consumed but I don't want to return a tuple. + existing = nextExisting; + } } + return prev; + } else { + // TODO: Throw for unknown children. + return previousSibling; } - return prev; - } else { - // TODO: Throw for unknown children. - return previousSibling; } -} -function createFirstChild(returnFiber, existingChild, newChildren, priority) { - if (typeof newChildren !== 'object' || newChildren === null) { - return null; - } + function createFirstChild(returnFiber, existingChild, newChildren, priority) { + if (typeof newChildren !== 'object' || newChildren === null) { + return null; + } - switch (newChildren.$$typeof) { - case REACT_ELEMENT_TYPE: { - const element = (newChildren : ReactElement); - if (existingChild && - element.type === existingChild.type && - element.key === existingChild.key) { - // Get the clone of the existing fiber. - const clone = cloneFiber(existingChild, priority); - clone.pendingProps = element.props; - clone.child = existingChild.child; - clone.sibling = null; - clone.return = returnFiber; - return clone; + switch (newChildren.$$typeof) { + case REACT_ELEMENT_TYPE: { + const element = (newChildren : ReactElement); + if (existingChild && + element.type === existingChild.type && + element.key === existingChild.key) { + // Get the clone of the existing fiber. + const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild; + if (!shouldClone) { + clone.pendingWorkPriority = priority; + } + clone.pendingProps = element.props; + clone.child = existingChild.child; + clone.sibling = null; + clone.return = returnFiber; + return clone; + } + const child = createFiberFromElement(element, priority); + child.return = returnFiber; + return child; } - const child = createFiberFromElement(element, priority); - child.return = returnFiber; - return child; - } - case REACT_COROUTINE_TYPE: { - const coroutine = (newChildren : ReactCoroutine); - const child = createFiberFromCoroutine(coroutine, priority); - child.return = returnFiber; - return child; - } + case REACT_COROUTINE_TYPE: { + const coroutine = (newChildren : ReactCoroutine); + const child = createFiberFromCoroutine(coroutine, priority); + child.return = returnFiber; + return child; + } - case REACT_YIELD_TYPE: { - // A yield results in a fragment fiber whose output is the continuation. - // TODO: When there is only a single child, we can optimize this to avoid - // the fragment. - const yieldNode = (newChildren : ReactYield); - const reifiedYield = createReifiedYield(yieldNode); - const child = createFiberFromYield(yieldNode, priority); - child.output = reifiedYield; - child.return = returnFiber; - return child; + case REACT_YIELD_TYPE: { + // A yield results in a fragment fiber whose output is the continuation. + // TODO: When there is only a single child, we can optimize this to avoid + // the fragment. + const yieldNode = (newChildren : ReactYield); + const reifiedYield = createReifiedYield(yieldNode); + const child = createFiberFromYield(yieldNode, priority); + child.output = reifiedYield; + child.return = returnFiber; + return child; + } } - } - if (isArray(newChildren)) { - var first : ?Fiber = null; - var prev : ?Fiber = null; - var existing : ?Fiber = existingChild; - for (var i = 0; i < newChildren.length; i++) { - if (prev == null) { - prev = createFirstChild(returnFiber, existing, newChildren[i], priority); - first = prev; - } else { - prev = createSubsequentChild(returnFiber, existing, prev, newChildren[i], priority); - } - if (prev && existing) { - // TODO: This is not correct because there could've been more - // than one sibling consumed but I don't want to return a tuple. - existing = existing.sibling; + if (isArray(newChildren)) { + var first : ?Fiber = null; + var prev : ?Fiber = null; + var existing : ?Fiber = existingChild; + for (var i = 0; i < newChildren.length; i++) { + var nextExisting = existing && existing.sibling; + if (prev == null) { + prev = createFirstChild(returnFiber, existing, newChildren[i], priority); + first = prev; + } else { + prev = createSubsequentChild(returnFiber, existing, prev, newChildren[i], priority); + } + if (prev && existing) { + // TODO: This is not correct because there could've been more + // than one sibling consumed but I don't want to return a tuple. + existing = nextExisting; + } } + return first; + } else { + // TODO: Throw for unknown children. + return null; } - return first; - } else { - // TODO: Throw for unknown children. - return null; } + + // TODO: This API won't work because we'll need to transfer the side-effects of + // unmounting children to the returnFiber. + function reconcileChildFibers( + returnFiber : Fiber, + currentFirstChild : ?Fiber, + newChildren : ReactNodeList, + priority : PriorityLevel + ) : ?Fiber { + return createFirstChild(returnFiber, currentFirstChild, newChildren, priority); + } + + return reconcileChildFibers; } -// TODO: This API won't work because we'll need to transfer the side-effects of -// unmounting children to the returnFiber. -exports.reconcileChildFibers = function( - returnFiber : Fiber, - currentFirstChild : ?Fiber, - newChildren : ReactNodeList, - priority : PriorityLevel -) : ?Fiber { - return createFirstChild(returnFiber, currentFirstChild, newChildren, priority); -}; +exports.reconcileChildFibers = ChildReconciler(true); + +exports.reconcileChildFibersInPlace = ChildReconciler(false); diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 02bb854c228c0..2bcfe3f496b2a 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -100,17 +100,9 @@ export type Fiber = Instance & { // memory if we need to. alternate: ?Fiber, - // Keeps track if we've completed this node or if we're currently in the - // middle of processing it. We really should know this based on pendingProps - // or something else. We could also reuse the tag for this purpose. However, - // I'm not really sure so I'll use a flag for now. - // TODO: Find another way to infer this flag. - hasWorkInProgress: bool, - // Keeps track if we've deprioritized the children of this node so we know to - // reuse the existing children. - // TODO: Find another way to infer this flag or separate children updates from - // property updates so that we simply don't try to update children here. - wasDeprioritized: bool, + // Keeps track of the children that are currently being processed but have not + // yet completed. + childInProgress: ?Fiber, // Conceptual aliases // workInProgress : Fiber -> alternate The alternate used for reuse happens @@ -150,8 +142,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { pendingWorkPriority: NoWork, - hasWorkInProgress: false, - wasDeprioritized: false, + childInProgress: null, alternate: null, @@ -173,6 +164,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi if (alt) { alt.stateNode = fiber.stateNode; alt.child = fiber.child; + alt.childInProgress = fiber.childInProgress; alt.sibling = fiber.sibling; alt.ref = alt.ref; alt.pendingProps = fiber.pendingProps; @@ -192,6 +184,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi alt.type = fiber.type; alt.stateNode = fiber.stateNode; alt.child = fiber.child; + alt.childInProgress = fiber.childInProgress; alt.sibling = fiber.sibling; alt.ref = alt.ref; // pendingProps is here for symmetry but is unnecessary in practice for now. @@ -232,7 +225,7 @@ function createFiberFromElementType(type : mixed, key : null | string) { throw new Error('Unknown component type: ' + typeof type); } return fiber; -}; +} exports.createFiberFromElementType = createFiberFromElementType; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 6d841b075b509..6c664784f07b1 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -16,7 +16,10 @@ import type { ReactCoroutine } from 'ReactCoroutine'; import type { Fiber } from 'ReactFiber'; import type { HostConfig } from 'ReactFiberReconciler'; -var { reconcileChildFibers } = require('ReactChildFiber'); +var { + reconcileChildFibers, + reconcileChildFibersInPlace, +} = require('ReactChildFiber'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { IndeterminateComponent, @@ -38,12 +41,34 @@ module.exports = function(config : HostConfig) { function reconcileChildren(current, workInProgress, nextChildren) { const priority = workInProgress.pendingWorkPriority; - workInProgress.child = reconcileChildFibers( - workInProgress, - current ? current.child : null, - nextChildren, - priority - ); + reconcileChildrenAtPriority(current, workInProgress, nextChildren, priority); + } + + function reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel) { + if (current && current.childInProgress) { + workInProgress.childInProgress = reconcileChildFibersInPlace( + workInProgress, + current.childInProgress, + nextChildren, + priorityLevel + ); + // This is now invalid because we reused nodes. + current.childInProgress = null; + } else if (workInProgress.childInProgress) { + workInProgress.childInProgress = reconcileChildFibersInPlace( + workInProgress, + workInProgress.childInProgress, + nextChildren, + priorityLevel + ); + } else { + workInProgress.childInProgress = reconcileChildFibers( + workInProgress, + current ? current.child : null, + nextChildren, + priorityLevel + ); + } } function updateFunctionalComponent(current, workInProgress) { @@ -68,13 +93,13 @@ module.exports = function(config : HostConfig) { return bailoutOnCurrent(current, workInProgress); } } - if (!workInProgress.hasWorkInProgress && workInProgress.memoizedProps) { + if (!workInProgress.childInProgress && workInProgress.memoizedProps) { // Reset the props, in case this is a ping-pong case rather than a // completed update case. For the completed update case, the instance // props will already be the memoizedProps. instance.props = workInProgress.memoizedProps; if (!instance.shouldComponentUpdate(props)) { - return bailoutOnAlreadyFinishedWork(workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress); } } } @@ -82,7 +107,7 @@ module.exports = function(config : HostConfig) { var nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); workInProgress.pendingWorkPriority = NoWork; - return workInProgress.child; + return workInProgress.childInProgress; } function updateHostComponent(current, workInProgress) { @@ -93,25 +118,19 @@ module.exports = function(config : HostConfig) { // If this host component is hidden, we can reconcile its children at // the lowest priority and bail out from this particular pass. Unless, we're // currently reconciling the lowest priority. - workInProgress.child = reconcileChildFibers( - workInProgress, - current ? current.child : null, - nextChildren, - OffscreenPriority - ); + // If we have a child in progress already, we reconcile against that set + // to retain any work within it. We'll recreate any component that was in + // the current set and next set but not in the previous in progress set. + // TODO: This attaches a node that hasn't completed rendering so it + // becomes part of the render tree, even though it never completed. Its + // `output` property is unpredictable because of it. + reconcileChildrenAtPriority(current, workInProgress, nextChildren, OffscreenPriority); workInProgress.pendingWorkPriority = OffscreenPriority; - workInProgress.wasDeprioritized = true; return null; } else { - workInProgress.child = reconcileChildFibers( - workInProgress, - current ? current.child : null, - nextChildren, - priority - ); + reconcileChildren(current, workInProgress, nextChildren); workInProgress.pendingWorkPriority = NoWork; - workInProgress.wasDeprioritized = false; - return workInProgress.child; + return workInProgress.childInProgress; } } @@ -193,6 +212,7 @@ module.exports = function(config : HostConfig) { workInProgress.pendingProps = null; workInProgress.pendingWorkPriority = NoWork; workInProgress.stateNode = current.stateNode; + workInProgress.childInProgress = current.childInProgress; if (current.child) { // If we bail out but still has work with the current priority in this // subtree, we need to go find it right now. If we don't, we won't flush @@ -213,7 +233,7 @@ module.exports = function(config : HostConfig) { } } - function bailoutOnAlreadyFinishedWork(workInProgress : Fiber) : ?Fiber { + function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber { // If we started this work before, and finished it, or if we're in a // ping-pong update scenario, this version could already be what we're // looking for. In that case, we should be able to just bail out. @@ -225,10 +245,15 @@ module.exports = function(config : HostConfig) { workInProgress.nextEffect = null; workInProgress.lastEffect = null; - if (workInProgress.child && workInProgress.child.alternate) { - // On the way up here, we reset the child node to be the current one. - // Therefore we have to reuse the alternate. This is super weird. - let child = workInProgress.child.alternate; + if (workInProgress.child) { + // On the way up here, we reset the child node to be the current one by + // cloning. However, it is really the original child that represents the + // already completed work. Therefore we have to reuse the alternate. + // But if we don't have a current, this was not cloned. This is super weird. + const child = !current ? workInProgress.child : workInProgress.child.alternate; + if (!child) { + throw new Error('We must have a current child to be able to use this.'); + } workInProgress.child = child; // Ensure that the effects of reused work are preserved. reuseChildrenEffects(workInProgress, child); @@ -256,58 +281,49 @@ module.exports = function(config : HostConfig) { return bailoutOnCurrent(current, workInProgress); } - if (!workInProgress.hasWorkInProgress && + if (!workInProgress.childInProgress && workInProgress.pendingProps === workInProgress.memoizedProps) { - return bailoutOnAlreadyFinishedWork(workInProgress); + return bailoutOnAlreadyFinishedWork(current, workInProgress); } - let nextWork; switch (workInProgress.tag) { case IndeterminateComponent: mountIndeterminateComponent(current, workInProgress); - workInProgress.hasWorkInProgress = true; - return workInProgress.child; + return workInProgress.childInProgress; case FunctionalComponent: updateFunctionalComponent(current, workInProgress); - workInProgress.hasWorkInProgress = true; - return workInProgress.child; + return workInProgress.childInProgress; case ClassComponent: - nextWork = updateClassComponent(current, workInProgress); - workInProgress.hasWorkInProgress = true; - return nextWork; + return updateClassComponent(current, workInProgress); case HostContainer: reconcileChildren(current, workInProgress, workInProgress.pendingProps); // A yield component is just a placeholder, we can just run through the // next one immediately. - workInProgress.hasWorkInProgress = true; workInProgress.pendingWorkPriority = NoWork; - if (workInProgress.child) { + if (workInProgress.childInProgress) { return beginWork( - workInProgress.child.alternate, - workInProgress.child + workInProgress.childInProgress.alternate, + workInProgress.childInProgress ); } return null; case HostComponent: - nextWork = updateHostComponent(current, workInProgress); - workInProgress.hasWorkInProgress = true; - return nextWork; + return updateHostComponent(current, workInProgress); case CoroutineHandlerPhase: // This is a restart. Reset the tag to the initial phase. workInProgress.tag = CoroutineComponent; // Intentionally fall through since this is now the same. case CoroutineComponent: updateCoroutineComponent(current, workInProgress); - workInProgress.hasWorkInProgress = true; // This doesn't take arbitrary time so we could synchronously just begin // eagerly do the work of workInProgress.child as an optimization. - if (workInProgress.child) { + if (workInProgress.childInProgress) { return beginWork( - workInProgress.child.alternate, - workInProgress.child + workInProgress.childInProgress.alternate, + workInProgress.childInProgress ); } - return workInProgress.child; + return workInProgress.childInProgress; case YieldComponent: // A yield component is just a placeholder, we can just run through the // next one immediately. diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 5a5a6ae74dcd8..c7a9f2f71098f 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -47,14 +47,7 @@ module.exports = function(config : HostConfig) { throw new Error('This should only be done during updates.'); } // Commit the work prepared earlier. - let child; - if (finishedWork.wasDeprioritized) { - // If this was a down priority, we need to preserve the old child in - // the output. - child = finishedWork.alternate ? finishedWork.alternate.child : null; - } else { - child = finishedWork.child; - } + const child = finishedWork.child; const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child; const newProps = finishedWork.memoizedProps; const current = finishedWork.alternate; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index fd0cb31f2da21..53421116b48c3 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -143,15 +143,8 @@ module.exports = function(config : HostConfig) { markForPreEffect(workInProgress); return null; case HostComponent: - let child; let newProps = workInProgress.pendingProps; - if (workInProgress.wasDeprioritized) { - // If this was a down priority, we need to preserve the old child in - // the output. - child = current ? current.child : null; - } else { - child = workInProgress.child; - } + const child = workInProgress.child; const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to diff --git a/src/renderers/shared/fiber/ReactFiberPendingWork.js b/src/renderers/shared/fiber/ReactFiberPendingWork.js index bf837c0426ca8..eb61c4595fffa 100644 --- a/src/renderers/shared/fiber/ReactFiberPendingWork.js +++ b/src/renderers/shared/fiber/ReactFiberPendingWork.js @@ -56,9 +56,26 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev // because it is the highest priority for the whole subtree. // TODO: Coroutines can have work in their stateNode which is another // type of child that needs to be searched for work. - if (current.child) { - // Ensure we have a work in progress copy to backtrack through. + if (current.childInProgress) { + let workInProgress = current.childInProgress; + while (workInProgress) { + workInProgress.return = current.alternate; + workInProgress = workInProgress.sibling; + } + workInProgress = current.childInProgress; + while (workInProgress) { + // Don't bother drilling further down this tree if there is no child. + if (workInProgress.pendingWorkPriority !== NoWork && + workInProgress.pendingWorkPriority <= priorityLevel && + workInProgress.pendingProps !== null) { + return workInProgress; + } + workInProgress = workInProgress.sibling; + } + } else if (current.child) { let currentChild = current.child; + currentChild.return = current; + // Ensure we have a work in progress copy to backtrack through. let workInProgress = current.alternate; if (!workInProgress) { throw new Error('Should have wip now'); @@ -74,7 +91,7 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev currentChild.pendingWorkPriority ); cloneSiblings(currentChild, workInProgress.child, workInProgress); - current = current.child; + current = currentChild; continue; } // If we match the priority but has no child and no work to do, @@ -82,6 +99,12 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev current.pendingWorkPriority = NoWork; } if (current === currentRoot) { + if (current.pendingWorkPriority <= priorityLevel) { + // If this subtree had work left to do, we would have returned it by + // now. This could happen if a child with pending work gets cleaned up + // but we don't clear the flag then. It is safe to reset it now. + current.pendingWorkPriority = NoWork; + } return null; } while (!current.sibling) { @@ -96,6 +119,7 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev current.pendingWorkPriority = NoWork; } } + current.sibling.return = current.return; current = current.sibling; } return null; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 1bf5150ac8919..b6e71289689eb 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -62,6 +62,8 @@ module.exports = function(config : HostConfig) { } nextScheduledRoot = nextScheduledRoot.nextScheduledRoot; } + // TODO: This is scanning one root at a time. It should be scanning all + // roots for high priority work before moving on to lower priorities. let root = nextScheduledRoot; while (root) { cloneFiber(root.current, root.current.pendingWorkPriority); @@ -87,6 +89,11 @@ module.exports = function(config : HostConfig) { // We didn't find anything to do in this root, so let's try the next one. root = root.nextScheduledRoot; } + root = nextScheduledRoot; + while (root) { + root = root.nextScheduledRoot; + } + nextPriorityLevel = NoWork; return null; } @@ -119,9 +126,6 @@ module.exports = function(config : HostConfig) { // The work is now done. We don't need this anymore. This flags // to the system not to redo any work here. workInProgress.pendingProps = null; - if (workInProgress.pendingWorkPriority === NoWork) { - workInProgress.hasWorkInProgress = false; - } const returnFiber = workInProgress.return; @@ -155,6 +159,15 @@ module.exports = function(config : HostConfig) { } else if (returnFiber) { // If there's no more work in this returnFiber. Complete the returnFiber. workInProgress = returnFiber; + // If we're stepping up through the child, that means we can now commit + // this work. We should only do this when we're stepping upwards because + // completing a downprioritized item is not the same as completing its + // children. + if (workInProgress.childInProgress) { + workInProgress.child = workInProgress.childInProgress; + workInProgress.childInProgress = null; + } + continue; } else { // If we're at the root, there's no more work to do. We can flush it. const root : FiberRoot = (workInProgress.stateNode : any); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 29c1f0968d86d..65d3b09e0abb4 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -283,7 +283,6 @@ describe('ReactIncremental', function() { }); it('can resume work in a bailed subtree within one pass', function() { - var ops = []; function Bar(props) { @@ -308,6 +307,8 @@ describe('ReactIncremental', function() { return {props.children}; } + // Should content not just bail out on current, not workInProgress? + class Content extends React.Component { shouldComponentUpdate() { return false; @@ -359,7 +360,6 @@ describe('ReactIncremental', function() { // after them which is not correct. ReactNoop.flush(); expect(ops).toEqual(['Bar', 'Middle', 'Bar']); - }); it('can reuse work done after being preempted', function() { @@ -384,19 +384,23 @@ describe('ReactIncremental', function() {
); + var step0 = ( +
+ Hi + {'Foo'} + There +
+ ); + function Foo(props) { ops.push('Foo'); return (
- {props.text} + {props.text2} @@ -405,16 +409,29 @@ describe('ReactIncremental', function() { } // Init - ReactNoop.render(); + ReactNoop.render(); + ReactNoop.flushLowPri(55); + + // We only finish the higher priority work. So the low pri content + // has not yet finished mounting. + expect(ops).toEqual(['Foo', 'Bar', 'Middle', 'Bar']); + + ops = []; + + // Interupt the rendering with a quick update. This should not touch the + // middle content. + ReactNoop.render(); ReactNoop.flush(); - expect(ops).toEqual(['Foo', 'Bar', 'Middle', 'Bar', 'Middle']); + // We've now rendered the entire tree but we didn't have to redo the work + // done by the first Middle and Bar already. + expect(ops).toEqual(['Foo', 'Bar', 'Middle']); ops = []; // Make a quick update which will schedule low priority work to // update the middle content. - ReactNoop.render(); + ReactNoop.render(); ReactNoop.flushLowPri(30); expect(ops).toEqual(['Foo', 'Bar']); @@ -429,7 +446,7 @@ describe('ReactIncremental', function() { // but we'll interupt it to render some higher priority work. // The middle content will bailout so it remains untouched. - ReactNoop.render(); + ReactNoop.render(); ReactNoop.flushLowPri(30); expect(ops).toEqual(['Foo', 'Bar']);