Context providers and consumers should bailout on already finished wo…

…rk (#12254) * Context providers and consumers should bail-out on already finished work Fixes bug where a consumer would re-render even if its props and context had not changed. * Encode output as JSON string * Add weights to random action generator * Add context to triangle fuzz tester * Move bailouts to as early as possible * Bailout if neither context value nor children haven't changed (sCU) * Change prop type invariant to a DEV-only warning
facebook · Mar 12, 2018 · c7f364d · c7f364d
1 parent 280acbc
commit c7f364d
Show file tree

Hide file tree

Showing 3 changed files with 367 additions and 111 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -857,58 +857,77 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
       pushProvider(workInProgress);
       return bailoutOnAlreadyFinishedWork(current, workInProgress);
     }
-    workInProgress.memoizedProps = newProps;
 
     const newValue = newProps.value;
+    workInProgress.memoizedProps = newProps;
 
     let changedBits: number;
     if (oldProps === null) {
       // Initial render
       changedBits = MAX_SIGNED_31_BIT_INT;
     } else {
-      const oldValue = oldProps.value;
-      // Use Object.is to compare the new context value to the old value.
-      // Inlined Object.is polyfill.
-      // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
-      if (
-        (oldValue === newValue &&
-          (oldValue !== 0 || 1 / oldValue === 1 / newValue)) ||
-        (oldValue !== oldValue && newValue !== newValue) // eslint-disable-line no-self-compare
-      ) {
-        // No change.
+      if (oldProps.value === newProps.value) {
+        // No change. Bailout early if children are the same.
+        if (oldProps.children === newProps.children) {
+          workInProgress.stateNode = 0;
+          pushProvider(workInProgress);
+          return bailoutOnAlreadyFinishedWork(current, workInProgress);
+        }
         changedBits = 0;
       } else {
-        changedBits =
-          typeof context.calculateChangedBits === 'function'
-            ? context.calculateChangedBits(oldValue, newValue)
-            : MAX_SIGNED_31_BIT_INT;
-        if (__DEV__) {
-          warning(
-            (changedBits & MAX_SIGNED_31_BIT_INT) === changedBits,
-            'calculateChangedBits: Expected the return value to be a ' +
-              '31-bit integer. Instead received: %s',
-            changedBits,
-          );
-        }
-        changedBits |= 0;
-
-        if (changedBits !== 0) {
-          propagateContextChange(
-            workInProgress,
-            context,
-            changedBits,
-            renderExpirationTime,
-          );
+        const oldValue = oldProps.value;
+        // Use Object.is to compare the new context value to the old value.
+        // Inlined Object.is polyfill.
+        // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
+        if (
+          (oldValue === newValue &&
+            (oldValue !== 0 || 1 / oldValue === 1 / newValue)) ||
+          (oldValue !== oldValue && newValue !== newValue) // eslint-disable-line no-self-compare
+        ) {
+          // No change. Bailout early if children are the same.
+          if (oldProps.children === newProps.children) {
+            workInProgress.stateNode = 0;
+            pushProvider(workInProgress);
+            return bailoutOnAlreadyFinishedWork(current, workInProgress);
+          }
+          changedBits = 0;
+        } else {
+          changedBits =
+            typeof context.calculateChangedBits === 'function'
+              ? context.calculateChangedBits(oldValue, newValue)
+              : MAX_SIGNED_31_BIT_INT;
+          if (__DEV__) {
+            warning(
+              (changedBits & MAX_SIGNED_31_BIT_INT) === changedBits,
+              'calculateChangedBits: Expected the return value to be a ' +
+                '31-bit integer. Instead received: %s',
+              changedBits,
+            );
+          }
+          changedBits |= 0;
+
+          if (changedBits === 0) {
+            // No change. Bailout early if children are the same.
+            if (oldProps.children === newProps.children) {
+              workInProgress.stateNode = 0;
+              pushProvider(workInProgress);
+              return bailoutOnAlreadyFinishedWork(current, workInProgress);
+            }
+          } else {
+            propagateContextChange(
+              workInProgress,
+              context,
+              changedBits,
+              renderExpirationTime,
+            );
+          }
         }
       }
     }
 
     workInProgress.stateNode = changedBits;
     pushProvider(workInProgress);
 
-    if (oldProps !== null && oldProps.children === newProps.children) {
-      return bailoutOnAlreadyFinishedWork(current, workInProgress);
-    }
     const newChildren = newProps.children;
     reconcileChildren(current, workInProgress, newChildren);
     return workInProgress.child;
@@ -921,11 +940,28 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
   ) {
     const context: ReactContext<any> = workInProgress.type;
     const newProps = workInProgress.pendingProps;
+    const oldProps = workInProgress.memoizedProps;
 
     const newValue = context.currentValue;
     const changedBits = context.changedBits;
 
-    if (changedBits !== 0) {
+    if (hasLegacyContextChanged()) {
+      // 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 (changedBits === 0 && oldProps === newProps) {
+      return bailoutOnAlreadyFinishedWork(current, workInProgress);
+    }
+    workInProgress.memoizedProps = newProps;
+
+    let observedBits = newProps.observedBits;
+    if (observedBits === undefined || observedBits === null) {
+      // Subscribe to all changes by default
+      observedBits = MAX_SIGNED_31_BIT_INT;
+    }
+    // Store the observedBits on the fiber's stateNode for quick access.
+    workInProgress.stateNode = observedBits;
+
+    if ((changedBits & observedBits) !== 0) {
       // Context change propagation stops at matching consumers, for time-
       // slicing. Continue the propagation here.
       propagateContextChange(
@@ -934,24 +970,20 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
         changedBits,
         renderExpirationTime,
       );
+    } else if (oldProps !== null && oldProps.children === newProps.children) {
+      // No change. Bailout early if children are the same.
+      return bailoutOnAlreadyFinishedWork(current, workInProgress);
     }
 
-    // Store the observedBits on the fiber's stateNode for quick access.
-    let observedBits = newProps.observedBits;
-    if (observedBits === undefined || observedBits === null) {
-      // Subscribe to all changes by default
-      observedBits = MAX_SIGNED_31_BIT_INT;
-    }
-    workInProgress.stateNode = observedBits;
-
     const render = newProps.children;
 
-    if (typeof render !== 'function') {
-      invariant(
-        false,
-        "A context consumer was rendered with multiple children, or a child that isn't a function. " +
-          'A context consumer expects a single child that is a function. ' +
-          'If you did pass a function, make sure there is no trailing or leading whitespace around it.',
+    if (__DEV__) {
+      warning(
+        typeof render === 'function',
+        'A context consumer was rendered with multiple children, or a child ' +
+          "that isn't a function. A context consumer expects a single child " +
+          'that is a function. If you did pass a function, make sure there ' +
+          'is no trailing or leading whitespace around it.',
       );
     }