[Fiber] Fix initial mount starvation problems

src/renderers/shared/fiber/ReactChildFiber.js

@@ -65,6 +65,7 @@ function ChildReconciler(shouldClone) {
           // Will fix reconciliation properly later.
           const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild;
           if (!shouldClone) {
+            // TODO: This might be lowering the priority of nested unfinished work.
             clone.pendingWorkPriority = priority;
           }
           clone.pendingProps = element.props;
@@ -132,6 +133,7 @@ function ChildReconciler(shouldClone) {
           // Get the clone of the existing fiber.
           const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild;
           if (!shouldClone) {
+            // TODO: This might be lowering the priority of nested unfinished work.
             clone.pendingWorkPriority = priority;
           }
           clone.pendingProps = element.props;

src/renderers/shared/fiber/ReactFiber.js

@@ -170,6 +170,9 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
     alt.pendingProps = fiber.pendingProps;
     alt.pendingWorkPriority = priorityLevel;
 
+    alt.memoizedProps = fiber.memoizedProps;
+    alt.output = fiber.output;
+
     // Whenever we clone, we do so to get a new work in progress.
     // This ensures that we've reset these in the new tree.
     alt.nextEffect = null;
@@ -191,6 +194,9 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
   alt.pendingProps = fiber.pendingProps;
   alt.pendingWorkPriority = priorityLevel;
 
+  alt.memoizedProps = fiber.memoizedProps;
+  alt.output = fiber.output;
+
   alt.alternate = fiber;
   fiber.alternate = alt;
   return alt;

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -40,6 +40,8 @@ var { findNextUnitOfWorkAtPriority } = require('ReactFiberPendingWork');
 module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
 
   function reconcileChildren(current, workInProgress, nextChildren) {
+    // TODO: Children needs to be able to reconcile in place if we are
+    // overriding progressed work.
     const priority = workInProgress.pendingWorkPriority;
     reconcileChildrenAtPriority(current, workInProgress, nextChildren, priority);
   }
@@ -76,7 +78,6 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
     var props = workInProgress.pendingProps;
     var nextChildren = fn(props);
     reconcileChildren(current, workInProgress, nextChildren);
-    workInProgress.pendingWorkPriority = NoWork;
   }
 
   function updateClassComponent(current : ?Fiber, workInProgress : Fiber) {
@@ -106,7 +107,6 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
     instance.props = props;
     var nextChildren = instance.render();
     reconcileChildren(current, workInProgress, nextChildren);
-    workInProgress.pendingWorkPriority = NoWork;
     return workInProgress.childInProgress;
   }
 
@@ -125,11 +125,9 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       // becomes part of the render tree, even though it never completed. Its
       // `output` property is unpredictable because of it.
       reconcileChildrenAtPriority(current, workInProgress, nextChildren, OffscreenPriority);
-      workInProgress.pendingWorkPriority = OffscreenPriority;
       return null;
     } else {
       reconcileChildren(current, workInProgress, nextChildren);
-      workInProgress.pendingWorkPriority = NoWork;
       return workInProgress.childInProgress;
     }
   }
@@ -153,7 +151,6 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       }
     }
     reconcileChildren(current, workInProgress, value);
-    workInProgress.pendingWorkPriority = NoWork;
   }
 
   function updateCoroutineComponent(current, workInProgress) {
@@ -162,10 +159,11 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       throw new Error('Should be resolved by now');
     }
     reconcileChildren(current, workInProgress, coroutine.children);
-    workInProgress.pendingWorkPriority = NoWork;
   }
 
   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
@@ -210,7 +208,6 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
     workInProgress.output = current.output;
     const priorityLevel = workInProgress.pendingWorkPriority;
     workInProgress.pendingProps = null;
-    workInProgress.pendingWorkPriority = NoWork;
     workInProgress.stateNode = current.stateNode;
     workInProgress.childInProgress = current.childInProgress;
     if (current.child) {
@@ -220,10 +217,10 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       workInProgress.child = current.child;
       reuseChildren(workInProgress, workInProgress.child);
       if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) {
-        // TODO: This passes the current node and reads the priority level and
-        // pending props from that. We want it to read our priority level and
-        // pending props from the work in progress. Needs restructuring.
-        return findNextUnitOfWorkAtPriority(current, priorityLevel);
+        return findNextUnitOfWorkAtPriority(
+          workInProgress,
+          workInProgress.pendingWorkPriority
+        );
       } else {
         return null;
       }
