diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 50b4e0cd2c..c03d709738 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -402,17 +402,8 @@ module.exports = function(config : HostConfig, g } function bailoutOnLowPriority(current, workInProgress) { - if (current) { - // TODO: If I have started work on this node, mark it as finished, then - // return do other work, come back and hit this node... we killed that - // work. It is now in an inconsistent state. We probably need to check - // progressedChild or something. - workInProgress.child = current.child; - workInProgress.memoizedProps = current.memoizedProps; - workInProgress.output = current.output; - workInProgress.firstEffect = null; - workInProgress.lastEffect = null; - } + // TODO: What if this is currently in progress? + // How can that happen? How is this not being cloned? return null; } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 3dfaa7defc..4250f97bb0 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -151,7 +151,7 @@ module.exports = function(config : HostConfig) { // 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; + newProps = workInProgress.memoizedProps || oldProps; } const instance : I = workInProgress.stateNode; if (prepareUpdate(instance, oldProps, newProps)) { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 3347b7212f..9982a078e8 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -380,7 +380,7 @@ describe('ReactIncrementalSideEffects', () => { it('can defer side-effects and resume them later on', function() { class Bar extends React.Component { shouldComponentUpdate(nextProps) { - return this.props.idx !== nextProps; + return this.props.idx !== nextProps.idx; } render() { return ; @@ -437,10 +437,11 @@ describe('ReactIncrementalSideEffects', () => { ) ), ]); - ReactNoop.flushDeferredPri(30); + ReactNoop.render(); + ReactNoop.flush(); expect(ReactNoop.root.children).toEqual([ div( - span(2), + span(3), div( // New numbers. span(1), @@ -456,6 +457,149 @@ describe('ReactIncrementalSideEffects', () => { expect(innerSpanA).toBe(innerSpanB); }); + it('can defer side-effects and reuse them later - complex', function() { + var ops = []; + + class Bar extends React.Component { + shouldComponentUpdate(nextProps) { + return this.props.idx !== nextProps.idx; + } + render() { + ops.push('Bar'); + return ; + } + } + class Baz extends React.Component { + shouldComponentUpdate(nextProps) { + return this.props.idx !== nextProps.idx; + } + render() { + ops.push('Baz'); + return [, ]; + } + } + function Foo(props) { + ops.push('Foo'); + return ( +
+ + +
+ ); + } + ReactNoop.render(); + ReactNoop.flushDeferredPri(65); + expect(ReactNoop.root.children).toEqual([ + div( + span(0), + div(/*the spans are down-prioritized and not rendered yet*/) + ), + ]); + + expect(ops).toEqual(['Foo', 'Baz', 'Bar']); + ops = []; + + ReactNoop.render(); + ReactNoop.flushDeferredPri(70); + expect(ReactNoop.root.children).toEqual([ + div( + span(1), + div(/*still not rendered yet*/) + ), + ]); + + expect(ops).toEqual(['Foo', 'Baz', 'Bar']); + ops = []; + + ReactNoop.flush(); + expect(ReactNoop.root.children).toEqual([ + div( + span(1), + div( + // Now we had enough time to finish the spans. + span(0), + span(0), + span(0), + span(0), + span(0), + span(0) + ) + ), + ]); + + expect(ops).toEqual(['Bar', 'Baz', 'Bar', 'Bar', 'Baz', 'Bar', 'Bar']); + ops = []; + + // Now we're going to update the index but we'll only let it finish half + // way through. + ReactNoop.render(); + ReactNoop.flushDeferredPri(95); + expect(ReactNoop.root.children).toEqual([ + div( + span(2), + div( + // Still same old numbers. + span(0), + span(0), + span(0), + span(0), + span(0), + span(0) + ) + ), + ]); + + // We let it finish half way through. That means we'll have one fully + // completed Baz, one half-way completed Baz and one fully incomplete Baz. + expect(ops).toEqual(['Foo', 'Baz', 'Bar', 'Bar', 'Baz', 'Bar']); + ops = []; + + // We'll update again, without letting the new index update yet. Only half + // way through. + ReactNoop.render(); + ReactNoop.flushDeferredPri(50); + expect(ReactNoop.root.children).toEqual([ + div( + span(3), + div( + // Old numbers. + span(0), + span(0), + span(0), + span(0), + span(0), + span(0) + ) + ), + ]); + + expect(ops).toEqual(['Foo']); + ops = []; + + // We should now be able to reuse some of the work we've already done + // and replay those side-effects. + ReactNoop.flush(); + expect(ReactNoop.root.children).toEqual([ + div( + span(3), + div( + // New numbers. + span(1), + span(1), + span(1), + span(1), + span(1), + span(1) + ) + ), + ]); + + expect(ops).toEqual(['Baz', 'Bar', 'Baz', '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