[Fiber] Some setState related issues

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -160,15 +160,24 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, 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<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, 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

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 <span prop={'' + (this.props.x === this.state.y)} />;
+      }
+    }
+
+    function Foo(props) {
+      ops.push('Foo');
+      return [
+        <Bar key="a" x="A" />,
+        <Bar key="b" x="B" />,
+        <Bar key="c" x="C" />,
+      ];
+    }
+
+    ReactNoop.render(<Foo />);
+    ReactNoop.flushDeferredPri(30);
+    expect(ops).toEqual(['Foo', 'Bar:A', 'Bar:B']);
+
+    ops = [];
+
+    ReactNoop.render(<Foo />);
+    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 <span prop={'' + (this.props.x === this.state.y)} />;
+      }
+    }
+
+    function Baz() {
+      // This component is used as a sibling to Foo so that we can fully
+      // complete Foo, without committing.
+      ops.push('Baz');
+      return <div />;
+    }
+
+    function Foo(props) {
+      ops.push('Foo');
+      return [
+        <Bar key="a" x="A" step={props.step} />,
+        <Bar key="b" x="B" step={props.step} />,
+      ];
+    }
+
+    ReactNoop.render(<div><Foo step={0} /><Baz /><Baz /></div>);
+    ReactNoop.flush();
+
+    ops = [];
+
+    // Flush part way through with new props, fully completing the first Bar.
+    // However, it doesn't commit yet.
+    ReactNoop.render(<div><Foo step={1} /><Baz /><Baz /></div>);
+    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']);
+  });
+
 });

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 <span prop={this.state.active ? 'X' : this.props.idx} />;
+      }
+    }
+    function Foo(props) {
+      ops.push('Foo');
+      return (
+        <div>
+          <span prop={props.tick} />
+          <div hidden={true}>
+            <Bar idx={props.idx} />
+            <Bar idx={props.idx} />
+            <Bar idx={props.idx} />
+          </div>
+        </div>
+      );
+    }
+    ReactNoop.render(<Foo tick={0} idx={0} />);
+    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(<Foo tick={1} idx={1} />);
+    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?