@@ -239,22 +236,13 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
     // looking for. In that case, we should be able to just bail out.
     const priorityLevel = workInProgress.pendingWorkPriority;
     workInProgress.pendingProps = null;
-    workInProgress.pendingWorkPriority = NoWork;
 
     workInProgress.firstEffect = null;
     workInProgress.nextEffect = null;
     workInProgress.lastEffect = null;
 
-    if (workInProgress.child) {
-      // On the way up here, we reset the child node to be the current one by
-      // cloning. However, it is really the original child that represents the
-      // already completed work. Therefore we have to reuse the alternate.
-      // But if we don't have a current, this was not cloned. This is super weird.
-      const child = !current ? workInProgress.child : workInProgress.child.alternate;
-      if (!child) {
-        throw new Error('We must have a current child to be able to use this.');
-      }
-      workInProgress.child = child;
+    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
@@ -299,7 +287,6 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
         reconcileChildren(current, workInProgress, workInProgress.pendingProps);
         // A yield component is just a placeholder, we can just run through the
         // next one immediately.
-        workInProgress.pendingWorkPriority = NoWork;
         if (workInProgress.childInProgress) {
           return beginWork(
             workInProgress.childInProgress.alternate,
@@ -327,7 +314,6 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       case YieldComponent:
         // A yield component is just a placeholder, we can just run through the
         // next one immediately.
-        workInProgress.pendingWorkPriority = NoWork;
         if (workInProgress.sibling) {
           return beginWork(
             workInProgress.sibling.alternate,

src/renderers/shared/fiber/ReactFiberPendingWork.js

@@ -34,20 +34,50 @@ function cloneSiblings(current : Fiber, workInProgress : Fiber, returnFiber : Fi
   workInProgress.sibling = null;
 }
 
-exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLevel : PriorityLevel) : ?Fiber {
-  let current = currentRoot;
-  while (current) {
-    if (current.pendingWorkPriority !== NoWork &&
-        current.pendingWorkPriority <= priorityLevel) {
+function cloneChildrenIfNeeded(workInProgress : Fiber) {
+  const current = workInProgress.alternate;
+  if (!current || workInProgress.child !== current.child) {
+    // If there is no alternate, then we don't need to clone the children.
+    // 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;
+      child = child.sibling;
+    }
+    return;
+  }
+  // TODO: This used to reset the pending priority. Not sure if that is needed.
+  // workInProgress.pendingWorkPriority = current.pendingWorkPriority;
+
+  // TODO: The below priority used to be set to NoWork which would've
+  // dropped work. This is currently unobservable but will become
+  // observable when the first sibling has lower priority work remaining
+  // than the next sibling. At that point we should add tests that catches
+  // this.
+
+  const currentChild = current.child;
+  if (!currentChild) {
+    return;
+  }
+  workInProgress.child = cloneFiber(
+    currentChild,
+    currentChild.pendingWorkPriority
+  );
+  cloneSiblings(currentChild, workInProgress.child, workInProgress);
+}
+
+exports.findNextUnitOfWorkAtPriority = function(workRoot : Fiber, priorityLevel : PriorityLevel) : ?Fiber {
+  let workInProgress = workRoot;
+  while (workInProgress) {
+    if (workInProgress.pendingWorkPriority !== NoWork &&
+        workInProgress.pendingWorkPriority <= priorityLevel) {
       // This node has work to do that fits our priority level criteria.
-      if (current.pendingProps !== null) {
-        // We found some work to do. We need to return the "work in progress"
-        // of this node which will be the alternate.
-        const workInProgress = current.alternate;
-        if (!workInProgress) {
-          throw new Error('Should have wip now');
-        }
-        workInProgress.pendingProps = current.pendingProps;
+      if (workInProgress.pendingProps !== null) {
         return workInProgress;
       }
 
@@ -56,71 +86,57 @@ exports.findNextUnitOfWorkAtPriority = function(currentRoot : Fiber, priorityLev
       // because it is the highest priority for the whole subtree.
       // TODO: Coroutines can have work in their stateNode which is another
       // type of child that needs to be searched for work.
-      if (current.childInProgress) {
-        let workInProgress = current.childInProgress;
-        while (workInProgress) {
-          workInProgress.return = current.alternate;
-          workInProgress = workInProgress.sibling;
+      if (workInProgress.childInProgress) {
+        let child = workInProgress.childInProgress;
+        while (child) {
+          child.return = workInProgress;
+          child = child.sibling;
         }
-        workInProgress = current.childInProgress;
-        while (workInProgress) {
-          // Don't bother drilling further down this tree if there is no child.
-          if (workInProgress.pendingWorkPriority !== NoWork &&
-              workInProgress.pendingWorkPriority <= priorityLevel &&
-              workInProgress.pendingProps !== null) {
-            return workInProgress;
+        child = workInProgress.childInProgress;
+        while (child) {
+          // Don't bother drilling further down this tree if there is no child
+          // with more content.
+          // TODO: Shouldn't this still drill down even though the first
+          // shallow level doesn't have anything pending on it.
+          if (child.pendingWorkPriority !== NoWork &&
+              child.pendingWorkPriority <= priorityLevel &&
+              child.pendingProps !== null) {
+            return child;
           }
-          workInProgress = workInProgress.sibling;
+          child = child.sibling;
         }
-      } else if (current.child) {
-        let currentChild = current.child;
-        currentChild.return = current;
-        // Ensure we have a work in progress copy to backtrack through.
-        let workInProgress = current.alternate;
-        if (!workInProgress) {
-          throw new Error('Should have wip now');
-        }
-        workInProgress.pendingWorkPriority = current.pendingWorkPriority;
-        // TODO: The below priority used to be set to NoWork which would've
-        // dropped work. This is currently unobservable but will become
-        // observable when the first sibling has lower priority work remaining
-        // than the next sibling. At that point we should add tests that catches
-        // this.
-        workInProgress.child = cloneFiber(
-          currentChild,
-          currentChild.pendingWorkPriority
-        );
-        cloneSiblings(currentChild, workInProgress.child, workInProgress);
-        current = currentChild;
+      } else if (workInProgress.child) {
+        cloneChildrenIfNeeded(workInProgress);
+        workInProgress = workInProgress.child;
         continue;
       }
       // If we match the priority but has no child and no work to do,
       // then we can safely reset the flag.
-      current.pendingWorkPriority = NoWork;
+      workInProgress.pendingWorkPriority = NoWork;
     }
-    if (current === currentRoot) {
-      if (current.pendingWorkPriority <= priorityLevel) {
+    if (workInProgress === workRoot) {
+      if (workInProgress.pendingWorkPriority <= priorityLevel) {
         // If this subtree had work left to do, we would have returned it by
         // now. This could happen if a child with pending work gets cleaned up
         // but we don't clear the flag then. It is safe to reset it now.
-        current.pendingWorkPriority = NoWork;
+        workInProgress.pendingWorkPriority = NoWork;
       }
       return null;
     }
-    while (!current.sibling) {
-      current = current.return;
-      if (!current) {
+    while (!workInProgress.sibling) {
+      workInProgress = workInProgress.return;
+      if (!workInProgress || workInProgress === workRoot) {
         return null;
       }
-      if (current.pendingWorkPriority <= priorityLevel) {
+      if (workInProgress.pendingWorkPriority <= priorityLevel) {
         // If this subtree had work left to do, we would have returned it by
         // now. This could happen if a child with pending work gets cleaned up
         // but we don't clear the flag then. It is safe to reset it now.
-        current.pendingWorkPriority = NoWork;
+        workInProgress.pendingWorkPriority = NoWork;
       }
     }
-    current.sibling.return = current.return;
-    current = current.sibling;
+    workInProgress.sibling.return = workInProgress.return;
+    workInProgress = workInProgress.sibling;
   }
   return null;
 };

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -70,6 +70,8 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) : Reconci
       const root = createFiberRoot(containerInfo);
       const container = root.current;
       // TODO: Use pending work/state instead of props.
+      // TODO: This should not override the pendingWorkPriority if there is
+      // higher priority work in the subtree.
       container.pendingProps = element;
       container.pendingWorkPriority = LowPriority;