[fail] Only warn on unacted effects for strict / non sync modes (face…

…book#16041)

* only warn on unacted effects for strict / non sync modes

(basically, when `fiber.mode !== 0b0000`)

Warnings on unacted effects may be too noisy, especially for legacy apps. This PR fires the warning only when in a non sync mode (concurrent/batched), or when in strict mode. This should make updating easier.

I also added batched mode tests to the act() suite.

* explicitly check for modes before warning, explicit tests for all modes

packages/react-dom/src/__tests__/ReactDOMHooks-test.js

@@ -53,32 +53,23 @@ describe('ReactDOMHooks', () => {
       return 3 * n;
     }
 
-    // we explicitly catch the missing act() warnings
-    // to simulate this tricky repro
-    expect(() => {
-      ReactDOM.render(<Example1 n={1} />, container);
-      expect(container.textContent).toBe('1');
-      expect(container2.textContent).toBe('');
-      expect(container3.textContent).toBe('');
-      Scheduler.unstable_flushAll();
-      expect(container.textContent).toBe('1');
-      expect(container2.textContent).toBe('2');
-      expect(container3.textContent).toBe('3');
-
-      ReactDOM.render(<Example1 n={2} />, container);
-      expect(container.textContent).toBe('2');
-      expect(container2.textContent).toBe('2'); // Not flushed yet
-      expect(container3.textContent).toBe('3'); // Not flushed yet
-      Scheduler.unstable_flushAll();
-      expect(container.textContent).toBe('2');
-      expect(container2.textContent).toBe('4');
-      expect(container3.textContent).toBe('6');
-    }).toWarnDev([
-      'An update to Example1 ran an effect',
-      'An update to Example2 ran an effect',
-      'An update to Example1 ran an effect',
-      'An update to Example2 ran an effect',
-    ]);
+    ReactDOM.render(<Example1 n={1} />, container);
+    expect(container.textContent).toBe('1');
+    expect(container2.textContent).toBe('');
+    expect(container3.textContent).toBe('');
+    Scheduler.unstable_flushAll();
+    expect(container.textContent).toBe('1');
+    expect(container2.textContent).toBe('2');
+    expect(container3.textContent).toBe('3');
+
+    ReactDOM.render(<Example1 n={2} />, container);
+    expect(container.textContent).toBe('2');
+    expect(container2.textContent).toBe('2'); // Not flushed yet
+    expect(container3.textContent).toBe('3'); // Not flushed yet
+    Scheduler.unstable_flushAll();
+    expect(container.textContent).toBe('2');
+    expect(container2.textContent).toBe('4');
+    expect(container3.textContent).toBe('6');
   });
 
   it('should not bail out when an update is scheduled from within an event handler', () => {

packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js

@@ -48,6 +48,73 @@ describe('ReactTestUtils.act()', () => {
     ReactDOM.unmountComponentAtNode(dom);
   }
   runActTests('legacy sync mode', renderSync, unmountSync);
+
+  // and then in batched mode
+  let batchedRoot;
+  function renderBatched(el, dom) {
+    batchedRoot = ReactDOM.unstable_createSyncRoot(dom);
+    batchedRoot.render(el);
+  }
+  function unmountBatched(dom) {
+    if (batchedRoot !== null) {
+      batchedRoot.unmount();
+      batchedRoot = null;
+    }
+  }
+  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(<App />, document.createElement('div'));
+      }).toWarnDev([]);
+    });
+
+    it('warns in strict mode', () => {
+      expect(() => {
+        ReactDOM.render(
+          <React.StrictMode>
+            <App />
+          </React.StrictMode>,
+          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(<App />);
+        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(<App />);
+        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) {

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -61,6 +61,7 @@ import {
 import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber';
 import {
   NoMode,
+  StrictMode,
   ProfileMode,
   BatchedMode,
   ConcurrentMode,
@@ -2453,6 +2454,10 @@ export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
   if (__DEV__) {
     if (
       warnsIfNotActing === true &&
+      (fiber.mode & StrictMode ||
+        fiber.mode & ProfileMode ||
+        fiber.mode & BatchedMode ||
+        fiber.mode & ConcurrentMode) &&
       IsSomeRendererActing.current === false &&
       IsThisRendererActing.current === false
     ) {

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

@@ -1806,7 +1806,6 @@ describe('ReactHooks', () => {
       globalListener();
       globalListener();
     }).toWarnDev([
-      'An update to C ran an effect',
       'An update to C inside a test was not wrapped in act',
       'An update to C inside a test was not wrapped in act',
       // Note: should *not* warn about updates on unmounted component.