warn if passive effects get queued outside of an act() call.

based on facebook#15591. of note, we don't modify our own tests to satisfy this warning, instead using a feature flag to disable the warning for some tests. this is because we're testing the actual sequencing of work in these tests, and don't want to flush everything with act().
threepointone · May 12, 2019 · 9f7a346 · 9f7a346
1 parent 6920e9f
commit 9f7a346
Show file tree

Hide file tree

Showing 20 changed files with 81 additions and 10 deletions.
diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
@@ -14,11 +14,14 @@ let React;
 let ReactTestRenderer;
 let Scheduler;
 let ReactDebugTools;
+let ReactFeatureFlags;
 let act;
 
 describe('ReactHooksInspectionIntegration', () => {
   beforeEach(() => {
     jest.resetModules();
+    ReactFeatureFlags = require('shared/ReactFeatureFlags');
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
     React = require('react');
     ReactTestRenderer = require('react-test-renderer');
     Scheduler = require('scheduler');

diff --git a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js
@@ -11,6 +11,7 @@
 
 let React;
 let ReactDOM;
+let ReactFeatureFlags;
 let Scheduler;
 
 describe('ReactDOMHooks', () => {
@@ -22,6 +23,8 @@ describe('ReactDOMHooks', () => {
     React = require('react');
     ReactDOM = require('react-dom');
     Scheduler = require('scheduler');
+    ReactFeatureFlags = require('shared/ReactFeatureFlags');
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
 
     container = document.createElement('div');
     document.body.appendChild(container);

diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js
@@ -38,6 +38,7 @@ function initModules() {
 
   ReactFeatureFlags = require('shared/ReactFeatureFlags');
   ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
+  ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');

diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js
@@ -43,6 +43,7 @@ describe('ReactErrorBoundaries', () => {
     PropTypes = require('prop-types');
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
     ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
     ReactDOM = require('react-dom');
     React = require('react');
     Scheduler = require('scheduler');

diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js
@@ -12,6 +12,7 @@
 let React;
 let ReactDOM;
 let ReactTestUtils;
+let ReactFeatureFlags;
 let Scheduler;
 
 describe('ReactUpdates', () => {
@@ -20,6 +21,8 @@ describe('ReactUpdates', () => {
     React = require('react');
     ReactDOM = require('react-dom');
     ReactTestUtils = require('react-dom/test-utils');
+    ReactFeatureFlags = require('shared/ReactFeatureFlags');
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
     Scheduler = require('scheduler');
   });
 

diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -33,7 +33,8 @@ import {
   computeExpirationForFiber,
   flushPassiveEffects,
   requestCurrentTime,
-  warnIfNotCurrentlyActingUpdatesInDev,
+  warnIfNotCurrentlyActingEffectsInDEV,
+  warnIfNotCurrentlyActingUpdatesInDEV,
   markRenderEventTime,
 } from './ReactFiberScheduler';
 
@@ -869,6 +870,16 @@ function mountEffect(
   create: () => (() => void) | void,
   deps: Array<mixed> | void | null,
 ): void {
+  if (__DEV__) {
+    // jest isn't a 'global', it's just exposed to tests via a wrapped function
+    // further, this isn't a test file, so flow doesn't recognize the symbol. So...
+    // $FlowExpectedError - because requirements don't give a damn about your type sigs.
+    if ('undefined' !== typeof jest) {
+      warnIfNotCurrentlyActingEffectsInDEV(
+        ((currentlyRenderingFiber: any): Fiber),
+      );
+    }
+  }
   return mountEffectImpl(
     UpdateEffect | PassiveEffect,
     UnmountPassive | MountPassive,
@@ -881,6 +892,16 @@ function updateEffect(
   create: () => (() => void) | void,
   deps: Array<mixed> | void | null,
 ): void {
+  if (__DEV__) {
+    // jest isn't a 'global', it's just exposed to tests via a wrapped function
+    // further, this isn't a test file, so flow doesn't recognize the symbol. So...
+    // $FlowExpectedError - because requirements don't give a damn about your type sigs.
+    if ('undefined' !== typeof jest) {
+      warnIfNotCurrentlyActingEffectsInDEV(
+        ((currentlyRenderingFiber: any): Fiber),
+      );
+    }
+  }
   return updateEffectImpl(
     UpdateEffect | PassiveEffect,
     UnmountPassive | MountPassive,
@@ -1180,7 +1201,7 @@ function dispatchAction<S, A>(
       // further, this isn't a test file, so flow doesn't recognize the symbol. So...
       // $FlowExpectedError - because requirements don't give a damn about your type sigs.
       if ('undefined' !== typeof jest) {
-        warnIfNotCurrentlyActingUpdatesInDev(fiber);
+        warnIfNotCurrentlyActingUpdatesInDEV(fiber);
       }
     }
     scheduleWork(fiber, expirationTime);

diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js
@@ -24,6 +24,7 @@ import {
   enableProfilerTimer,
   disableYielding,
   enableSchedulerTracing,
+  warnAboutUnactedEffectsinDEV,
 } from 'shared/ReactFeatureFlags';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import invariant from 'shared/invariant';
@@ -2088,7 +2089,32 @@ function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) {
   }
 }
 
-function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
+export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
+  if (__DEV__) {
+    if (warnAboutUnactedEffectsinDEV) {
+      if (ReactShouldWarnActingUpdates.current === false) {
+        warningWithoutStack(
+          false,
+          'An effect from %s inside a test was not wrapped in act(...).\n\n' +
+            'When testing, code that causes React state updates should be ' +
+            'wrapped into act(...):\n\n' +
+            'act(() => {\n' +
+            '  /* fire events that update state */\n' +
+            '});\n' +
+            '/* assert on the output */\n\n' +
+            "This ensures that you're testing the behavior the user would see " +
+            'in the browser.' +
+            ' Learn more at https://fb.me/react-wrap-tests-with-act' +
+            '%s',
+          getComponentName(fiber.type),
+          getStackByFiberInDevAndProd(fiber),
+        );
+      }
+    }
+  }
+}
+
+export function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
   if (__DEV__) {
     if (
       workPhase === NotWorking &&
@@ -2114,8 +2140,6 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
   }
 }
 
-export const warnIfNotCurrentlyActingUpdatesInDev = warnIfNotCurrentlyActingUpdatesInDEV;
-
 let componentsWithSuspendedDiscreteUpdates = null;
 export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
   if (__DEV__) {

diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
@@ -27,6 +27,7 @@ describe('ReactHooks', () => {
 
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
     ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
     React = require('react');
     ReactTestRenderer = require('react-test-renderer');
     Scheduler = require('scheduler');

diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
@@ -39,6 +39,7 @@ describe('ReactHooksWithNoopRenderer', () => {
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
     ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
     ReactFeatureFlags.enableSchedulerTracing = true;
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
     React = require('react');
     ReactNoop = require('react-noop-renderer');
     Scheduler = require('scheduler');

diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js
@@ -20,6 +20,7 @@ describe('ReactIncrementalScheduling', () => {
     jest.resetModules();
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
     ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
     React = require('react');
     ReactNoop = require('react-noop-renderer');
     Scheduler = require('scheduler');

diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js
@@ -20,6 +20,7 @@ describe('ReactIncrementalUpdates', () => {
     jest.resetModuleRegistry();
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
     ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
     React = require('react');
     ReactNoop = require('react-noop-renderer');
     Scheduler = require('scheduler');

diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js
@@ -26,6 +26,7 @@ describe('ReactSchedulerIntegration', () => {
     jest.resetModules();
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
     ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
+    ReactFeatureFlags.warnAboutUnactedEffectsinDEV = false;
     React = require('react');
     ReactNoop = require('react-noop-renderer');
     Scheduler = require('scheduler');

diff --git a/packages/react/src/__tests__/withComponentStack-test.js b/packages/react/src/__tests__/withComponentStack-test.js
@@ -44,7 +44,6 @@ describe('withComponentStack', () => {
   let React = null;
   let ReactTestRenderer = null;
   let error = null;
-  let scheduler = null;
   let warn = null;
 
   beforeEach(() => {
@@ -53,7 +52,6 @@ describe('withComponentStack', () => {
 
     React = require('react');
     ReactTestRenderer = require('react-test-renderer');
-    scheduler = require('scheduler');
 
     error = React.error;
     warn = React.warn;
@@ -179,9 +177,9 @@ describe('withComponentStack', () => {
         return null;
       }
 
-      ReactTestRenderer.create(<Parent />);
-
-      scheduler.flushAll(); // Flush passive effects
+      ReactTestRenderer.act(() => {
+        ReactTestRenderer.create(<Parent />);
+      });
 
       expectMessageAndStack(
         'logged in child render method',

diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -71,3 +71,8 @@ export const enableJSXTransformAPI = false;
 // We will enforce mocking scheduler with scheduler/unstable_mock at some point. (v17?)
 // Till then, we warn about the missing mock, but still fallback to a sync mode compatible version
 export const warnAboutMissingMockScheduler = false;
+
+// We warn if effects are queued in tests without a surround act()
+// However, this interferes withour internal tests that test the actual
+// sequencing work. So we use a feature flag to disable the warning *only* for ourselves
+export const warnAboutUnactedEffectsinDEV = true;
diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -33,6 +33,7 @@ export const warnAboutDeprecatedSetNativeProps = true;
 export const enableEventAPI = false;
 export const enableJSXTransformAPI = false;
 export const warnAboutMissingMockScheduler = true;
+export const warnAboutUnactedEffectsinDEV = true;
 
 // Only used in www builds.
 export function addUserTimingListener() {

diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -30,6 +30,7 @@ export const warnAboutDeprecatedSetNativeProps = false;
 export const enableEventAPI = false;
 export const enableJSXTransformAPI = false;
 export const warnAboutMissingMockScheduler = false;
+export const warnAboutUnactedEffectsinDEV = true;
 
 // Only used in www builds.
 export function addUserTimingListener() {

diff --git a/packages/shared/forks/ReactFeatureFlags.persistent.js b/packages/shared/forks/ReactFeatureFlags.persistent.js
@@ -30,6 +30,7 @@ export const warnAboutDeprecatedSetNativeProps = false;
 export const enableEventAPI = false;
 export const enableJSXTransformAPI = false;
 export const warnAboutMissingMockScheduler = true;
+export const warnAboutUnactedEffectsinDEV = true;
 
 // Only used in www builds.
 export function addUserTimingListener() {

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -30,6 +30,7 @@ export const warnAboutDeprecatedSetNativeProps = false;
 export const enableEventAPI = false;
 export const enableJSXTransformAPI = false;
 export const warnAboutMissingMockScheduler = false;
+export const warnAboutUnactedEffectsinDEV = true;
 
 // Only used in www builds.
 export function addUserTimingListener() {

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -28,6 +28,7 @@ export const disableYielding = false;
 export const enableEventAPI = true;
 export const enableJSXTransformAPI = true;
 export const warnAboutMissingMockScheduler = true;
+export const warnAboutUnactedEffectsinDEV = true;
 
 // Only used in www builds.
 export function addUserTimingListener() {

diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -73,6 +73,8 @@ export const enableJSXTransformAPI = true;
 
 export const warnAboutMissingMockScheduler = true;
 
+export const warnAboutUnactedEffectsinDEV = true;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;