Refactor to use module-level variable instead of effect bit

facebook · Jan 17, 2019 · e7a99f9 · e7a99f9
1 parent 535d76c
commit e7a99f9
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 32 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -90,7 +90,7 @@ import {
   prepareToReadContext,
   calculateChangedBits,
 } from './ReactFiberNewContext';
-import {resetHooks, renderWithHooks} from './ReactFiberHooks';
+import {resetHooks, renderWithHooks, bailoutHooks} from './ReactFiberHooks';
 import {stopProfilerTimerIfRunning} from './ReactProfilerTimer';
 import {
   getMaskedContext,
@@ -128,6 +128,8 @@ import {
 
 const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
 
+let didReceiveUpdate: boolean = false;
+
 let didWarnAboutBadClass;
 let didWarnAboutContextTypeOnFunctionComponent;
 let didWarnAboutGetDerivedStateOnFunctionComponent;
@@ -260,14 +262,17 @@ function updateForwardRef(
     );
   }
 
-  if ((workInProgress.effectTag & PerformedWork) === NoEffect) {
+  if (current !== null && !didReceiveUpdate) {
+    bailoutHooks(current, workInProgress, renderExpirationTime);
     return bailoutOnAlreadyFinishedWork(
       current,
       workInProgress,
       renderExpirationTime,
     );
   }
 
+  // React DevTools reads this flag.
+  workInProgress.effectTag |= PerformedWork;
   reconcileChildren(
     current,
     workInProgress,
@@ -368,6 +373,8 @@ function updateMemoComponent(
       );
     }
   }
+  // React DevTools reads this flag.
+  workInProgress.effectTag |= PerformedWork;
   let newChild = createWorkInProgress(
     currentChild,
     nextProps,
@@ -411,17 +418,20 @@ function updateSimpleMemoComponent(
       // Inner propTypes will be validated in the function component path.
     }
   }
-  if (current !== null && updateExpirationTime < renderExpirationTime) {
+  if (current !== null) {
     const prevProps = current.memoizedProps;
     if (
       shallowEqual(prevProps, nextProps) &&
       current.ref === workInProgress.ref
     ) {
-      return bailoutOnAlreadyFinishedWork(
-        current,
-        workInProgress,
-        renderExpirationTime,
-      );
+      didReceiveUpdate = false;
+      if (updateExpirationTime < renderExpirationTime) {
+        return bailoutOnAlreadyFinishedWork(
+          current,
+          workInProgress,
+          renderExpirationTime,
+        );
+      }
     }
   }
   return updateFunctionComponent(
@@ -545,14 +555,16 @@ function updateFunctionComponent(
     );
   }
 
-  if ((workInProgress.effectTag & PerformedWork) === NoEffect) {
+  if (current !== null && !didReceiveUpdate) {
+    bailoutHooks(current, workInProgress, renderExpirationTime);
     return bailoutOnAlreadyFinishedWork(
       current,
       workInProgress,
       renderExpirationTime,
     );
   }
 
+  workInProgress.effectTag |= PerformedWork;
   reconcileChildren(
     current,
     workInProgress,
@@ -1142,6 +1154,8 @@ function mountIndeterminateComponent(
       renderExpirationTime,
     );
   }
+  // React DevTools reads this flag.
+  workInProgress.effectTag |= PerformedWork;
 
   if (
     typeof value === 'object' &&
@@ -1682,6 +1696,10 @@ function updateContextConsumer(
   return workInProgress.child;
 }
 
+export function markWorkInProgressReceivedUpdate() {
+  didReceiveUpdate = true;
+}
+
 function bailoutOnAlreadyFinishedWork(
   current: Fiber | null,
   workInProgress: Fiber,
@@ -1694,8 +1712,6 @@ function bailoutOnAlreadyFinishedWork(
     workInProgress.contextDependencies = current.contextDependencies;
   }
 
-  workInProgress.effectTag &= ~PerformedWork;
-
   if (enableProfilerTimer) {
     // Don't update "base" render times for bailouts.
     stopProfilerTimerIfRunning(workInProgress);
@@ -1730,8 +1746,9 @@ function beginWork(
     if (oldProps !== newProps || hasLegacyContextChanged()) {
       // If props or context changed, mark the fiber as having performed work.
       // This may be unset if the props are determined to be equal later (memo).
-      workInProgress.effectTag |= PerformedWork;
+      didReceiveUpdate = true;
     } else if (updateExpirationTime < renderExpirationTime) {
+      didReceiveUpdate = false;
       // This fiber does not have any pending work. Bailout without entering
       // the begin phase. There's still some bookkeeping we that needs to be done
       // in this optimized path, mostly pushing stuff onto the stack.
@@ -1815,8 +1832,7 @@ function beginWork(
       );
     }
   } else {
-    // No bailouts on initial mount.
-    workInProgress.effectTag |= PerformedWork;
+    didReceiveUpdate = false;
   }
 
   // Before entering the begin phase, clear the expiration time.

diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -18,8 +18,6 @@ import {readContext} from './ReactFiberNewContext';
 import {
   Update as UpdateEffect,
   Passive as PassiveEffect,
-  PerformedWork,
-  NoEffect,
 } from 'shared/ReactSideEffectTags';
 import {
   NoEffect as NoHookEffect,
@@ -37,6 +35,7 @@ import {
 
 import invariant from 'shared/invariant';
 import areHookInputsEqual from 'shared/areHookInputsEqual';
+import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
 
 type Update<S, A> = {
   expirationTime: ExpirationTime,
@@ -171,18 +170,6 @@ export function renderWithHooks(
 
   const renderedWork: Fiber = (currentlyRenderingFiber: any);
 
-  if (
-    current !== null &&
-    (renderedWork.effectTag & PerformedWork) === NoEffect
-  ) {
-    // If nothing updated, clear the effects. We're going to bail out.
-    componentUpdateQueue = (current.updateQueue: any);
-    renderedWork.effectTag &= ~(PassiveEffect | UpdateEffect);
-    if (current.expirationTime <= renderExpirationTime) {
-      current.expirationTime = NoWork;
-    }
-  }
-
   renderedWork.memoizedState = firstWorkInProgressHook;
   renderedWork.expirationTime = remainingExpirationTime;
   renderedWork.updateQueue = componentUpdateQueue;
@@ -218,6 +205,18 @@ export function renderWithHooks(
   return children;
 }
 
+export function bailoutHooks(
+  current: Fiber,
+  workInProgress: Fiber,
+  expirationTime: ExpirationTime,
+) {
+  workInProgress.updateQueue = current.updateQueue;
+  workInProgress.effectTag &= ~(PassiveEffect | UpdateEffect);
+  if (current.expirationTime <= expirationTime) {
+    current.expirationTime = NoWork;
+  }
+}
+
 export function resetHooks(): void {
   if (!enableHooks) {
     return;
@@ -462,7 +461,7 @@ export function useReducer<S, A>(
       // Mark that the fiber performed work, but only if the new state is
       // different from the current state.
       if (newState !== (currentHook: any).memoizedState) {
-        currentlyRenderingFiber.effectTag |= PerformedWork;
+        markWorkInProgressReceivedUpdate();
       }
 
       queue.eagerReducer = reducer;
@@ -489,7 +488,6 @@ export function useReducer<S, A>(
     eagerReducer: reducer,
     eagerState: initialState,
   };
-  currentlyRenderingFiber.effectTag |= PerformedWork;
   const dispatch: Dispatch<A> = (queue.dispatch = (dispatchAction.bind(
     null,
     currentlyRenderingFiber,

diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js
@@ -38,7 +38,7 @@ import {
   ForceUpdate,
 } from 'react-reconciler/src/ReactUpdateQueue';
 import {NoWork} from './ReactFiberExpirationTime';
-import {PerformedWork} from 'shared/ReactSideEffectTags';
+import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
 
 const valueCursor: StackCursor<mixed> = createCursor(null);
 
@@ -268,7 +268,7 @@ export function prepareToReadContext(
     currentDependencies.expirationTime >= renderExpirationTime
   ) {
     // Context list has a pending update. Mark that this fiber performed work.
-    workInProgress.effectTag |= PerformedWork;
+    markWorkInProgressReceivedUpdate();
   }
 
   // Reset the work-in-progress list