diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index d6733db76eafb..a9d16d92ebc4a 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1143,7 +1143,9 @@ src/renderers/shared/__tests__/ReactPerf-test.js src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js * should render a coroutine +* should update a coroutine * should unmount a composite in a coroutine +* should handle deep updates in coroutine src/renderers/shared/fiber/__tests__/ReactIncremental-test.js * should render a simple component diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index f3910590dca5f..cc60aee305d51 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -200,8 +200,7 @@ var ReactElementValidator = { createElement: function(type, props, children) { var validType = typeof type === 'string' || - typeof type === 'function' || - type !== null && typeof type === 'object' && typeof type.tag === 'number'; + typeof type === 'function'; // We warn in this case but don't throw. We expect the element creation to // succeed and there will likely be errors in render. if (!validType) { diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 95cea880cf906..0505ae07e34cd 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -29,7 +29,6 @@ var { } = require('ReactPortal'); var ReactFiber = require('ReactFiber'); -var ReactReifiedYield = require('ReactReifiedYield'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); var ReactTypeOfWork = require('ReactTypeOfWork'); @@ -53,11 +52,6 @@ const { createFiberFromPortal, } = ReactFiber; -const { - createReifiedYield, - createUpdatedReifiedYield, -} = ReactReifiedYield; - const isArray = Array.isArray; const { @@ -316,21 +310,16 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { yieldNode : ReactYield, priority : PriorityLevel ) : Fiber { - // TODO: Should this also compare continuation to determine whether to reuse? if (current == null || current.tag !== YieldComponent) { // Insert - const reifiedYield = createReifiedYield(yieldNode); const created = createFiberFromYield(yieldNode, priority); - created.type = reifiedYield; + created.type = yieldNode.value; created.return = returnFiber; return created; } else { // Move based on index const existing = useFiber(current, priority); - existing.type = createUpdatedReifiedYield( - current.type, - yieldNode - ); + existing.type = yieldNode.value; existing.return = returnFiber; return existing; } @@ -411,9 +400,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { } case REACT_YIELD_TYPE: { - const reifiedYield = createReifiedYield(newChild); const created = createFiberFromYield(newChild, priority); - created.type = reifiedYield; + created.type = newChild.value; created.return = returnFiber; return created; } @@ -474,7 +462,10 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { } case REACT_YIELD_TYPE: { - if (newChild.key === key) { + // Yields doesn't have keys. If the previous node is implicitly keyed + // we can continue to replace it without aborting even if it is not a + // yield. + if (key === null) { return updateYield(returnFiber, oldFiber, newChild, priority); } else { return null; @@ -527,9 +518,9 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { } case REACT_YIELD_TYPE: { - const matchedFiber = existingChildren.get( - newChild.key === null ? newIdx : newChild.key - ) || null; + // Yields doesn't have keys, so we neither have to check the old nor + // new node for the key. If both are yields, they match. + const matchedFiber = existingChildren.get(newIdx) || null; return updateYield(returnFiber, matchedFiber, newChild, priority); } @@ -561,7 +552,6 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { switch (child.$$typeof) { case REACT_ELEMENT_TYPE: case REACT_COROUTINE_TYPE: - case REACT_YIELD_TYPE: case REACT_PORTAL_TYPE: const key = child.key; if (typeof key !== 'string') { @@ -1019,34 +1009,22 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { yieldNode : ReactYield, priority : PriorityLevel ) : Fiber { - const key = yieldNode.key; + // There's no need to check for keys on yields since they're stateless. let child = currentFirstChild; - while (child) { - // TODO: If key === null and child.key === null, then this only applies to - // the first item in the list. - if (child.key === key) { - if (child.tag === YieldComponent) { - deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, priority); - existing.type = createUpdatedReifiedYield( - child.type, - yieldNode - ); - existing.return = returnFiber; - return existing; - } else { - deleteRemainingChildren(returnFiber, child); - break; - } + if (child) { + if (child.tag === YieldComponent) { + deleteRemainingChildren(returnFiber, child.sibling); + const existing = useFiber(child, priority); + existing.type = yieldNode.value; + existing.return = returnFiber; + return existing; } else { - deleteChild(returnFiber, child); + deleteRemainingChildren(returnFiber, child); } - child = child.sibling; } - const reifiedYield = createReifiedYield(yieldNode); const created = createFiberFromYield(yieldNode, priority); - created.type = reifiedYield; + created.type = yieldNode.value; created.return = returnFiber; return created; } diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 3657d087bb8c8..b488c0bbb69c3 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -393,8 +393,7 @@ exports.createFiberFromCoroutine = function(coroutine : ReactCoroutine, priority }; exports.createFiberFromYield = function(yieldNode : ReactYield, priorityLevel : PriorityLevel) : Fiber { - const fiber = createFiber(YieldComponent, yieldNode.key); - fiber.pendingProps = {}; + const fiber = createFiber(YieldComponent, null); return fiber; }; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index dbbb91466956a..a19f6659fc2a6 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -516,13 +516,54 @@ module.exports = function( } } } else if (nextCoroutine === null || workInProgress.memoizedProps === nextCoroutine) { - return bailoutOnAlreadyFinishedWork(current, workInProgress); + nextCoroutine = workInProgress.memoizedProps; + // TODO: When bailing out, we might need to return the stateNode instead + // of the child. To check it for work. + // return bailoutOnAlreadyFinishedWork(current, workInProgress); + } + + const nextChildren = nextCoroutine.children; + const priorityLevel = workInProgress.pendingWorkPriority; + + // The following is a fork of reconcileChildrenAtPriority but using + // stateNode to store the child. + + // At this point any memoization is no longer valid since we'll have changed + // the children. + workInProgress.memoizedProps = null; + if (!current) { + workInProgress.stateNode = mountChildFibersInPlace( + workInProgress, + workInProgress.stateNode, + nextChildren, + priorityLevel + ); + } else if (current.child === workInProgress.child) { + clearDeletions(workInProgress); + + workInProgress.stateNode = reconcileChildFibers( + workInProgress, + workInProgress.stateNode, + nextChildren, + priorityLevel + ); + + transferDeletions(workInProgress); + } else { + workInProgress.stateNode = reconcileChildFibersInPlace( + workInProgress, + workInProgress.stateNode, + nextChildren, + priorityLevel + ); + + transferDeletions(workInProgress); } - reconcileChildren(current, workInProgress, nextCoroutine.children); + memoizeProps(workInProgress, nextCoroutine); // This doesn't take arbitrary time so we could synchronously just begin // eagerly do the work of workInProgress.child as an optimization. - return workInProgress.child; + return workInProgress.stateNode; } function updatePortalComponent(current, workInProgress) { diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 37bef63964c2b..0dd429cdda195 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -136,7 +136,6 @@ module.exports = function( while (node.tag !== HostComponent && node.tag !== HostText) { // If it is not host node and, we might have a host node inside it. // Try to search down until we find one. - // TODO: For coroutines, this will have to search the stateNode. if (node.effectTag & Placement) { // If we don't have a child, try the siblings instead. continue siblings; @@ -198,7 +197,6 @@ module.exports = function( // down its children. Instead, we'll get insertions from each child in // the portal directly. } else if (node.child) { - // TODO: Coroutines need to visit the stateNode. node.child.return = node; node = node.child; continue; @@ -229,7 +227,6 @@ module.exports = function( // Visit children because they may contain more composite or host nodes. // Skip portals because commitUnmount() currently visits them recursively. if (node.child && node.tag !== HostPortal) { - // TODO: Coroutines need to visit the stateNode. node.child.return = node; node = node.child; continue; @@ -273,7 +270,6 @@ module.exports = function( commitUnmount(node); // Visit children because we may find more host components below. if (node.child) { - // TODO: Coroutines need to visit the stateNode. node.child.return = node; node = node.child; continue; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 776db30ecc018..400bdf839465e 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -17,7 +17,6 @@ import type { Fiber } from 'ReactFiber'; import type { HostContext } from 'ReactFiberHostContext'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; -import type { ReifiedYield } from 'ReactReifiedYield'; var { reconcileChildFibers } = require('ReactChildFiber'); var { @@ -66,14 +65,29 @@ module.exports = function( popHostContainer, } = hostContext; + function markChildAsProgressed(current, workInProgress, priorityLevel) { + // We now have clones. Let's store them as the currently progressed work. + workInProgress.progressedChild = workInProgress.child; + workInProgress.progressedPriority = priorityLevel; + if (current) { + // We also store it on the current. When the alternate swaps in we can + // continue from this point. + current.progressedChild = workInProgress.progressedChild; + current.progressedPriority = workInProgress.progressedPriority; + } + } + function markUpdate(workInProgress : Fiber) { // Tag the fiber with an update effect. This turns a Placement into // an UpdateAndPlacement. workInProgress.effectTag |= Update; } - function appendAllYields(yields : Array, workInProgress : Fiber) { - let node = workInProgress.child; + function appendAllYields(yields : Array, workInProgress : Fiber) { + let node = workInProgress.stateNode; + if (node) { + node.return = workInProgress; + } while (node) { if (node.tag === HostComponent || node.tag === HostText || node.tag === HostPortal) { @@ -81,14 +95,10 @@ module.exports = function( } else if (node.tag === YieldComponent) { yields.push(node.type); } else if (node.child) { - // TODO: Coroutines need to visit the stateNode. node.child.return = node; node = node.child; continue; } - if (node === workInProgress) { - return; - } while (!node.sibling) { if (!node.return || node.return === workInProgress) { return; @@ -117,22 +127,23 @@ module.exports = function( // Build up the yields. // TODO: Compare this to a generator or opaque helpers like Children. - var yields : Array = []; + var yields : Array = []; appendAllYields(yields, workInProgress); var fn = coroutine.handler; var props = coroutine.props; var nextChildren = fn(props, yields); - var currentFirstChild = current ? current.stateNode : null; + var currentFirstChild = current ? current.child : null; // Inherit the priority of the returnFiber. const priority = workInProgress.pendingWorkPriority; - workInProgress.stateNode = reconcileChildFibers( + workInProgress.child = reconcileChildFibers( workInProgress, currentFirstChild, nextChildren, priority ); - return workInProgress.stateNode; + markChildAsProgressed(current, workInProgress, priority); + return workInProgress.child; } function appendAllChildren(parent : I, workInProgress : Fiber) { @@ -147,7 +158,6 @@ module.exports = function( // down its children. Instead, we'll get insertions from each child in // the portal directly. } else if (node.child) { - // TODO: Coroutines need to visit the stateNode. node = node.child; continue; } diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 1b2fc170048ad..7cdf1b917d15b 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -435,6 +435,8 @@ module.exports = function(config : HostConfig { ReactFeatureFlags.disableNewFiberFeatures = false; }); + function div(...children) { + children = children.map(c => typeof c === 'string' ? { text: c } : c); + return { type: 'div', children, prop: undefined }; + } + + function span(prop) { + return { type: 'span', children: [], prop }; + } + it('should render a coroutine', () => { var ops = []; function Continuation({ isSame }) { ops.push(['Continuation', isSame]); - return {isSame ? 'foo==bar' : 'foo!=bar'}; + return ; } // An alternative API could mark Continuation as something that needs @@ -39,8 +48,11 @@ describe('ReactCoroutine', () => { function Child({ bar }) { ops.push(['Child', bar]); return ReactCoroutine.createYield({ - bar: bar, - }, Continuation, null); + props: { + bar: bar, + }, + continuation: Continuation, + }); } function Indirection() { @@ -85,6 +97,67 @@ describe('ReactCoroutine', () => { ['Continuation', true], ['Continuation', false], ]); + expect(ReactNoop.getChildren()).toEqual([ + div( + span('foo==bar'), + span('foo!=bar'), + ), + ]); + }); + + it('should update a coroutine', () => { + function Continuation({ isSame }) { + return ; + } + + function Child({ bar }) { + return ReactCoroutine.createYield({ + props: { + bar: bar, + }, + continuation: Continuation, + }); + } + + function Indirection() { + return [, ]; + } + + function HandleYields(props, yields) { + return yields.map(y => + + ); + } + + function Parent(props) { + return ReactCoroutine.createCoroutine( + props.children, + HandleYields, + props + ); + } + + function App(props) { + return
; + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + div( + span('foo==bar'), + span('foo!=bar'), + ), + ]); + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + div( + span('foo!=bar'), + span('foo==bar'), + ), + ]); }); it('should unmount a composite in a coroutine', () => { @@ -103,7 +176,7 @@ describe('ReactCoroutine', () => { class Child extends React.Component { render() { ops.push('Child'); - return ReactCoroutine.createYield({}, Continuation, null); + return ReactCoroutine.createYield(Continuation); } componentWillUnmount() { ops.push('Unmount Child'); @@ -112,7 +185,7 @@ describe('ReactCoroutine', () => { function HandleYields(props, yields) { ops.push('HandleYields'); - return yields.map(y => ); + return yields.map(ContinuationComponent => ); } class Parent extends React.Component { @@ -146,11 +219,50 @@ describe('ReactCoroutine', () => { expect(ops).toEqual([ 'Unmount Parent', - // TODO: This should happen in the order Child, Continuation which it - // will once we swap stateNode and child positions of these. - 'Unmount Continuation', 'Unmount Child', + 'Unmount Continuation', + ]); + + }); + + it('should handle deep updates in coroutine', () => { + let instances = {}; + + class Counter extends React.Component { + state = {value: 5}; + render() { + instances[this.props.id] = this; + return ReactCoroutine.createYield(this.state.value); + } + } + + function App(props) { + return ReactCoroutine.createCoroutine( + [ + , + , + , + ], + (p, yields) => yields.map(y => ), + {} + ); + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + span(500), + span(500), + span(500), ]); + instances.a.setState({value: 1}); + instances.b.setState({value: 2}); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + span(100), + span(200), + span(500), + ]); }); }); diff --git a/src/renderers/shared/fiber/isomorphic/ReactCoroutine.js b/src/renderers/shared/fiber/isomorphic/ReactCoroutine.js index 609470fd39bc9..68f3a99b5d8e5 100644 --- a/src/renderers/shared/fiber/isomorphic/ReactCoroutine.js +++ b/src/renderers/shared/fiber/isomorphic/ReactCoroutine.js @@ -16,30 +16,29 @@ import type { ReactNodeList } from 'ReactTypes'; // The Symbol used to tag the special React types. If there is no native Symbol // nor polyfill, then a plain number is used for performance. -var REACT_COROUTINE_TYPE = - (typeof Symbol === 'function' && Symbol.for && Symbol.for('react.coroutine')) || - 0xeac8; +var REACT_COROUTINE_TYPE; +var REACT_YIELD_TYPE; +if (typeof Symbol === 'function' && Symbol.for) { + REACT_COROUTINE_TYPE = Symbol.for('react.coroutine'); + REACT_YIELD_TYPE = Symbol.for('react.yield'); +} else { + REACT_COROUTINE_TYPE = 0xeac8; + REACT_YIELD_TYPE = 0xeac9; +} -var REACT_YIELD_TYPE = - (typeof Symbol === 'function' && Symbol.for && Symbol.for('react.yield')) || - 0xeac9; - -type ReifiedYield = { continuation: Object, props: Object }; -type CoroutineHandler = (props: T, yields: Array) => ReactNodeList; +type CoroutineHandler = (props: T, yields: Array) => ReactNodeList; export type ReactCoroutine = { $$typeof: Symbol | number, key: null | string, children: any, // This should be a more specific CoroutineHandler - handler: (props: any, yields: Array) => ReactNodeList, + handler: (props: any, yields: Array) => ReactNodeList, props: any, }; export type ReactYield = { $$typeof: Symbol | number, - key: null | string, - props: Object, - continuation: mixed + value: mixed, }; exports.createCoroutine = function( @@ -68,19 +67,16 @@ exports.createCoroutine = function( return coroutine; }; -exports.createYield = function(props : mixed, continuation : mixed, key : ?string = null) { +exports.createYield = function(value : mixed) : ReactYield { var yieldNode = { // This tag allow us to uniquely identify this as a React Yield $$typeof: REACT_YIELD_TYPE, - key: key == null ? null : '' + key, - props: props, - continuation: continuation, + value: value, }; if (__DEV__) { // TODO: Add _store property for marking this as validated. if (Object.freeze) { - Object.freeze(yieldNode.props); Object.freeze(yieldNode); } } diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index 1a393d426b243..7a01f13cb7c38 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -97,7 +97,6 @@ function findAllInRenderedFiberTreeInternal(fiber, test) { } } if (node.child) { - // TODO: Coroutines need to visit the stateNode. node.child.return = node; node = node.child; continue;