Progressed work

src/renderers/noop/ReactNoop.js

@@ -170,16 +170,12 @@ var ReactNoop = {
         '  '.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');
-          }
+      const childInProgress = fiber.progressedChild;
+      if (childInProgress && childInProgress !== fiber.child) {
+        console.log('  '.repeat(depth + 1) + 'IN PROGRESS: ' + fiber.progressedPriority);
+        logFiber(childInProgress, depth + 1);
+        if (fiber.child) {
+          console.log('  '.repeat(depth + 1) + 'CURRENT');
         }
       }
       if (fiber.child) {

src/renderers/shared/fiber/ReactChildFiber.js

@@ -67,7 +67,7 @@ function ChildReconciler(shouldClone) {
             clone.pendingWorkPriority = priority;
           }
           clone.pendingProps = element.props;
-          clone.child = existingChild.child;
+          // clone.child = existingChild.child;
           clone.sibling = null;
           clone.return = returnFiber;
           previousSibling.sibling = clone;
@@ -139,7 +139,7 @@ function ChildReconciler(shouldClone) {
             clone.pendingWorkPriority = priority;
           }
           clone.pendingProps = element.props;
-          clone.child = existingChild.child;
+          // clone.child = existingChild.child;
           clone.sibling = null;
           clone.return = returnFiber;
           return clone;
@@ -246,9 +246,6 @@ exports.cloneChildFibers = function(workInProgress : Fiber) {
     // If the children of the alternate fiber is a different set, then we don't
     // need to clone. We need to reset the return fiber though since we'll
     // traverse down into them.
-    // TODO: I don't think it is actually possible for them to be anything but
-    // equal at this point because this fiber was just cloned. Can we skip this
-    // check? Similar question about the return fiber.
     let child = workInProgress.child;
     while (child) {
       child.return = workInProgress;
@@ -265,7 +262,7 @@ exports.cloneChildFibers = function(workInProgress : Fiber) {
   // than the next sibling. At that point we should add tests that catches
   // this.
 
-  const currentChild = current.child;
+  const currentChild = workInProgress.child;
   if (!currentChild) {
     return;
   }
@@ -274,4 +271,4 @@ exports.cloneChildFibers = function(workInProgress : Fiber) {
     currentChild.pendingWorkPriority
   );
   cloneSiblings(currentChild, workInProgress.child, workInProgress);
-}
+};

src/renderers/shared/fiber/ReactFiber.js

@@ -92,6 +92,17 @@ export type Fiber = Instance & {
   // This will be used to quickly determine if a subtree has no pending changes.
   pendingWorkPriority: PriorityLevel,
 
+  // This value represents the priority level that was last used to process this
+  // component. This indicates whether it is better to continue from the
+  // progressed work or if it is better to continue from the current state.
+  progressedPriority: PriorityLevel,
+
+  // If work bails out on a Fiber that already had some work started at a lower
+  // priority, then we need to store the progressed work somewhere. This holds
+  // the started child set until we need to get back to working on it. It may
+  // or may not be the same as the "current" child.
+  progressedChild: ?Fiber,
+
   // This is a pooled version of a Fiber. Every fiber that gets updated will
   // eventually have a pair. There are cases when we can clean up pairs to save
   // memory if we need to.
@@ -147,6 +158,8 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
     lastEffect: null,
 
     pendingWorkPriority: NoWork,
+    progressedPriority: NoWork,
+    progressedChild: null,
 
     alternate: null,
 
@@ -158,7 +171,16 @@ function shouldConstruct(Component) {
 }
 
 // This is used to create an alternate fiber to do work on.
+// TODO: Rename to createWorkInProgressFiber or something like that.
 exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fiber {
+  // We clone to get a work in progress. That means that this fiber is the
+  // current. To make it safe to reuse that fiber later on as work in progress
+  // we need to reset its work in progress flag now. We don't have an
+  // opportunity to do this earlier since we don't traverse the tree when
+  // the work in progress tree becomes the current tree.
+  // fiber.progressedPriority = NoWork;
+  // fiber.progressedChild = null;
+
   // We use a double buffering pooling technique because we know that we'll only
   // ever need at most two versions of a tree. We pool the "other" unused node
   // that we're free to reuse. This is lazily created to avoid allocating extra
@@ -167,12 +189,12 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
   let alt = fiber.alternate;
   if (alt) {
     alt.stateNode = fiber.stateNode;
-    alt.child = fiber.child;
-    alt.sibling = fiber.sibling;
+    alt.sibling = fiber.sibling; // This should always be overridden. TODO: null
     alt.ref = fiber.ref;
-    alt.pendingProps = fiber.pendingProps;
+    alt.pendingProps = fiber.pendingProps; // TODO: Pass as argument.
     alt.pendingWorkPriority = priorityLevel;
 
+    alt.child = fiber.child;
     alt.memoizedProps = fiber.memoizedProps;
     alt.output = fiber.output;
 
@@ -190,15 +212,19 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
   alt.type = fiber.type;
   alt.stateNode = fiber.stateNode;
   alt.child = fiber.child;
-  alt.sibling = fiber.sibling;
+  alt.sibling = fiber.sibling; // This should always be overridden. TODO: null
   alt.ref = fiber.ref;
   // pendingProps is here for symmetry but is unnecessary in practice for now.
+  // TODO: Pass in the new pendingProps as an argument maybe?
   alt.pendingProps = fiber.pendingProps;
   alt.pendingWorkPriority = priorityLevel;
 
   alt.memoizedProps = fiber.memoizedProps;
   alt.output = fiber.output;
 
+  alt.progressedChild = fiber.progressedChild;
+  alt.progressedPriority = fiber.progressedPriority;
+
   alt.alternate = fiber;
   fiber.alternate = alt;
   return alt;

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -15,6 +15,7 @@
 import type { ReactCoroutine } from 'ReactCoroutine';
 import type { Fiber } from 'ReactFiber';
 import type { HostConfig } from 'ReactFiberReconciler';
+import type { PriorityLevel } from 'ReactPriorityLevel';
 
 var {
   reconcileChildFibers,
@@ -39,8 +40,24 @@ var {
 
 module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
 
+  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 reconcileChildren(current, workInProgress, nextChildren) {
     const priorityLevel = workInProgress.pendingWorkPriority;
+    reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel);
+  }
+
+  function reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel) {
     // At this point any memoization is no longer valid since we'll have changed
     // the children.
     workInProgress.memoizedProps = null;
@@ -66,6 +83,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
         priorityLevel
       );
     }
+    markChildAsProgressed(current, workInProgress, priorityLevel);
   }
 
   function updateFunctionalComponent(current, workInProgress) {
@@ -94,20 +112,13 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       var ctor = workInProgress.type;
       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.memoizedProps) {
+      if (workInProgress.memoizedProps !== null) {
         // 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(current, workInProgress);
+          return bailoutOnAlreadyFinishedWork(current, workInProgress);
         }
       }
     }
@@ -120,14 +131,29 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
   }
 
   function updateHostComponent(current, workInProgress) {
+    const nextChildren = workInProgress.pendingProps.children;
     if (workInProgress.pendingProps.hidden &&
         workInProgress.pendingWorkPriority !== OffscreenPriority) {
-      // If this host component is hidden, we can bail out and ignore this.
-      // We'll rerender it later at the lower priority.
-      workInProgress.pendingWorkPriority = OffscreenPriority;
-      return bailoutOnLowPriority(current, workInProgress);
+      // If this host component is hidden, we can bail out on the children.
+      // We'll rerender the children later at the lower priority.
+
+      // It is unfortunate that we have to do the reconciliation of these
+      // children already since that will add them to the tree even though
+      // they are not actually done yet. If this is a large set it is also
+      // confusing that this takes time to do right now instead of later.
+
+      if (workInProgress.progressedPriority === OffscreenPriority) {
+        // If we already made some progress on the offscreen priority before,
+        // then we should continue from where we left off.
+        workInProgress.child = workInProgress.progressedChild;
+      }
+
+      // Reconcile the children and stash them for later work.
+      reconcileChildrenAtPriority(current, workInProgress, nextChildren, OffscreenPriority);
+      workInProgress.child = current ? current.child : null;
+      // Abort and don't process children yet.
+      return null;
     } else {
-      var nextChildren = workInProgress.pendingProps.children;
       reconcileChildren(current, workInProgress, nextChildren);
       return workInProgress.child;
     }
@@ -163,31 +189,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
     reconcileChildren(current, workInProgress, coroutine.children);
   }
 
-  function reuseChildren(returnFiber : Fiber, firstChild : Fiber) {
-    // TODO on the TODO: Is this not necessary anymore because I moved the
-    // priority reset?
-    // TODO: None of this should be necessary if structured better.
-    // The returnFiber pointer only needs to be updated when we walk into this child
-    // which we don't do right now. If the pending work priority indicated only
-    // if a child has work rather than if the node has work, then we would know
-    // by a single lookup on workInProgress rather than having to go through
-    // each child.
-    let child = firstChild;
-    do {
-      // Update the returnFiber of the child to the newest fiber.
-      child.return = returnFiber;
-      // Retain the priority if there's any work left to do in the children.
-      /*if (child.pendingWorkPriority !== NoWork &&
-          (returnFiber.pendingWorkPriority === NoWork ||
-          returnFiber.pendingWorkPriority > child.pendingWorkPriority)) {
-        returnFiber.pendingWorkPriority = child.pendingWorkPriority;
-      }*/
-      if (!child.pendingProps && !child.memoizedProps) {
-        throw new Error('Should have memoized props by now');
-      }
-    } while (child = child.sibling);
-  }
-
+  /*
   function reuseChildrenEffects(returnFiber : Fiber, firstChild : Fiber) {
     let child = firstChild;
     do {
@@ -204,56 +206,28 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       }
     } 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.stateNode = current.stateNode;
-
-    workInProgress.nextEffect = null;
-    workInProgress.firstEffect = null;
-    workInProgress.lastEffect = null;
-
-    workInProgress.child = current.child;
-
-    cloneChildFibers(workInProgress);
-
-      // TODO: Maybe bailout with null if the children priority flag indicate
-      // that there is no nested work.
-    return workInProgress.child;
-  }
-*/
+  */
 
   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.
     const priorityLevel = workInProgress.pendingWorkPriority;
-    // workInProgress.pendingProps = null;
 
-    workInProgress.firstEffect = null;
-    workInProgress.nextEffect = null;
-    workInProgress.lastEffect = 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
+    // of the same work we do anyway. Once we have that separation we can just
+    // bail out here if the children has no more work at this priority level.
+    // if (workInProgress.priorityOfChildren <= priorityLevel) {
+    //   // If there are side-effects in these children that have not yet been
+    //   // committed we need to ensure that they get properly transferred up.
+    //   if (current && current.child !== workInProgress.child) {
+    //     reuseChildrenEffects(workInProgress, child);
+    //   }
+    //   return null;
+    // }
 
-    const child = workInProgress.child;
-    if (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);
-      // TODO: Maybe bailout with null if the children priority flag indicate
-      // that there is no nested work.
-      return workInProgress.child;
-    }
-    return null;
+    cloneChildFibers(workInProgress);
+    markChildAsProgressed(current, workInProgress, priorityLevel);
+    return workInProgress.child;
   }
 
   function bailoutOnLowPriority(current, workInProgress) {
@@ -265,26 +239,22 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
     return null;
   }
 
