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

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

While the code itself isn't much (it adds the warning to mountEffect() and updateEffect() in ReactFiberHooks), it does change a lot of our tests. We follow a bad-ish pattern here, which is doing asserts inside act() scopes, but it makes sense for *us* because we're testing intermediate states, and we're manually flush/yield what we need in these tests.

This commit has one last failing test. Working on it.

* pass lint

* pass failing test, fixes another

- a test was failing in ReactDOMServerIntegrationHooks while testing an effect; the behaviour of yields was different from browser and server when wrapped with act(). further, because of how we initialized modules, act() around renders wasn't working corrrectly. solved by passing in ReactTestUtils in initModules, and checking on the finally yielded values in the specific test.
- in ReactUpdates, while testing an infinite recursion detection, the test needed to be wrapped in an act(), which would have caused the recusrsion error to throw. solived by rethrowing the error from inside the act().

* pass ReactDOMServerSuspense

* stray todo

* a better message, consistent with the state update one.

Author: Sunil Pai

packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js

@@ -146,7 +146,10 @@ describe('ReactHooksInspectionIntegration', () => {
         </div>
       );
     }
-    let renderer = ReactTestRenderer.create(<Foo prop="prop" />);
+    let renderer;
+    act(() => {
+      renderer = ReactTestRenderer.create(<Foo prop="prop" />);
+    });
 
     let childFiber = renderer.root.findByType(Foo)._currentFiber();
 

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

@@ -53,23 +53,32 @@ describe('ReactDOMHooks', () => {
       return 3 * n;
     }
 
-    ReactDOM.render(<Example1 n={1} />, container);
-    expect(container.textContent).toBe('1');
-    expect(container2.textContent).toBe('');
-    expect(container3.textContent).toBe('');
-    Scheduler.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.flushAll();
-    expect(container.textContent).toBe('2');
-    expect(container2.textContent).toBe('4');
-    expect(container3.textContent).toBe('6');
+    // 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.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.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',
+    ]);
   });
 
   it('should not bail out when an update is scheduled from within an event handler', () => {

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

@@ -13,6 +13,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio
 
 let React;
 let ReactDOM;
+let ReactTestUtils;
 let ReactDOMServer;
 
 function initModules() {
@@ -21,11 +22,13 @@ function initModules() {
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');
+  ReactTestUtils = require('react-dom/test-utils');
 
   // Make them available to the helpers.
   return {
     ReactDOM,
     ReactDOMServer,
+    ReactTestUtils,
   };
 }
 

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

@@ -16,18 +16,21 @@ const TEXT_NODE_TYPE = 3;
 let React;
 let ReactDOM;
 let ReactDOMServer;
+let ReactTestUtils;
 
 function initModules() {
   // Reset warning cache.
   jest.resetModuleRegistry();
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');
+  ReactTestUtils = require('react-dom/test-utils');
 
   // Make them available to the helpers.
   return {
     ReactDOM,
     ReactDOMServer,
+    ReactTestUtils,
   };
 }
 

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

@@ -16,18 +16,21 @@ const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');
 let React;
 let ReactDOM;
 let ReactDOMServer;
+let ReactTestUtils;
 
 function initModules() {
   // Reset warning cache.
   jest.resetModuleRegistry();
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');
+  ReactTestUtils = require('react-dom/test-utils');
 
   // Make them available to the helpers.
   return {
     ReactDOM,
     ReactDOMServer,
+    ReactTestUtils,
   };
 }
 

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

@@ -14,18 +14,21 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio
 let React;
 let ReactDOM;
 let ReactDOMServer;
+let ReactTestUtils;
 
 function initModules() {
   // Reset warning cache.
   jest.resetModuleRegistry();
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');
+  ReactTestUtils = require('react-dom/test-utils');
 
   // Make them available to the helpers.
   return {
     ReactDOM,
     ReactDOMServer,
+    ReactTestUtils,
   };
 }
 

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

@@ -16,17 +16,20 @@ const TEXT_NODE_TYPE = 3;
 let React;
 let ReactDOM;
 let ReactDOMServer;
+let ReactTestUtils;
 
 function initModules() {
   jest.resetModuleRegistry();
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');
+  ReactTestUtils = require('react-dom/test-utils');
 
   // Make them available to the helpers.
   return {
     ReactDOM,
     ReactDOMServer,
+    ReactTestUtils,
   };
 }
 

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

@@ -14,18 +14,21 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio
 let React;
 let ReactDOM;
 let ReactDOMServer;
+let ReactTestUtils;
 
 function initModules() {
   // Reset warning cache.
   jest.resetModuleRegistry();
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');
+  ReactTestUtils = require('react-dom/test-utils');
 
   // Make them available to the helpers.
   return {
     ReactDOM,
     ReactDOMServer,
+    ReactTestUtils,
   };
 }
 

packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js

@@ -17,6 +17,7 @@ let React;
 let ReactFeatureFlags;
 let ReactDOM;
 let ReactDOMServer;
+let ReactTestUtils;
 let useState;
 let useReducer;
 let useEffect;
@@ -41,6 +42,7 @@ function initModules() {
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');
+  ReactTestUtils = require('react-dom/test-utils');
   useState = React.useState;
   useReducer = React.useReducer;
   useEffect = React.useEffect;
@@ -67,6 +69,7 @@ function initModules() {
   return {
     ReactDOM,
     ReactDOMServer,
+    ReactTestUtils,
   };
 }
 
@@ -539,18 +542,30 @@ describe('ReactDOMServerHooks', () => {
   });
 
   describe('useEffect', () => {
+    const yields = [];
     itRenders('should ignore effects on the server', async render => {
       function Counter(props) {
         useEffect(() => {
-          yieldValue('should not be invoked');
+          yieldValue('invoked on client');
         });
         return <Text text={'Count: ' + props.count} />;
       }
+
       const domNode = await render(<Counter count={0} />);
-      expect(clearYields()).toEqual(['Count: 0']);
+      yields.push(clearYields());
       expect(domNode.tagName).toEqual('SPAN');
       expect(domNode.textContent).toEqual('Count: 0');
     });
+
+    it('verifies yields in order', () => {
+      expect(yields).toEqual([
+        ['Count: 0'], // server render
+        ['Count: 0'], // server stream
+        ['Count: 0', 'invoked on client'], // clean render
+        ['Count: 0', 'invoked on client'], // hydrated render
+        // nothing yielded for bad markup
+      ]);
+    });
   });
 
   describe('useCallback', () => {

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

@@ -16,18 +16,21 @@ const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');
 let React;
 let ReactDOM;
 let ReactDOMServer;
+let ReactTestUtils;
 
 function initModules() {
   // Reset warning cache.
   jest.resetModuleRegistry();
   React = require('react');
   ReactDOM = require('react-dom');
   ReactDOMServer = require('react-dom/server');
+  ReactTestUtils = require('react-dom/test-utils');
 
   // Make them available to the helpers.
   return {
     ReactDOM,
     ReactDOMServer,
+    ReactTestUtils,
   };
 }