Test bad useEffect return value with noop-renderer (#22258)

* Test bad useEffect return value with noop-renderer * Use previous "root"-approach Tests should now be invariant under variants * Add same test for layout effects
facebook · Sep 8, 2021 · 4ce89a5 · 4ce89a5
1 parent a3fde23
commit 4ce89a5
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 36 deletions.
diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
@@ -727,42 +727,6 @@ describe('ReactHooks', () => {
     ReactTestRenderer.create(<App deps={undefined} />);
   });
 
-  it('assumes useEffect clean-up function is either a function or undefined', () => {
-    const {useLayoutEffect} = React;
-
-    function App(props) {
-      useLayoutEffect(() => {
-        return props.return;
-      });
-      return null;
-    }
-
-    const root1 = ReactTestRenderer.create(null);
-    expect(() => root1.update(<App return={17} />)).toErrorDev([
-      'Warning: An effect function must not return anything besides a ' +
-        'function, which is used for clean-up. You returned: 17',
-    ]);
-
-    const root2 = ReactTestRenderer.create(null);
-    expect(() => root2.update(<App return={null} />)).toErrorDev([
-      'Warning: An effect function must not return anything besides a ' +
-        'function, which is used for clean-up. You returned null. If your ' +
-        'effect does not require clean up, return undefined (or nothing).',
-    ]);
-
-    const root3 = ReactTestRenderer.create(null);
-    expect(() => root3.update(<App return={Promise.resolve()} />)).toErrorDev([
-      'Warning: An effect function must not return anything besides a ' +
-        'function, which is used for clean-up.\n\n' +
-        'It looks like you wrote useEffect(async () => ...) or returned a Promise.',
-    ]);
-
-    // Error on unmount because React assumes the value is a function
-    expect(() => {
-      root3.update(null);
-    }).toThrow('is not a function');
-  });
-
   it('does not forget render phase useState updates inside an effect', () => {
     const {useState, useEffect} = React;
 

diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
@@ -2632,6 +2632,54 @@ describe('ReactHooksWithNoopRenderer', () => {
       });
       expect(Scheduler).toHaveYielded(['layout destroy', 'passive destroy']);
     });
+
+    it('assumes passive effect destroy function is either a function or undefined', () => {
+      function App(props) {
+        useEffect(() => {
+          return props.return;
+        });
+        return null;
+      }
+
+      const root1 = ReactNoop.createRoot();
+      expect(() =>
+        act(() => {
+          root1.render(<App return={17} />);
+        }),
+      ).toErrorDev([
+        'Warning: An effect function must not return anything besides a ' +
+          'function, which is used for clean-up. You returned: 17',
+      ]);
+
+      const root2 = ReactNoop.createRoot();
+      expect(() =>
+        act(() => {
+          root2.render(<App return={null} />);
+        }),
+      ).toErrorDev([
+        'Warning: An effect function must not return anything besides a ' +
+          'function, which is used for clean-up. You returned null. If your ' +
+          'effect does not require clean up, return undefined (or nothing).',
+      ]);
+
+      const root3 = ReactNoop.createRoot();
+      expect(() =>
+        act(() => {
+          root3.render(<App return={Promise.resolve()} />);
+        }),
+      ).toErrorDev([
+        'Warning: An effect function must not return anything besides a ' +
+          'function, which is used for clean-up.\n\n' +
+          'It looks like you wrote useEffect(async () => ...) or returned a Promise.',
+      ]);
+
+      // Error on unmount because React assumes the value is a function
+      expect(() =>
+        act(() => {
+          root3.unmount();
+        }),
+      ).toThrow('is not a function');
+    });
   });
 
   describe('useLayoutEffect', () => {
@@ -2810,6 +2858,54 @@ describe('ReactHooksWithNoopRenderer', () => {
       ]);
       expect(ReactNoop.getChildren()).toEqual([span('OuterFallback')]);
     });
+
+    it('assumes layout effect destroy function is either a function or undefined', () => {
+      function App(props) {
+        useLayoutEffect(() => {
+          return props.return;
+        });
+        return null;
+      }
+
+      const root1 = ReactNoop.createRoot();
+      expect(() =>
+        act(() => {
+          root1.render(<App return={17} />);
+        }),
+      ).toErrorDev([
+        'Warning: An effect function must not return anything besides a ' +
+          'function, which is used for clean-up. You returned: 17',
+      ]);
+
+      const root2 = ReactNoop.createRoot();
+      expect(() =>
+        act(() => {
+          root2.render(<App return={null} />);
+        }),
+      ).toErrorDev([
+        'Warning: An effect function must not return anything besides a ' +
+          'function, which is used for clean-up. You returned null. If your ' +
+          'effect does not require clean up, return undefined (or nothing).',
+      ]);
+
+      const root3 = ReactNoop.createRoot();
+      expect(() =>
+        act(() => {
+          root3.render(<App return={Promise.resolve()} />);
+        }),
+      ).toErrorDev([
+        'Warning: An effect function must not return anything besides a ' +
+          'function, which is used for clean-up.\n\n' +
+          'It looks like you wrote useEffect(async () => ...) or returned a Promise.',
+      ]);
+
+      // Error on unmount because React assumes the value is a function
+      expect(() =>
+        act(() => {
+          root3.unmount();
+        }),
+      ).toThrow('is not a function');
+    });
   });
 
   describe('useCallback', () => {