-  function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel) : ?Fiber {
-    if (!workInProgress.pendingProps) {
-      throw new Error('should have pending props here');
-    }
-
+  function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : ?Fiber {
     if (workInProgress.pendingWorkPriority === NoWork ||
         workInProgress.pendingWorkPriority > priorityLevel) {
       return bailoutOnLowPriority(current, workInProgress);
     }
 
-    // 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) {
-      return bailoutOnCurrent(current, workInProgress);
+    if (workInProgress.progressedPriority === priorityLevel) {
+      // If we have progressed work on this priority level already, we can
+      // proceed this that as the child.
+      workInProgress.child = workInProgress.progressedChild;
     }
 
-    if (workInProgress.memoizedProps &&
-        workInProgress.pendingProps === workInProgress.memoizedProps) {
+    if (workInProgress.pendingProps === null || (
+      workInProgress.memoizedProps !== null &&
+      workInProgress.pendingProps === workInProgress.memoizedProps
+      )) {
       return bailoutOnAlreadyFinishedWork(current, workInProgress);
     }
 
@@ -302,7 +272,8 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
         if (workInProgress.child) {
           return beginWork(
             workInProgress.child.alternate,
-            workInProgress.child
+            workInProgress.child,
+            priorityLevel
           );
         }
         return null;
@@ -322,7 +293,8 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
         if (workInProgress.child) {
           return beginWork(
             workInProgress.child.alternate,
-            workInProgress.child
+            workInProgress.child,
+            priorityLevel
           );
         }
         return workInProgress.child;
@@ -332,7 +304,8 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
         if (workInProgress.sibling) {
           return beginWork(
             workInProgress.sibling.alternate,
-            workInProgress.sibling
+            workInProgress.sibling,
+            priorityLevel
           );
         }
         return null;

src/renderers/shared/fiber/ReactFiberCompleteWork.js

@@ -162,10 +162,16 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
             // This returns true if there was something to update.
             markForPreEffect(workInProgress);
           }
+          // TODO: Is this actually ever going to change? Why set it every time?
           workInProgress.output = instance;
         } else {
           if (!newProps) {
-            throw new Error('We must have new props for new mounts.');
+            if (workInProgress.stateNode === null) {
+              throw new Error('We must have new props for new mounts.');
+            } else {
+              // This can happen when we abort work.
+              return null;
+            }
           }
           const instance = createInstance(workInProgress.type, newProps, children);
           // TODO: This seems like unnecessary duplication.

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -25,9 +25,6 @@ var { cloneFiber } = require('ReactFiber');
 
 var {
   NoWork,
-  HighPriority,
-  LowPriority,
-  OffscreenPriority,
 } = require('ReactPriorityLevel');
 
 var timeHeuristicForUnitOfWork = 1;
@@ -105,7 +102,10 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
 
   function resetWorkPriority(workInProgress : Fiber) {
     let newPriority = NoWork;
-    let child = workInProgress.child;
+    // progressedChild is going to be the child set with the highest priority.
+    // Either it is the same as child, or it just bailed out because it choose
+    // not to do the work.
+    let child = workInProgress.progressedChild;
     while (child) {
       // Ensure that remaining work priority bubbles up.
       if (child.pendingWorkPriority !== NoWork &&
@@ -125,25 +125,13 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       // means that we don't need an additional field on the work in
       // progress.
       const current = workInProgress.alternate;
-      let next = null;
+      const next = completeWork(current, workInProgress);
 
-      // If this bailed at a lower priority.
-      // TODO: This branch is currently needed if a particular type of component
-      // ends up being a priority lowering. We should probably know that already
-      // before entering begin work.
-      if (workInProgress.pendingWorkPriority === NoWork ||
-          workInProgress.pendingWorkPriority > nextPriorityLevel) {
-        // This fiber was ignored. We need to fall through to the next fiber
-        // and leave the pending props and work untouched on this fiber.
-      } else {
-        next = completeWork(current, workInProgress);
+      resetWorkPriority(workInProgress);
 
-        resetWorkPriority(workInProgress);
-
-        // 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;
-      }
+      // 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;
 
       const returnFiber = workInProgress.return;
 
@@ -175,6 +163,12 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       } else {
         // If we're at the root, there's no more work to do. We can flush it.
         const root : FiberRoot = (workInProgress.stateNode : any);
+        if (root.current === workInProgress) {
+          throw new Error(
+            'Cannot commit the same tree as before. This is probably a bug ' +
+            'related to the return field.'
+          );
+        }
         root.current = workInProgress;
         // TODO: We can be smarter here and only look for more work in the
         // "next" scheduled work since we've already scanned passed. That

src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js

@@ -270,6 +270,85 @@ 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;
+      }
+      render() {
+        return <span prop={this.props.idx} />;
+      }
+    }
+    function Foo(props) {
+      return (
+        <div>
+          <span prop={props.tick} />
+          <div hidden={true}>
+            <Bar idx={props.idx} />
+            <Bar idx={props.idx + 1} />
+          </div>
+        </div>
+      );
+    }
+    ReactNoop.render(<Foo tick={0} idx={0} />);
+    ReactNoop.flushLowPri(40 + 25);
+    expect(ReactNoop.root.children).toEqual([
+      div(
+        span(0),
+        div(/*the spans are down-prioritized and not rendered yet*/)
+      ),
+    ]);
+    ReactNoop.render(<Foo tick={1} idx={0} />);
+    ReactNoop.flushLowPri(35 + 25);
+    expect(ReactNoop.root.children).toEqual([
+      div(
+        span(1),
+        div(/*still not rendered yet*/)
+      ),
+    ]);
+    ReactNoop.flushLowPri(30 + 25);
+    expect(ReactNoop.root.children).toEqual([
+      div(
+        span(1),
+        div(
+          // Now we had enough time to finish the spans.
+          span(0),
+          span(1)
+        )
+      ),
+    ]);
+    var innerSpanA = ReactNoop.root.children[0].children[1].children[1];
+    ReactNoop.render(<Foo tick={2} idx={1} />);
+    ReactNoop.flushLowPri(30 + 25);
+    expect(ReactNoop.root.children).toEqual([
+      div(
+        span(2),
+        div(
+          // Still same old numbers.
+          span(0),
+          span(1)
+        )
+      ),
+    ]);
+    ReactNoop.flushLowPri(30);
+    expect(ReactNoop.root.children).toEqual([
+      div(
+        span(2),
+        div(
+          // New numbers.
+          span(1),
+          span(2)
+        )
+      ),
+    ]);
+
+    var innerSpanB = ReactNoop.root.children[0].children[1].children[1];
+    // This should have been an update to an existing instance, not recreation.
+    // We verify that by ensuring that the child instance was the same as
+    // before.
+    expect(innerSpanA).toBe(innerSpanB);
+  });
+
 
   // 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