From 5d2fb1e36c2dde0e86588873f2365b52ef5cde24 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 3 Jul 2019 01:03:35 +0100 Subject: [PATCH] explicitly check for modes before warning, explicit tests for all modes --- .../src/__tests__/ReactTestUtilsAct-test.js | 73 ++++++++++++++----- .../src/ReactFiberWorkLoop.js | 5 +- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js index ac37e5ba07ac9..827b3aa2f1855 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js @@ -62,6 +62,59 @@ describe('ReactTestUtils.act()', () => { } } runActTests('batched mode', renderBatched, unmountBatched); + + describe('unacted effects', () => { + function App() { + React.useEffect(() => {}, []); + return null; + } + + it('does not warn in legacy sync mode', () => { + expect(() => { + ReactDOM.render(, document.createElement('div')); + }).toWarnDev([]); + }); + + it('warns in strict mode', () => { + expect(() => { + ReactDOM.render( + + + , + document.createElement('div'), + ); + }).toWarnDev([ + 'An update to App ran an effect, but was not wrapped in act(...)', + 'An update to App ran an effect, but was not wrapped in act(...)', + ]); + }); + + it('warns in batched mode', () => { + expect(() => { + const root = ReactDOM.unstable_createSyncRoot( + document.createElement('div'), + ); + root.render(); + Scheduler.unstable_flushAll(); + }).toWarnDev([ + 'An update to App ran an effect, but was not wrapped in act(...)', + 'An update to App ran an effect, but was not wrapped in act(...)', + ]); + }); + + it('warns in concurrent mode', () => { + expect(() => { + const root = ReactDOM.unstable_createRoot( + document.createElement('div'), + ); + root.render(); + Scheduler.unstable_flushAll(); + }).toWarnDev([ + 'An update to App ran an effect, but was not wrapped in act(...)', + 'An update to App ran an effect, but was not wrapped in act(...)', + ]); + }); + }); }); function runActTests(label, render, unmount) { @@ -82,26 +135,6 @@ function runActTests(label, render, unmount) { document.body.removeChild(container); }); describe('sync', () => { - it('warns if an effect is queued outside an act scope, except in legacy sync+non-strict mode', () => { - function App() { - React.useEffect(() => {}, []); - return null; - } - expect(() => { - render(, container); - // flush all queued work - Scheduler.unstable_flushAll(); - }).toWarnDev( - label !== 'legacy sync mode' - ? [ - // warns twice because we're in strict+dev mode - 'An update to App ran an effect, but was not wrapped in act(...)', - 'An update to App ran an effect, but was not wrapped in act(...)', - ] - : [], - ); - }); - it('can use act to flush effects', () => { function App() { React.useEffect(() => { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index f8a389e63577b..a704f1e86398a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -61,6 +61,7 @@ import { import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber'; import { NoMode, + StrictMode, ProfileMode, BatchedMode, ConcurrentMode, @@ -2453,7 +2454,9 @@ export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void { if (__DEV__) { if ( warnsIfNotActing === true && - fiber.mode && + (fiber.mode & StrictMode || + fiber.mode & BatchedMode || + fiber.mode & ConcurrentMode) && IsSomeRendererActing.current === false && IsThisRendererActing.current === false ) {