Skip to content

Commit 4ce89a5

Browse files
authored
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
1 parent a3fde23 commit 4ce89a5

File tree

2 files changed

+96
-36
lines changed

2 files changed

+96
-36
lines changed

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

-36
Original file line numberDiff line numberDiff line change
@@ -727,42 +727,6 @@ describe('ReactHooks', () => {
727727
ReactTestRenderer.create(<App deps={undefined} />);
728728
});
729729

730-
it('assumes useEffect clean-up function is either a function or undefined', () => {
731-
const {useLayoutEffect} = React;
732-
733-
function App(props) {
734-
useLayoutEffect(() => {
735-
return props.return;
736-
});
737-
return null;
738-
}
739-
740-
const root1 = ReactTestRenderer.create(null);
741-
expect(() => root1.update(<App return={17} />)).toErrorDev([
742-
'Warning: An effect function must not return anything besides a ' +
743-
'function, which is used for clean-up. You returned: 17',
744-
]);
745-
746-
const root2 = ReactTestRenderer.create(null);
747-
expect(() => root2.update(<App return={null} />)).toErrorDev([
748-
'Warning: An effect function must not return anything besides a ' +
749-
'function, which is used for clean-up. You returned null. If your ' +
750-
'effect does not require clean up, return undefined (or nothing).',
751-
]);
752-
753-
const root3 = ReactTestRenderer.create(null);
754-
expect(() => root3.update(<App return={Promise.resolve()} />)).toErrorDev([
755-
'Warning: An effect function must not return anything besides a ' +
756-
'function, which is used for clean-up.\n\n' +
757-
'It looks like you wrote useEffect(async () => ...) or returned a Promise.',
758-
]);
759-
760-
// Error on unmount because React assumes the value is a function
761-
expect(() => {
762-
root3.update(null);
763-
}).toThrow('is not a function');
764-
});
765-
766730
it('does not forget render phase useState updates inside an effect', () => {
767731
const {useState, useEffect} = React;
768732

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js

+96
Original file line numberDiff line numberDiff line change
@@ -2632,6 +2632,54 @@ describe('ReactHooksWithNoopRenderer', () => {
26322632
});
26332633
expect(Scheduler).toHaveYielded(['layout destroy', 'passive destroy']);
26342634
});
2635+
2636+
it('assumes passive effect destroy function is either a function or undefined', () => {
2637+
function App(props) {
2638+
useEffect(() => {
2639+
return props.return;
2640+
});
2641+
return null;
2642+
}
2643+
2644+
const root1 = ReactNoop.createRoot();
2645+
expect(() =>
2646+
act(() => {
2647+
root1.render(<App return={17} />);
2648+
}),
2649+
).toErrorDev([
2650+
'Warning: An effect function must not return anything besides a ' +
2651+
'function, which is used for clean-up. You returned: 17',
2652+
]);
2653+
2654+
const root2 = ReactNoop.createRoot();
2655+
expect(() =>
2656+
act(() => {
2657+
root2.render(<App return={null} />);
2658+
}),
2659+
).toErrorDev([
2660+
'Warning: An effect function must not return anything besides a ' +
2661+
'function, which is used for clean-up. You returned null. If your ' +
2662+
'effect does not require clean up, return undefined (or nothing).',
2663+
]);
2664+
2665+
const root3 = ReactNoop.createRoot();
2666+
expect(() =>
2667+
act(() => {
2668+
root3.render(<App return={Promise.resolve()} />);
2669+
}),
2670+
).toErrorDev([
2671+
'Warning: An effect function must not return anything besides a ' +
2672+
'function, which is used for clean-up.\n\n' +
2673+
'It looks like you wrote useEffect(async () => ...) or returned a Promise.',
2674+
]);
2675+
2676+
// Error on unmount because React assumes the value is a function
2677+
expect(() =>
2678+
act(() => {
2679+
root3.unmount();
2680+
}),
2681+
).toThrow('is not a function');
2682+
});
26352683
});
26362684

26372685
describe('useLayoutEffect', () => {
@@ -2810,6 +2858,54 @@ describe('ReactHooksWithNoopRenderer', () => {
28102858
]);
28112859
expect(ReactNoop.getChildren()).toEqual([span('OuterFallback')]);
28122860
});
2861+
2862+
it('assumes layout effect destroy function is either a function or undefined', () => {
2863+
function App(props) {
2864+
useLayoutEffect(() => {
2865+
return props.return;
2866+
});
2867+
return null;
2868+
}
2869+
2870+
const root1 = ReactNoop.createRoot();
2871+
expect(() =>
2872+
act(() => {
2873+
root1.render(<App return={17} />);
2874+
}),
2875+
).toErrorDev([
2876+
'Warning: An effect function must not return anything besides a ' +
2877+
'function, which is used for clean-up. You returned: 17',
2878+
]);
2879+
2880+
const root2 = ReactNoop.createRoot();
2881+
expect(() =>
2882+
act(() => {
2883+
root2.render(<App return={null} />);
2884+
}),
2885+
).toErrorDev([
2886+
'Warning: An effect function must not return anything besides a ' +
2887+
'function, which is used for clean-up. You returned null. If your ' +
2888+
'effect does not require clean up, return undefined (or nothing).',
2889+
]);
2890+
2891+
const root3 = ReactNoop.createRoot();
2892+
expect(() =>
2893+
act(() => {
2894+
root3.render(<App return={Promise.resolve()} />);
2895+
}),
2896+
).toErrorDev([
2897+
'Warning: An effect function must not return anything besides a ' +
2898+
'function, which is used for clean-up.\n\n' +
2899+
'It looks like you wrote useEffect(async () => ...) or returned a Promise.',
2900+
]);
2901+
2902+
// Error on unmount because React assumes the value is a function
2903+
expect(() =>
2904+
act(() => {
2905+
root3.unmount();
2906+
}),
2907+
).toThrow('is not a function');
2908+
});
28132909
});
28142910

28152911
describe('useCallback', () => {

0 commit comments

Comments
 (0)