Fixes bug when initial mount of a host component is hidden

`oldProps` was null. This went uncaught by the unit tests because
ReactNoop did not use `oldProps` in either `prepareUpdate` or
`completeUpdate`. I added some invariants so we don't regress in
the future.

Author: acdlite

packages/react-noop-renderer/src/ReactNoop.js

@@ -130,6 +130,12 @@ let SharedHostConfig = {
     oldProps: Props,
     newProps: Props,
   ): null | {} {
+    if (oldProps === null) {
+      throw new Error('Should have old props');
+    }
+    if (newProps === null) {
+      throw new Error('Should have old props');
+    }
     return UPDATE_SIGNAL;
   },
 
@@ -196,6 +202,9 @@ const NoopRenderer = ReactFiberReconciler({
       oldProps: Props,
       newProps: Props,
     ): void {
+      if (oldProps === null) {
+        throw new Error('Should have old props');
+      }
       instance.prop = newProps.prop;
     },
 

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -474,7 +474,18 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
       // Normally we can bail out on props equality but if context has changed
       // we don't do the bailout and we have to reuse existing props instead.
     } else if (memoizedProps === nextProps) {
-      return bailoutOnAlreadyFinishedWork(current, workInProgress);
+      const isHidden =
+        workInProgress.mode & AsyncMode &&
+        shouldDeprioritizeSubtree(type, nextProps);
+      if (isHidden) {
+        // Before bailing out, make sure we've deprioritized a hidden component.
+        workInProgress.expirationTime = Never;
+      }
+      if (!isHidden || renderExpirationTime !== Never) {
+        return bailoutOnAlreadyFinishedWork(current, workInProgress);
+      }
+      // If we're rendering a hidden node at hidden priority, don't bailout. The
+      // parent is complete, but the children may not be.
     }
 
     let nextChildren = nextProps.children;
@@ -503,6 +514,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
       // Down-prioritize the children.
       workInProgress.expirationTime = Never;
       // Bailout and come back to this fiber later.
+      workInProgress.memoizedProps = nextProps;
       return null;
     }
 

packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js

@@ -758,7 +758,7 @@ describe('ReactIncrementalSideEffects', () => {
     // 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 + 5);
+    ReactNoop.flushDeferredPri(35);
     expect(ReactNoop.getChildren()).toEqual([
       div(
         // Updated.
@@ -790,7 +790,7 @@ describe('ReactIncrementalSideEffects', () => {
       ),
     ]);
 
-    expect(ops).toEqual(['Bar', 'Bar']);
+    expect(ops).toEqual(['Bar', '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