diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index a1fc0b99867..b75fe2e67be 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -160,15 +160,24 @@ module.exports = function(config : HostConfig, s // Account for the possibly of missing pending props by falling back to the // memoized props. var props = workInProgress.pendingProps; - if (!props && current) { - props = current.memoizedProps; + if (!props) { + // If there isn't any new props, then we'll reuse the memoized props. + // This could be from already completed work. + props = workInProgress.memoizedProps; + if (!props) { + throw new Error('There should always be pending or memoized props.'); + } } + // Compute the state using the memoized state and the update queue. var updateQueue = workInProgress.updateQueue; - var previousState = current ? current.memoizedState : null; - var state = updateQueue ? - mergeUpdateQueue(updateQueue, previousState, props) : - previousState; + var previousState = workInProgress.memoizedState; + var state; + if (updateQueue) { + state = mergeUpdateQueue(updateQueue, previousState, props); + } else { + state = previousState; + } var instance = workInProgress.stateNode; if (!instance) { @@ -300,6 +309,26 @@ module.exports = function(config : HostConfig, s function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber { const priorityLevel = workInProgress.pendingWorkPriority; + if (workInProgress.tag === HostComponent && + workInProgress.memoizedProps.hidden && + workInProgress.pendingWorkPriority !== OffscreenPriority) { + // This subtree still has work, but it should be deprioritized so we need + // to bail out and not do any work yet. + // TODO: It would be better if this tree got its correct priority set + // during scheduleUpdate instead because otherwise we'll start a higher + // priority reconciliation first before we can get down here. However, + // that is a bit tricky since workInProgress and current can have + // different "hidden" settings. + let child = workInProgress.progressedChild; + while (child) { + // To ensure that this subtree gets its priority reset, the children + // need to be reset. + child.pendingWorkPriority = OffscreenPriority; + child = child.sibling; + } + return null; + } + // TODO: We should ideally be able to bail out early if the children have no // more work to do. However, since we don't have a separation of this // Fiber's priority and its children yet - we don't know without doing lots diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 6d4db53996b..aa6cc245541 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -835,4 +835,96 @@ describe('ReactIncremental', () => { ReactNoop.flush(); expect(ops).toEqual(['Foo', 'Bar', 'Baz', 'Bar', 'Baz']); }); + + it('can call sCU while resuming a partly mounted component', () => { + var ops = []; + + class Bar extends React.Component { + state = { y: 'A' }; + shouldComponentUpdate(newProps, newState) { + return this.props.x !== newProps.x || + this.state.y !== newState.y; + } + render() { + ops.push('Bar:' + this.props.x); + return ; + } + } + + function Foo(props) { + ops.push('Foo'); + return [ + , + , + , + ]; + } + + ReactNoop.render(); + ReactNoop.flushDeferredPri(30); + expect(ops).toEqual(['Foo', 'Bar:A', 'Bar:B']); + + ops = []; + + ReactNoop.render(); + ReactNoop.flushDeferredPri(40); + expect(ops).toEqual(['Foo', 'Bar:B', 'Bar:C']); + }); + + it('gets new props when setting state on a partly updated component', () => { + var ops = []; + var instances = []; + + class Bar extends React.Component { + state = { y: 'A' }; + constructor() { + super(); + instances.push(this); + } + performAction() { + this.setState({ + y: 'B', + }); + } + render() { + ops.push('Bar:' + this.props.x + '-' + this.props.step); + return ; + } + } + + function Baz() { + // This component is used as a sibling to Foo so that we can fully + // complete Foo, without committing. + ops.push('Baz'); + return
; + } + + function Foo(props) { + ops.push('Foo'); + return [ + , + , + ]; + } + + ReactNoop.render(
); + ReactNoop.flush(); + + ops = []; + + // Flush part way through with new props, fully completing the first Bar. + // However, it doesn't commit yet. + ReactNoop.render(
); + ReactNoop.flushDeferredPri(45); + expect(ops).toEqual(['Foo', 'Bar:A-1', 'Bar:B-1', 'Baz']); + + // Make an update to the same Bar. + instances[0].performAction(); + + ops = []; + + ReactNoop.flush(); + expect(ops).toEqual(['Bar:A-1', 'Baz', 'Baz']); + }); + }); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 9982a078e80..81a1e1c0932 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -601,6 +601,113 @@ describe('ReactIncrementalSideEffects', () => { expect(ops).toEqual(['Baz', 'Bar', 'Baz', 'Bar', 'Bar']); }); + it('deprioritizes setStates that happens within a deprioritized tree', () => { + var ops = []; + + var barInstances = []; + + class Bar extends React.Component { + constructor() { + super(); + this.state = { active: false }; + barInstances.push(this); + } + activate() { + this.setState({ active: true }); + } + render() { + ops.push('Bar'); + return ; + } + } + function Foo(props) { + ops.push('Foo'); + return ( +
+ + +
+ ); + } + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.root.children).toEqual([ + div( + span(0), + div( + span(0), + span(0), + span(0) + ) + ), + ]); + + expect(ops).toEqual(['Foo', 'Bar', 'Bar', 'Bar']); + + ops = []; + + ReactNoop.render(); + ReactNoop.flushDeferredPri(70); + expect(ReactNoop.root.children).toEqual([ + div( + // Updated. + span(1), + div( + // Still not updated. + span(0), + span(0), + span(0) + ) + ), + ]); + + expect(ops).toEqual(['Foo', 'Bar', 'Bar']); + ops = []; + + barInstances[0].activate(); + + // This should not be enough time to render the content of all the hidden + // items. Including the set state since that is deprioritized. + // TODO: The cycles it takes to do this could be lowered with further + // optimizations. + ReactNoop.flushDeferredPri(60); + expect(ReactNoop.root.children).toEqual([ + div( + // Updated. + span(1), + div( + // Still not updated. + span(0), + span(0), + span(0) + ) + ), + ]); + + expect(ops).toEqual(['Bar']); + ops = []; + + // However, once we render fully, we will have enough time to finish it all + // at once. + ReactNoop.flush(); + expect(ReactNoop.root.children).toEqual([ + div( + span(1), + div( + // Now we had enough time to finish the spans. + span('X'), + span(1), + span(1), + ) + ), + ]); + + expect(ops).toEqual(['Bar', 'Bar']); + }); // 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?