From 17e94fe3c9f956ade977df9cafb69b569a4ad44d Mon Sep 17 00:00:00 2001 From: eps1lon Date: Thu, 2 May 2024 21:11:03 +0200 Subject: [PATCH 1/2] Stop using Scheduler.log to test double invocations We don't do this to test double render invocations either since we ignore Scheduler.log for the second render. The plan is to do the same for double invoking effects to reduce test noise. --- .../__tests__/ReactInternalTestUtils-test.js | 6 +- .../src/__tests__/StrictEffectsMode-test.js | 235 +++++++++-------- ...StrictEffectsModeDefaults-test.internal.js | 237 ++++++++++++------ 3 files changed, 300 insertions(+), 178 deletions(-) diff --git a/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js b/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js index fa985ef9948af..be98bffe45706 100644 --- a/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js +++ b/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js @@ -161,7 +161,11 @@ describe('ReactInternalTestUtils', () => { const root = ReactNoop.createRoot(); await act(() => { - root.render(); + root.render( + + + + ); }); assertLog(['A', 'B', 'C']); }); diff --git a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js index 34ee0b1851613..646638e9ed2d6 100644 --- a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js +++ b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js @@ -11,33 +11,29 @@ let React; let ReactNoop; -let Scheduler; let act; -let assertLog; describe('StrictEffectsMode', () => { beforeEach(() => { jest.resetModules(); act = require('internal-test-utils').act; - const InternalTestUtils = require('internal-test-utils'); - assertLog = InternalTestUtils.assertLog; React = require('react'); - Scheduler = require('scheduler'); ReactNoop = require('react-noop-renderer'); }); // @gate !disableLegacyMode it('should not double invoke effects in legacy mode', async () => { + const log = []; function App({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); return text; @@ -52,19 +48,20 @@ describe('StrictEffectsMode', () => { ); }); - assertLog(['useLayoutEffect mount', 'useEffect mount']); + expect(log).toEqual(['useLayoutEffect mount', 'useEffect mount']); }); it('double invoking for effects works properly', async () => { + const log = []; function App({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); return text; @@ -80,7 +77,7 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'useLayoutEffect mount', 'useEffect mount', 'useLayoutEffect unmount', @@ -89,9 +86,10 @@ describe('StrictEffectsMode', () => { 'useEffect mount', ]); } else { - assertLog(['useLayoutEffect mount', 'useEffect mount']); + expect(log).toEqual(['useLayoutEffect mount', 'useEffect mount']); } + log.length = 0; await act(() => { ReactNoop.renderToRootWithID( @@ -101,30 +99,32 @@ describe('StrictEffectsMode', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect unmount', 'useLayoutEffect mount', 'useEffect unmount', 'useEffect mount', ]); + log.length = 0; await act(() => { ReactNoop.unmountRootWithID('root'); }); - assertLog(['useLayoutEffect unmount', 'useEffect unmount']); + expect(log).toEqual(['useLayoutEffect unmount', 'useEffect unmount']); }); it('multiple effects are double invoked in the right order (all mounted, all unmounted, all remounted)', async () => { + const log = []; function App({text}) { React.useEffect(() => { - Scheduler.log('useEffect One mount'); - return () => Scheduler.log('useEffect One unmount'); + log.push('useEffect One mount'); + return () => log.push('useEffect One unmount'); }); React.useEffect(() => { - Scheduler.log('useEffect Two mount'); - return () => Scheduler.log('useEffect Two unmount'); + log.push('useEffect Two mount'); + return () => log.push('useEffect Two unmount'); }); return text; @@ -140,7 +140,7 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'useEffect One mount', 'useEffect Two mount', 'useEffect One unmount', @@ -149,9 +149,10 @@ describe('StrictEffectsMode', () => { 'useEffect Two mount', ]); } else { - assertLog(['useEffect One mount', 'useEffect Two mount']); + expect(log).toEqual(['useEffect One mount', 'useEffect Two mount']); } + log.length = 0; await act(() => { ReactNoop.renderToRootWithID( @@ -161,30 +162,32 @@ describe('StrictEffectsMode', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useEffect One unmount', 'useEffect Two unmount', 'useEffect One mount', 'useEffect Two mount', ]); + log.length = 0; await act(() => { ReactNoop.unmountRootWithID('root'); }); - assertLog(['useEffect One unmount', 'useEffect Two unmount']); + expect(log).toEqual(['useEffect One unmount', 'useEffect Two unmount']); }); it('multiple layout effects are double invoked in the right order (all mounted, all unmounted, all remounted)', async () => { + const log = []; function App({text}) { React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect One mount'); - return () => Scheduler.log('useLayoutEffect One unmount'); + log.push('useLayoutEffect One mount'); + return () => log.push('useLayoutEffect One unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect Two mount'); - return () => Scheduler.log('useLayoutEffect Two unmount'); + log.push('useLayoutEffect Two mount'); + return () => log.push('useLayoutEffect Two unmount'); }); return text; @@ -200,7 +203,7 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'useLayoutEffect One mount', 'useLayoutEffect Two mount', 'useLayoutEffect One unmount', @@ -209,9 +212,13 @@ describe('StrictEffectsMode', () => { 'useLayoutEffect Two mount', ]); } else { - assertLog(['useLayoutEffect One mount', 'useLayoutEffect Two mount']); + expect(log).toEqual([ + 'useLayoutEffect One mount', + 'useLayoutEffect Two mount', + ]); } + log.length = 0; await act(() => { ReactNoop.renderToRootWithID( @@ -221,28 +228,33 @@ describe('StrictEffectsMode', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect One unmount', 'useLayoutEffect Two unmount', 'useLayoutEffect One mount', 'useLayoutEffect Two mount', ]); + log.length = 0; await act(() => { ReactNoop.unmountRootWithID('root'); }); - assertLog(['useLayoutEffect One unmount', 'useLayoutEffect Two unmount']); + expect(log).toEqual([ + 'useLayoutEffect One unmount', + 'useLayoutEffect Two unmount', + ]); }); it('useEffect and useLayoutEffect is called twice when there is no unmount', async () => { + const log = []; function App({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); + log.push('useEffect mount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); + log.push('useLayoutEffect mount'); }); return text; @@ -257,16 +269,17 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'useLayoutEffect mount', 'useEffect mount', 'useLayoutEffect mount', 'useEffect mount', ]); } else { - assertLog(['useLayoutEffect mount', 'useEffect mount']); + expect(log).toEqual(['useLayoutEffect mount', 'useEffect mount']); } + log.length = 0; await act(() => { ReactNoop.renderToRootWithID( @@ -275,32 +288,34 @@ describe('StrictEffectsMode', () => { ); }); - assertLog(['useLayoutEffect mount', 'useEffect mount']); + expect(log).toEqual(['useLayoutEffect mount', 'useEffect mount']); + log.length = 0; await act(() => { ReactNoop.unmountRootWithID('root'); }); - assertLog([]); + expect(log).toEqual([]); }); it('passes the right context to class component lifecycles', async () => { + const log = []; class App extends React.PureComponent { test() {} componentDidMount() { this.test(); - Scheduler.log('componentDidMount'); + log.push('componentDidMount'); } componentDidUpdate() { this.test(); - Scheduler.log('componentDidUpdate'); + log.push('componentDidUpdate'); } componentWillUnmount() { this.test(); - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -317,28 +332,29 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'componentDidMount', 'componentWillUnmount', 'componentDidMount', ]); } else { - assertLog(['componentDidMount']); + expect(log).toEqual(['componentDidMount']); } }); it('double invoking works for class components', async () => { + const log = []; class App extends React.PureComponent { componentDidMount() { - Scheduler.log('componentDidMount'); + log.push('componentDidMount'); } componentDidUpdate() { - Scheduler.log('componentDidUpdate'); + log.push('componentDidUpdate'); } componentWillUnmount() { - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -356,15 +372,16 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'componentDidMount', 'componentWillUnmount', 'componentDidMount', ]); } else { - assertLog(['componentDidMount']); + expect(log).toEqual(['componentDidMount']); } + log.length = 0; await act(() => { ReactNoop.renderToRootWithID( @@ -374,23 +391,25 @@ describe('StrictEffectsMode', () => { ); }); - assertLog(['componentDidUpdate']); + expect(log).toEqual(['componentDidUpdate']); + log.length = 0; await act(() => { ReactNoop.unmountRootWithID('root'); }); - assertLog(['componentWillUnmount']); + expect(log).toEqual(['componentWillUnmount']); }); it('invokes componentWillUnmount for class components without componentDidMount', async () => { + const log = []; class App extends React.PureComponent { componentDidUpdate() { - Scheduler.log('componentDidUpdate'); + log.push('componentDidUpdate'); } componentWillUnmount() { - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -408,11 +427,12 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog(['componentWillUnmount']); + expect(log).toEqual(['componentWillUnmount']); } else { - assertLog([]); + expect(log).toEqual([]); } + log.length = 0; await act(() => { ReactNoop.renderToRootWithID( @@ -422,28 +442,30 @@ describe('StrictEffectsMode', () => { ); }); - assertLog(['componentDidUpdate']); + expect(log).toEqual(['componentDidUpdate']); + log.length = 0; await act(() => { ReactNoop.unmountRootWithID('root'); }); - assertLog(['componentWillUnmount']); + expect(log).toEqual(['componentWillUnmount']); }); // @gate !disableLegacyMode it('should not double invoke class lifecycles in legacy mode', async () => { + const log = []; class App extends React.PureComponent { componentDidMount() { - Scheduler.log('componentDidMount'); + log.push('componentDidMount'); } componentDidUpdate() { - Scheduler.log('componentDidUpdate'); + log.push('componentDidUpdate'); } componentWillUnmount() { - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -460,26 +482,27 @@ describe('StrictEffectsMode', () => { ); }); - assertLog(['componentDidMount']); + expect(log).toEqual(['componentDidMount']); }); it('double flushing passive effects only results in one double invoke', async () => { + const log = []; function App({text}) { const [state, setState] = React.useState(0); React.useEffect(() => { if (state !== 1) { setState(1); } - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); - Scheduler.log(text); + log.push(text); return text; } @@ -492,7 +515,8 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ + 'mount', 'mount', 'useLayoutEffect mount', 'useEffect mount', @@ -501,13 +525,14 @@ describe('StrictEffectsMode', () => { 'useLayoutEffect mount', 'useEffect mount', 'mount', + 'mount', 'useLayoutEffect unmount', 'useLayoutEffect mount', 'useEffect unmount', 'useEffect mount', ]); } else { - assertLog([ + expect(log).toEqual([ 'mount', 'useLayoutEffect mount', 'useEffect mount', @@ -521,15 +546,16 @@ describe('StrictEffectsMode', () => { }); it('newly mounted components after initial mount get double invoked', async () => { + const log = []; let _setShowChild; function Child() { React.useEffect(() => { - Scheduler.log('Child useEffect mount'); - return () => Scheduler.log('Child useEffect unmount'); + log.push('Child useEffect mount'); + return () => log.push('Child useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('Child useLayoutEffect mount'); - return () => Scheduler.log('Child useLayoutEffect unmount'); + log.push('Child useLayoutEffect mount'); + return () => log.push('Child useLayoutEffect unmount'); }); return null; @@ -539,12 +565,12 @@ describe('StrictEffectsMode', () => { const [showChild, setShowChild] = React.useState(false); _setShowChild = setShowChild; React.useEffect(() => { - Scheduler.log('App useEffect mount'); - return () => Scheduler.log('App useEffect unmount'); + log.push('App useEffect mount'); + return () => log.push('App useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('App useLayoutEffect mount'); - return () => Scheduler.log('App useLayoutEffect unmount'); + log.push('App useLayoutEffect mount'); + return () => log.push('App useLayoutEffect unmount'); }); return showChild && ; @@ -560,7 +586,7 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'App useLayoutEffect mount', 'App useEffect mount', 'App useLayoutEffect unmount', @@ -569,15 +595,16 @@ describe('StrictEffectsMode', () => { 'App useEffect mount', ]); } else { - assertLog(['App useLayoutEffect mount', 'App useEffect mount']); + expect(log).toEqual(['App useLayoutEffect mount', 'App useEffect mount']); } + log.length = 0; await act(() => { _setShowChild(true); }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'App useLayoutEffect unmount', 'Child useLayoutEffect mount', 'App useLayoutEffect mount', @@ -590,7 +617,7 @@ describe('StrictEffectsMode', () => { 'Child useEffect mount', ]); } else { - assertLog([ + expect(log).toEqual([ 'App useLayoutEffect unmount', 'Child useLayoutEffect mount', 'App useLayoutEffect mount', @@ -602,13 +629,14 @@ describe('StrictEffectsMode', () => { }); it('classes and functions are double invoked together correctly', async () => { + const log = []; class ClassChild extends React.PureComponent { componentDidMount() { - Scheduler.log('componentDidMount'); + log.push('componentDidMount'); } componentWillUnmount() { - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -618,12 +646,12 @@ describe('StrictEffectsMode', () => { function FunctionChild({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); return text; } @@ -647,7 +675,7 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'componentDidMount', 'useLayoutEffect mount', 'useEffect mount', @@ -659,13 +687,14 @@ describe('StrictEffectsMode', () => { 'useEffect mount', ]); } else { - assertLog([ + expect(log).toEqual([ 'componentDidMount', 'useLayoutEffect mount', 'useEffect mount', ]); } + log.length = 0; await act(() => { ReactNoop.renderToRootWithID( @@ -675,18 +704,19 @@ describe('StrictEffectsMode', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect unmount', 'useLayoutEffect mount', 'useEffect unmount', 'useEffect mount', ]); + log.length = 0; await act(() => { ReactNoop.unmountRootWithID('root'); }); - assertLog([ + expect(log).toEqual([ 'componentWillUnmount', 'useLayoutEffect unmount', 'useEffect unmount', @@ -694,9 +724,10 @@ describe('StrictEffectsMode', () => { }); it('classes without componentDidMount and functions are double invoked together correctly', async () => { + const log = []; class ClassChild extends React.PureComponent { componentWillUnmount() { - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -706,12 +737,12 @@ describe('StrictEffectsMode', () => { function FunctionChild({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); return text; } @@ -735,7 +766,7 @@ describe('StrictEffectsMode', () => { }); if (__DEV__) { - assertLog([ + expect(log).toEqual([ 'useLayoutEffect mount', 'useEffect mount', 'componentWillUnmount', @@ -745,9 +776,10 @@ describe('StrictEffectsMode', () => { 'useEffect mount', ]); } else { - assertLog(['useLayoutEffect mount', 'useEffect mount']); + expect(log).toEqual(['useLayoutEffect mount', 'useEffect mount']); } + log.length = 0; await act(() => { ReactNoop.renderToRootWithID( @@ -757,18 +789,19 @@ describe('StrictEffectsMode', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect unmount', 'useLayoutEffect mount', 'useEffect unmount', 'useEffect mount', ]); + log.length = 0; await act(() => { ReactNoop.unmountRootWithID('root'); }); - assertLog([ + expect(log).toEqual([ 'componentWillUnmount', 'useLayoutEffect unmount', 'useEffect unmount', @@ -777,7 +810,7 @@ describe('StrictEffectsMode', () => { // @gate __DEV__ it('should double invoke effects after a re-suspend', async () => { - // Not using Scheduler.log because it silences double render logs. + // Not using log.push because it silences double render logs. let log = []; let shouldSuspend = true; let resolve; diff --git a/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js b/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js index ef0f494b687da..7d1a3b77cb543 100644 --- a/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js +++ b/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js @@ -13,7 +13,6 @@ let React; let ReactNoop; let Scheduler; let act; -let assertLog; let waitFor; let waitForAll; let waitForPaint; @@ -31,20 +30,20 @@ describe('StrictEffectsMode defaults', () => { waitFor = InternalTestUtils.waitFor; waitForAll = InternalTestUtils.waitForAll; waitForPaint = InternalTestUtils.waitForPaint; - assertLog = InternalTestUtils.assertLog; }); // @gate !disableLegacyMode it('should not double invoke effects in legacy mode', async () => { + const log = []; function App({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); return text; @@ -58,22 +57,23 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog(['useLayoutEffect mount', 'useEffect mount']); + expect(log).toEqual(['useLayoutEffect mount', 'useEffect mount']); }); // @gate !disableLegacyMode it('should not double invoke class lifecycles in legacy mode', async () => { + const log = []; class App extends React.PureComponent { componentDidMount() { - Scheduler.log('componentDidMount'); + log.push('componentDidMount'); } componentDidUpdate() { - Scheduler.log('componentDidUpdate'); + log.push('componentDidUpdate'); } componentWillUnmount() { - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -89,15 +89,20 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog(['componentDidMount']); + expect(log).toEqual(['componentDidMount']); }); if (__DEV__) { it('should flush double-invoked effects within the same frame as layout effects if there are no passive effects', async () => { + const log = []; function ComponentWithEffects({label}) { React.useLayoutEffect(() => { Scheduler.log(`useLayoutEffect mount "${label}"`); - return () => Scheduler.log(`useLayoutEffect unmount "${label}"`); + log.push(`useLayoutEffect mount "${label}"`); + return () => { + Scheduler.log(`useLayoutEffect unmount "${label}"`); + log.push(`useLayoutEffect unmount "${label}"`); + }; }); return label; @@ -115,8 +120,14 @@ describe('StrictEffectsMode defaults', () => { 'useLayoutEffect unmount "one"', 'useLayoutEffect mount "one"', ]); + expect(log).toEqual([ + 'useLayoutEffect mount "one"', + 'useLayoutEffect unmount "one"', + 'useLayoutEffect mount "one"', + ]); }); + log.length = 0; await act(async () => { ReactNoop.render( @@ -125,13 +136,23 @@ describe('StrictEffectsMode defaults', () => { , ); - assertLog([]); + expect(log).toEqual([]); await waitForPaint([ // Cleanup and re-run "one" (and "two") since there is no dependencies array. 'useLayoutEffect unmount "one"', 'useLayoutEffect mount "one"', 'useLayoutEffect mount "two"', + // Since "two" is new, it should be double-invoked. + 'useLayoutEffect unmount "two"', + 'useLayoutEffect mount "two"', + ]); + expect(log).toEqual([ + // Cleanup and re-run "one" (and "two") since there is no dependencies array. + 'useLayoutEffect unmount "one"', + 'useLayoutEffect mount "one"', + 'useLayoutEffect mount "two"', + // Since "two" is new, it should be double-invoked. 'useLayoutEffect unmount "two"', 'useLayoutEffect mount "two"', @@ -142,15 +163,24 @@ describe('StrictEffectsMode defaults', () => { // This test also verifies that double-invoked effects flush synchronously // within the same frame as passive effects. it('should double invoke effects only for newly mounted components', async () => { + const log = []; function ComponentWithEffects({label}) { React.useEffect(() => { + log.push(`useEffect mount "${label}"`); Scheduler.log(`useEffect mount "${label}"`); - return () => Scheduler.log(`useEffect unmount "${label}"`); + return () => { + log.push(`useEffect unmount "${label}"`); + Scheduler.log(`useEffect unmount "${label}"`); + }; }); React.useLayoutEffect(() => { + log.push(`useLayoutEffect mount "${label}"`); Scheduler.log(`useLayoutEffect mount "${label}"`); - return () => Scheduler.log(`useLayoutEffect unmount "${label}"`); + return () => { + log.push(`useLayoutEffect unmount "${label}"`); + Scheduler.log(`useLayoutEffect unmount "${label}"`); + }; }); return label; @@ -171,8 +201,17 @@ describe('StrictEffectsMode defaults', () => { 'useLayoutEffect mount "one"', 'useEffect mount "one"', ]); + expect(log).toEqual([ + 'useLayoutEffect mount "one"', + 'useEffect mount "one"', + 'useLayoutEffect unmount "one"', + 'useEffect unmount "one"', + 'useLayoutEffect mount "one"', + 'useEffect mount "one"', + ]); }); + log.length = 0; await act(async () => { ReactNoop.render( @@ -187,11 +226,29 @@ describe('StrictEffectsMode defaults', () => { 'useLayoutEffect mount "one"', 'useLayoutEffect mount "two"', ]); + expect(log).toEqual([ + // Cleanup and re-run "one" (and "two") since there is no dependencies array. + 'useLayoutEffect unmount "one"', + 'useLayoutEffect mount "one"', + 'useLayoutEffect mount "two"', + ]); + log.length = 0; await waitForAll([ 'useEffect unmount "one"', 'useEffect mount "one"', 'useEffect mount "two"', + // Since "two" is new, it should be double-invoked. + 'useLayoutEffect unmount "two"', + 'useEffect unmount "two"', + 'useLayoutEffect mount "two"', + 'useEffect mount "two"', + ]); + expect(log).toEqual([ + 'useEffect unmount "one"', + 'useEffect mount "one"', + 'useEffect mount "two"', + // Since "two" is new, it should be double-invoked. 'useLayoutEffect unmount "two"', 'useEffect unmount "two"', @@ -202,15 +259,16 @@ describe('StrictEffectsMode defaults', () => { }); it('double invoking for effects for modern roots', async () => { + const log = []; function App({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); return text; @@ -223,7 +281,7 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect mount', 'useEffect mount', 'useLayoutEffect unmount', @@ -232,6 +290,7 @@ describe('StrictEffectsMode defaults', () => { 'useEffect mount', ]); + log.length = 0; await act(() => { ReactNoop.render( @@ -240,30 +299,32 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect unmount', 'useLayoutEffect mount', 'useEffect unmount', 'useEffect mount', ]); + log.length = 0; await act(() => { ReactNoop.render(null); }); - assertLog(['useLayoutEffect unmount', 'useEffect unmount']); + expect(log).toEqual(['useLayoutEffect unmount', 'useEffect unmount']); }); it('multiple effects are double invoked in the right order (all mounted, all unmounted, all remounted)', async () => { + const log = []; function App({text}) { React.useEffect(() => { - Scheduler.log('useEffect One mount'); - return () => Scheduler.log('useEffect One unmount'); + log.push('useEffect One mount'); + return () => log.push('useEffect One unmount'); }); React.useEffect(() => { - Scheduler.log('useEffect Two mount'); - return () => Scheduler.log('useEffect Two unmount'); + log.push('useEffect Two mount'); + return () => log.push('useEffect Two unmount'); }); return text; @@ -277,7 +338,7 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useEffect One mount', 'useEffect Two mount', 'useEffect One unmount', @@ -286,6 +347,7 @@ describe('StrictEffectsMode defaults', () => { 'useEffect Two mount', ]); + log.length = 0; await act(() => { ReactNoop.render( @@ -294,30 +356,32 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useEffect One unmount', 'useEffect Two unmount', 'useEffect One mount', 'useEffect Two mount', ]); + log.length = 0; await act(() => { ReactNoop.render(null); }); - assertLog(['useEffect One unmount', 'useEffect Two unmount']); + expect(log).toEqual(['useEffect One unmount', 'useEffect Two unmount']); }); it('multiple layout effects are double invoked in the right order (all mounted, all unmounted, all remounted)', async () => { + const log = []; function App({text}) { React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect One mount'); - return () => Scheduler.log('useLayoutEffect One unmount'); + log.push('useLayoutEffect One mount'); + return () => log.push('useLayoutEffect One unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect Two mount'); - return () => Scheduler.log('useLayoutEffect Two unmount'); + log.push('useLayoutEffect Two mount'); + return () => log.push('useLayoutEffect Two unmount'); }); return text; @@ -331,7 +395,7 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect One mount', 'useLayoutEffect Two mount', 'useLayoutEffect One unmount', @@ -340,6 +404,7 @@ describe('StrictEffectsMode defaults', () => { 'useLayoutEffect Two mount', ]); + log.length = 0; await act(() => { ReactNoop.render( @@ -348,28 +413,33 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect One unmount', 'useLayoutEffect Two unmount', 'useLayoutEffect One mount', 'useLayoutEffect Two mount', ]); + log.length = 0; await act(() => { ReactNoop.render(null); }); - assertLog(['useLayoutEffect One unmount', 'useLayoutEffect Two unmount']); + expect(log).toEqual([ + 'useLayoutEffect One unmount', + 'useLayoutEffect Two unmount', + ]); }); it('useEffect and useLayoutEffect is called twice when there is no unmount', async () => { + const log = []; function App({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); + log.push('useEffect mount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); + log.push('useLayoutEffect mount'); }); return text; @@ -383,13 +453,14 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect mount', 'useEffect mount', 'useLayoutEffect mount', 'useEffect mount', ]); + log.length = 0; await act(() => { ReactNoop.render( @@ -398,13 +469,14 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog(['useLayoutEffect mount', 'useEffect mount']); + expect(log).toEqual(['useLayoutEffect mount', 'useEffect mount']); + log.length = 0; await act(() => { ReactNoop.render(null); }); - assertLog([]); + expect(log).toEqual([]); }); //@gate useModernStrictMode @@ -436,22 +508,23 @@ describe('StrictEffectsMode defaults', () => { }); it('passes the right context to class component lifecycles', async () => { + const log = []; class App extends React.PureComponent { test() {} componentDidMount() { this.test(); - Scheduler.log('componentDidMount'); + log.push('componentDidMount'); } componentDidUpdate() { this.test(); - Scheduler.log('componentDidUpdate'); + log.push('componentDidUpdate'); } componentWillUnmount() { this.test(); - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -467,7 +540,7 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'componentDidMount', 'componentWillUnmount', 'componentDidMount', @@ -475,17 +548,18 @@ describe('StrictEffectsMode defaults', () => { }); it('double invoking works for class components', async () => { + const log = []; class App extends React.PureComponent { componentDidMount() { - Scheduler.log('componentDidMount'); + log.push('componentDidMount'); } componentDidUpdate() { - Scheduler.log('componentDidUpdate'); + log.push('componentDidUpdate'); } componentWillUnmount() { - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -501,12 +575,13 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'componentDidMount', 'componentWillUnmount', 'componentDidMount', ]); + log.length = 0; await act(() => { ReactNoop.render( @@ -515,35 +590,38 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog(['componentDidUpdate']); + expect(log).toEqual(['componentDidUpdate']); + log.length = 0; await act(() => { ReactNoop.render(null); }); - assertLog(['componentWillUnmount']); + expect(log).toEqual(['componentWillUnmount']); }); it('double flushing passive effects only results in one double invoke', async () => { + const log = []; function App({text}) { const [state, setState] = React.useState(0); React.useEffect(() => { if (state !== 1) { setState(1); } - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); - Scheduler.log(text); + log.push(text); return text; } + log.length = 0; await act(() => { ReactNoop.render( @@ -552,7 +630,8 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ + 'mount', 'mount', 'useLayoutEffect mount', 'useEffect mount', @@ -561,6 +640,7 @@ describe('StrictEffectsMode defaults', () => { 'useLayoutEffect mount', 'useEffect mount', 'mount', + 'mount', 'useLayoutEffect unmount', 'useLayoutEffect mount', 'useEffect unmount', @@ -570,14 +650,15 @@ describe('StrictEffectsMode defaults', () => { it('newly mounted components after initial mount get double invoked', async () => { let _setShowChild; + const log = []; function Child() { React.useEffect(() => { - Scheduler.log('Child useEffect mount'); - return () => Scheduler.log('Child useEffect unmount'); + log.push('Child useEffect mount'); + return () => log.push('Child useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('Child useLayoutEffect mount'); - return () => Scheduler.log('Child useLayoutEffect unmount'); + log.push('Child useLayoutEffect mount'); + return () => log.push('Child useLayoutEffect unmount'); }); return null; @@ -587,12 +668,12 @@ describe('StrictEffectsMode defaults', () => { const [showChild, setShowChild] = React.useState(false); _setShowChild = setShowChild; React.useEffect(() => { - Scheduler.log('App useEffect mount'); - return () => Scheduler.log('App useEffect unmount'); + log.push('App useEffect mount'); + return () => log.push('App useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('App useLayoutEffect mount'); - return () => Scheduler.log('App useLayoutEffect unmount'); + log.push('App useLayoutEffect mount'); + return () => log.push('App useLayoutEffect unmount'); }); return showChild && ; @@ -606,7 +687,7 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'App useLayoutEffect mount', 'App useEffect mount', 'App useLayoutEffect unmount', @@ -615,11 +696,12 @@ describe('StrictEffectsMode defaults', () => { 'App useEffect mount', ]); + log.length = 0; await act(() => { _setShowChild(true); }); - assertLog([ + expect(log).toEqual([ 'App useLayoutEffect unmount', 'Child useLayoutEffect mount', 'App useLayoutEffect mount', @@ -634,13 +716,14 @@ describe('StrictEffectsMode defaults', () => { }); it('classes and functions are double invoked together correctly', async () => { + const log = []; class ClassChild extends React.PureComponent { componentDidMount() { - Scheduler.log('componentDidMount'); + log.push('componentDidMount'); } componentWillUnmount() { - Scheduler.log('componentWillUnmount'); + log.push('componentWillUnmount'); } render() { @@ -650,12 +733,12 @@ describe('StrictEffectsMode defaults', () => { function FunctionChild({text}) { React.useEffect(() => { - Scheduler.log('useEffect mount'); - return () => Scheduler.log('useEffect unmount'); + log.push('useEffect mount'); + return () => log.push('useEffect unmount'); }); React.useLayoutEffect(() => { - Scheduler.log('useLayoutEffect mount'); - return () => Scheduler.log('useLayoutEffect unmount'); + log.push('useLayoutEffect mount'); + return () => log.push('useLayoutEffect unmount'); }); return text; } @@ -677,7 +760,7 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'componentDidMount', 'useLayoutEffect mount', 'useEffect mount', @@ -689,6 +772,7 @@ describe('StrictEffectsMode defaults', () => { 'useEffect mount', ]); + log.length = 0; await act(() => { ReactNoop.render( @@ -697,18 +781,19 @@ describe('StrictEffectsMode defaults', () => { ); }); - assertLog([ + expect(log).toEqual([ 'useLayoutEffect unmount', 'useLayoutEffect mount', 'useEffect unmount', 'useEffect mount', ]); + log.length = 0; await act(() => { ReactNoop.render(null); }); - assertLog([ + expect(log).toEqual([ 'componentWillUnmount', 'useLayoutEffect unmount', 'useEffect unmount', From 686bed8c8d5d86d856541626c5d848429d7bde31 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Mon, 6 May 2024 23:04:09 +0200 Subject: [PATCH 2/2] Keep log assertions small The longer the list of log lines asserted one, the harder future diffs are to comprehend. git-diff produces hard to decipher diffs if the StrictEffects behavior is changed slightly that results in more lines added. It won't be obvious anymore which lines were added and which ones were removed. --- .../src/__tests__/StrictEffectsMode-test.js | 57 +++++++------------ 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js index 646638e9ed2d6..bd458b2cd5328 100644 --- a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js +++ b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js @@ -900,8 +900,17 @@ describe('StrictEffectsMode', () => { ); }); + expect(log).toEqual([ + 'Parent rendered', + 'Parent rendered', + 'Child rendered', + 'Child suspended', + 'Fallback', + 'Fallback', + ]); + + log = []; // while suspended, update - log.push('-----------------------after update'); await act(() => { ReactNoop.render( @@ -910,9 +919,19 @@ describe('StrictEffectsMode', () => { ); }); - // Now resolve and commit - log.push('-----------------------after suspense'); + expect(log).toEqual([ + 'Parent rendered', + 'Parent rendered', + 'Child rendered', + 'Child suspended', + 'Fallback', + 'Fallback', + 'Parent dep destroy', + 'Parent dep create', + ]); + log = []; + // Now resolve and commit await act(() => { resolve(); shouldSuspend = false; @@ -920,22 +939,6 @@ describe('StrictEffectsMode', () => { if (gate(flags => flags.useModernStrictMode)) { expect(log).toEqual([ - 'Parent rendered', - 'Parent rendered', - 'Child rendered', - 'Child suspended', - 'Fallback', - 'Fallback', - '-----------------------after update', - 'Parent rendered', - 'Parent rendered', - 'Child rendered', - 'Child suspended', - 'Fallback', - 'Fallback', - 'Parent dep destroy', - 'Parent dep create', - '-----------------------after suspense', 'Child rendered', 'Child rendered', // !!! Committed, destroy and create effect. @@ -957,22 +960,6 @@ describe('StrictEffectsMode', () => { ]); } else { expect(log).toEqual([ - 'Parent rendered', - 'Parent rendered', - 'Child rendered', - 'Child suspended', - 'Fallback', - 'Fallback', - '-----------------------after update', - 'Parent rendered', - 'Parent rendered', - 'Child rendered', - 'Child suspended', - 'Fallback', - 'Fallback', - 'Parent dep destroy', - 'Parent dep create', - '-----------------------after suspense', 'Child rendered', 'Child rendered', 'Child dep destroy',