From cccaa04836f81ff2a572d409c1d465e2e1483acc Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 29 May 2019 17:58:58 +0100 Subject: [PATCH] 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. --- .../ReactHooksInspectionIntegration-test.js | 5 +- .../src/__tests__/ReactDOMHooks-test.js | 46 +- ...DOMServerIntegrationHooks-test.internal.js | 12 +- .../ReactErrorBoundaries-test.internal.js | 41 +- .../src/__tests__/ReactUpdates-test.js | 85 +- .../react-reconciler/src/ReactFiberHooks.js | 17 + .../src/ReactFiberWorkLoop.js | 23 + .../src/__tests__/ReactHooks-test.internal.js | 79 +- ...eactHooksWithNoopRenderer-test.internal.js | 782 ++++++++++-------- ...eactIncrementalScheduling-test.internal.js | 38 +- .../ReactIncrementalUpdates-test.internal.js | 305 +++---- ...ReactSchedulerIntegration-test.internal.js | 25 +- .../src/__tests__/ReactFresh-test.js | 118 +-- .../ReactDOMTracing-test.internal.js | 148 ++-- .../src/__tests__/withComponentStack-test.js | 7 +- 15 files changed, 968 insertions(+), 763 deletions(-) diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js index 5c092f5853288..6ab8fe74a9cf0 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js @@ -146,7 +146,10 @@ describe('ReactHooksInspectionIntegration', () => { ); } - let renderer = ReactTestRenderer.create(); + let renderer; + act(() => { + renderer = ReactTestRenderer.create(); + }); let childFiber = renderer.root.findByType(Foo)._currentFiber(); diff --git a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js index 4f24770546176..c703d5e86a1c2 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js @@ -11,6 +11,7 @@ let React; let ReactDOM; +let act; let Scheduler; describe('ReactDOMHooks', () => { @@ -21,6 +22,7 @@ describe('ReactDOMHooks', () => { React = require('react'); ReactDOM = require('react-dom'); + act = require('react-dom/test-utils').act; Scheduler = require('scheduler'); container = document.createElement('div'); @@ -53,23 +55,33 @@ describe('ReactDOMHooks', () => { return 3 * n; } - ReactDOM.render(, 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(, 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 + // todo - is this ok? + expect(() => { + ReactDOM.render(, 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(, 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([ + 'Your test just caused an effect from Example1', + 'Your test just caused an effect from Example2', + 'Your test just caused an effect from Example1', + 'Your test just caused an effect from Example2', + ]); }); it('should not bail out when an update is scheduled from within an event handler', () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index a0a91763a82cf..97d304c0647dd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -17,6 +17,7 @@ let React; let ReactFeatureFlags; let ReactDOM; let ReactDOMServer; +let act; let useState; let useReducer; let useEffect; @@ -41,6 +42,7 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + act = require('react-dom/test-utils').act; useState = React.useState; useReducer = React.useReducer; useEffect = React.useEffect; @@ -546,10 +548,12 @@ describe('ReactDOMServerHooks', () => { }); return ; } - const domNode = await render(); - expect(clearYields()).toEqual(['Count: 0']); - expect(domNode.tagName).toEqual('SPAN'); - expect(domNode.textContent).toEqual('Count: 0'); + await act(async () => { + const domNode = await render(); + expect(clearYields()).toEqual(['Count: 0']); + expect(domNode.tagName).toEqual('SPAN'); + expect(domNode.textContent).toEqual('Count: 0'); + }); }); }); diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 3b19a87ecbf87..81c333e4b44f5 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -12,6 +12,7 @@ let PropTypes; let React; let ReactDOM; +let act; let ReactFeatureFlags; let Scheduler; @@ -45,6 +46,7 @@ describe('ReactErrorBoundaries', () => { ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; ReactDOM = require('react-dom'); React = require('react'); + act = require('react-dom/test-utils').act; Scheduler = require('scheduler'); log = []; @@ -1835,25 +1837,26 @@ describe('ReactErrorBoundaries', () => { it('catches errors in useEffect', () => { const container = document.createElement('div'); - ReactDOM.render( - - Initial value - , - container, - ); - expect(log).toEqual([ - 'ErrorBoundary constructor', - 'ErrorBoundary componentWillMount', - 'ErrorBoundary render success', - 'BrokenUseEffect render', - 'ErrorBoundary componentDidMount', - ]); - - expect(container.firstChild.textContent).toBe('Initial value'); - log.length = 0; - - // Flush passive effects and handle the error - Scheduler.flushAll(); + act(() => { + ReactDOM.render( + + Initial value + , + container, + ); + expect(log).toEqual([ + 'ErrorBoundary constructor', + 'ErrorBoundary componentWillMount', + 'ErrorBoundary render success', + 'BrokenUseEffect render', + 'ErrorBoundary componentDidMount', + ]); + + expect(container.firstChild.textContent).toBe('Initial value'); + log.length = 0; + }); + + // verify flushed passive effects and handle the error expect(log).toEqual([ 'BrokenUseEffect useEffect [!]', // Handle the error diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index 0b6a4b2792f3f..4eea6a6b3d746 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -12,6 +12,7 @@ let React; let ReactDOM; let ReactTestUtils; +let act; let Scheduler; describe('ReactUpdates', () => { @@ -20,6 +21,7 @@ describe('ReactUpdates', () => { React = require('react'); ReactDOM = require('react-dom'); ReactTestUtils = require('react-dom/test-utils'); + act = ReactTestUtils.act; Scheduler = require('scheduler'); }); @@ -1322,30 +1324,31 @@ describe('ReactUpdates', () => { } const root = ReactDOM.unstable_createRoot(container); - root.render(); - if (__DEV__) { - expect(Scheduler).toFlushAndYieldThrough([ - 'Foo', - 'Foo', - 'Baz', - 'Foo#effect', - ]); - } else { - expect(Scheduler).toFlushAndYieldThrough(['Foo', 'Baz', 'Foo#effect']); - } - - const hiddenDiv = container.firstChild.firstChild; - expect(hiddenDiv.hidden).toBe(true); - expect(hiddenDiv.innerHTML).toBe(''); - - // Run offscreen update - if (__DEV__) { - expect(Scheduler).toFlushAndYield(['Bar', 'Bar']); - } else { - expect(Scheduler).toFlushAndYield(['Bar']); - } - expect(hiddenDiv.hidden).toBe(true); - expect(hiddenDiv.innerHTML).toBe('

bar 0

'); + let hiddenDiv; + act(() => { + root.render(); + if (__DEV__) { + expect(Scheduler).toFlushAndYieldThrough([ + 'Foo', + 'Foo', + 'Baz', + 'Foo#effect', + ]); + } else { + expect(Scheduler).toFlushAndYieldThrough(['Foo', 'Baz', 'Foo#effect']); + } + hiddenDiv = container.firstChild.firstChild; + expect(hiddenDiv.hidden).toBe(true); + expect(hiddenDiv.innerHTML).toBe(''); + // Run offscreen update + if (__DEV__) { + expect(Scheduler).toFlushAndYield(['Bar', 'Bar']); + } else { + expect(Scheduler).toFlushAndYield(['Bar']); + } + expect(hiddenDiv.hidden).toBe(true); + expect(hiddenDiv.innerHTML).toBe('

bar 0

'); + }); ReactDOM.flushSync(() => { setCounter(1); @@ -1618,8 +1621,11 @@ describe('ReactUpdates', () => { let stack = null; let originalConsoleError = console.error; console.error = (e, s) => { - error = e; - stack = s; + // skip the missing act() error + if (e.slice(0, 40) !== 'Warning: Your test just caused an effect') { + error = e; + stack = s; + } }; try { const container = document.createElement('div'); @@ -1651,7 +1657,9 @@ describe('ReactUpdates', () => { } const container = document.createElement('div'); - ReactDOM.render(, container); + act(() => { + ReactDOM.render(, container); + }); // Verify we can flush them asynchronously without warning for (let i = 0; i < LIMIT * 2; i++) { @@ -1660,16 +1668,16 @@ describe('ReactUpdates', () => { expect(container.textContent).toBe('50'); // Verify restarting from 0 doesn't cross the limit - expect(() => { + act(() => { _setStep(0); - }).toWarnDev( - 'An update to Terminating inside a test was not wrapped in act', - ); - expect(container.textContent).toBe('0'); - for (let i = 0; i < LIMIT * 2; i++) { + // flush once to update the dom Scheduler.unstable_flushNumberOfYields(1); - } - expect(container.textContent).toBe('50'); + expect(container.textContent).toBe('0'); + for (let i = 0; i < LIMIT * 2; i++) { + Scheduler.unstable_flushNumberOfYields(1); + } + expect(container.textContent).toBe('50'); + }); }); it('can have many updates inside useEffect without triggering a warning', () => { @@ -1685,8 +1693,11 @@ describe('ReactUpdates', () => { } const container = document.createElement('div'); - ReactDOM.render(, container); - expect(Scheduler).toFlushAndYield(['Done']); + act(() => { + ReactDOM.render(, container); + }); + + expect(Scheduler).toHaveYielded(['Done']); expect(container.textContent).toBe('1000'); }); } diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 2946c931cdc58..2aef8e5d386fc 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -34,6 +34,7 @@ import { computeExpirationForFiber, flushPassiveEffects, requestCurrentTime, + warnIfNotCurrentlyActingEffectsInDEV, warnIfNotCurrentlyActingUpdatesInDev, warnIfNotScopedWithMatchingAct, markRenderEventTimeAndConfig, @@ -892,6 +893,14 @@ function mountEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { + if (__DEV__) { + // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests + if ('undefined' !== typeof jest) { + warnIfNotCurrentlyActingEffectsInDEV( + ((currentlyRenderingFiber: any): Fiber), + ); + } + } return mountEffectImpl( UpdateEffect | PassiveEffect, UnmountPassive | MountPassive, @@ -904,6 +913,14 @@ function updateEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { + if (__DEV__) { + // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests + if ('undefined' !== typeof jest) { + warnIfNotCurrentlyActingEffectsInDEV( + ((currentlyRenderingFiber: any): Fiber), + ); + } + } return updateEffectImpl( UpdateEffect | PassiveEffect, UnmountPassive | MountPassive, diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index aa865be4382ed..479471bd4d8ae 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2429,6 +2429,29 @@ export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void { } } +export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void { + if (__DEV__) { + if (ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil) { + warningWithoutStack( + false, + 'Your test just caused an effect from %s, but was not wrapped in act(...).\n\n' + + 'When testing, code that causes React state updates should be ' + + 'wrapped into act(...):\n\n' + + 'act(() => {\n' + + ' /* fire events that update state */\n' + + '});\n' + + '/* assert on the output */\n\n' + + "This ensures that you're testing the behavior the user would see " + + 'in the browser.' + + ' Learn more at https://fb.me/react-wrap-tests-with-act' + + '%s', + getComponentName(fiber.type), + getStackByFiberInDevAndProd(fiber), + ); + } + } +} + function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void { if (__DEV__) { if ( diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 74152cbcc6c31..922aa8c89110a 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -652,7 +652,9 @@ describe('ReactHooks', () => { } expect(() => { - ReactTestRenderer.create(); + act(() => { + ReactTestRenderer.create(); + }); }).toWarnDev([ 'Warning: useEffect received a final argument that is not an array (instead, received `string`). ' + 'When specified, the final argument must be an array.', @@ -664,7 +666,9 @@ describe('ReactHooks', () => { 'When specified, the final argument must be an array.', ]); expect(() => { - ReactTestRenderer.create(); + act(() => { + ReactTestRenderer.create(); + }); }).toWarnDev([ 'Warning: useEffect received a final argument that is not an array (instead, received `number`). ' + 'When specified, the final argument must be an array.', @@ -676,7 +680,9 @@ describe('ReactHooks', () => { 'When specified, the final argument must be an array.', ]); expect(() => { - ReactTestRenderer.create(); + act(() => { + ReactTestRenderer.create(); + }); }).toWarnDev([ 'Warning: useEffect received a final argument that is not an array (instead, received `object`). ' + 'When specified, the final argument must be an array.', @@ -687,9 +693,12 @@ describe('ReactHooks', () => { 'Warning: useCallback received a final argument that is not an array (instead, received `object`). ' + 'When specified, the final argument must be an array.', ]); - ReactTestRenderer.create(); - ReactTestRenderer.create(); - ReactTestRenderer.create(); + + act(() => { + ReactTestRenderer.create(); + ReactTestRenderer.create(); + ReactTestRenderer.create(); + }); }); it('warns if deps is not an array for useImperativeHandle', () => { @@ -980,8 +989,11 @@ describe('ReactHooks', () => { return null; } - const root = ReactTestRenderer.create(); - expect(() => root.update()).toThrow( + expect(() => { + act(() => { + ReactTestRenderer.create(); + }); + }).toThrow( // The exact message doesn't matter, just make sure we don't allow this 'Context can only be read while React is rendering', ); @@ -1173,7 +1185,9 @@ describe('ReactHooks', () => { } // Verify it doesn't think we're still inside a Hook. // Should have no warnings. - ReactTestRenderer.create(); + act(() => { + ReactTestRenderer.create(); + }); // Verify warnings don't get permanently disabled. expect(() => { @@ -1499,10 +1513,15 @@ describe('ReactHooks', () => { return null; /* eslint-enable no-unused-vars */ } - let root = ReactTestRenderer.create(); + let root; + act(() => { + root = ReactTestRenderer.create(); + }); expect(() => { try { - root.update(); + act(() => { + root.update(); + }); } catch (error) { // Swapping certain types of hooks will cause runtime errors. // This is okay as far as this test is concerned. @@ -1521,7 +1540,9 @@ describe('ReactHooks', () => { // further warnings for this component are silenced try { - root.update(); + act(() => { + root.update(); + }); } catch (error) { // Swapping certain types of hooks will cause runtime errors. // This is okay as far as this test is concerned. @@ -1542,10 +1563,16 @@ describe('ReactHooks', () => { return null; /* eslint-enable no-unused-vars */ } - let root = ReactTestRenderer.create(); + let root; + act(() => { + root = ReactTestRenderer.create(); + }); + expect(() => { try { - root.update(); + act(() => { + root.update(); + }); } catch (error) { // Swapping certain types of hooks will cause runtime errors. // This is okay as far as this test is concerned. @@ -1604,9 +1631,15 @@ describe('ReactHooks', () => { return null; /* eslint-enable no-unused-vars */ } - let root = ReactTestRenderer.create(); + let root; + act(() => { + root = ReactTestRenderer.create(); + }); + expect(() => { - root.update(); + act(() => { + root.update(); + }); }).toThrow('Rendered fewer hooks than expected.'); }); }); @@ -1767,6 +1800,7 @@ describe('ReactHooks', () => { globalListener(); globalListener(); }).toWarnDev([ + 'Your test just caused an effect from C', '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. @@ -1908,11 +1942,14 @@ describe('ReactHooks', () => { return 'Throw!'; } - const root = ReactTestRenderer.create( - - - , - ); + let root; + act(() => { + root = ReactTestRenderer.create( + + + , + ); + }); expect(root).toMatchRenderedOutput('Throw!'); act(() => setShouldThrow(true)); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index 496cd6d1f7579..049111e17095c 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -622,21 +622,25 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - // Effects are deferred until after the commit - expect(Scheduler).toFlushAndYield(['Passive effect [0]']); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + // Effects are deferred until after the commit + expect(Scheduler).toFlushAndYield(['Passive effect [0]']); + }); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - // Effects are deferred until after the commit - expect(Scheduler).toFlushAndYield(['Passive effect [1]']); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + // Effects are deferred until after the commit + expect(Scheduler).toFlushAndYield(['Passive effect [1]']); + }); }); it('flushes passive effects even with sibling deletions', () => { @@ -653,25 +657,24 @@ describe('ReactHooksWithNoopRenderer', () => { return ; } let passive = ; - ReactNoop.render([, passive]); - expect(Scheduler).toFlushAndYieldThrough([ - 'Layout', - 'Passive', - 'Layout effect', - ]); - expect(ReactNoop.getChildren()).toEqual([ - span('Layout'), - span('Passive'), - ]); - - // Destroying the first child shouldn't prevent the passive effect from - // being executed - ReactNoop.render([passive]); - expect(Scheduler).toFlushAndYield(['Passive effect']); - expect(ReactNoop.getChildren()).toEqual([span('Passive')]); - - // (No effects are left to flush.) - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render([, passive]); + expect(Scheduler).toFlushAndYieldThrough([ + 'Layout', + 'Passive', + 'Layout effect', + ]); + expect(ReactNoop.getChildren()).toEqual([ + span('Layout'), + span('Passive'), + ]); + // Destroying the first child shouldn't prevent the passive effect from + // being executed + ReactNoop.render([passive]); + expect(Scheduler).toFlushAndYield(['Passive effect']); + expect(ReactNoop.getChildren()).toEqual([span('Passive')]); + }); + // exiting act calls flushePassiveEffects(), but there none are left with flush. expect(Scheduler).toHaveYielded([]); }); @@ -728,18 +731,20 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render([, ]); - expect(Scheduler).toFlushAndYield([ - 'Passive', - 'Layout', - 'Layout effect', - 'Passive effect', - 'New Root', - ]); - expect(ReactNoop.getChildren()).toEqual([ - span('Passive'), - span('Layout'), - ]); + act(() => { + ReactNoop.render([, ]); + expect(Scheduler).toFlushAndYield([ + 'Passive', + 'Layout', + 'Layout effect', + 'Passive effect', + 'New Root', + ]); + expect(ReactNoop.getChildren()).toEqual([ + span('Passive'), + span('Layout'), + ]); + }); }); it( @@ -762,25 +767,25 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough([0, 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span(0)]); - - // Before the effects have a chance to flush, schedule another update - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough([ - // The previous effect flushes before the reconciliation - 'Committed state when effect was fired: 0', - 1, - 'Sync effect', - ]); - expect(ReactNoop.getChildren()).toEqual([span(1)]); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough([0, 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span(0)]); + // Before the effects have a chance to flush, schedule another update + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough([ + // The previous effect flushes before the reconciliation + 'Committed state when effect was fired: 0', + 1, + 'Sync effect', + ]); + expect(ReactNoop.getChildren()).toEqual([span(1)]); + }); - ReactNoop.flushPassiveEffects(); expect(Scheduler).toHaveYielded([ 'Committed state when effect was fired: 1', ]); @@ -799,26 +804,30 @@ describe('ReactHooksWithNoopRenderer', () => { ); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough([ - 'Count: (empty)', - 'Sync effect', - ]); - expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); - ReactNoop.flushPassiveEffects(); - expect(Scheduler).toHaveYielded(['Schedule update [0]']); - expect(Scheduler).toFlushAndYield(['Count: 0']); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough([ + 'Count: (empty)', + 'Sync effect', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); + ReactNoop.flushPassiveEffects(); + expect(Scheduler).toHaveYielded(['Schedule update [0]']); + expect(Scheduler).toFlushAndYield(['Count: 0']); + }); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); - expect(Scheduler).toHaveYielded(['Schedule update [1]']); - expect(Scheduler).toFlushAndYield(['Count: 1']); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + ReactNoop.flushPassiveEffects(); + expect(Scheduler).toHaveYielded(['Schedule update [1]']); + expect(Scheduler).toFlushAndYield(['Count: 1']); + }); }); it('updates have async priority even if effects are flushed early', () => { @@ -833,32 +842,33 @@ describe('ReactHooksWithNoopRenderer', () => { ); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough([ - 'Count: (empty)', - 'Sync effect', - ]); - expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); - - // Rendering again should flush the previous commit's effects - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough([ - 'Schedule update [0]', - 'Count: 0', - ]); - expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough([ + 'Count: (empty)', + 'Sync effect', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); - expect(Scheduler).toFlushAndYieldThrough(['Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + // Rendering again should flush the previous commit's effects + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough([ + 'Schedule update [0]', + 'Count: 0', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); - ReactNoop.flushPassiveEffects(); - expect(Scheduler).toHaveYielded(['Schedule update [1]']); - expect(Scheduler).toFlushAndYield(['Count: 1']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + expect(Scheduler).toFlushAndYieldThrough(['Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + ReactNoop.flushPassiveEffects(); + expect(Scheduler).toHaveYielded(['Schedule update [1]']); + expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + }); }); it('flushes passive effects when flushing discrete updates', () => { @@ -873,15 +883,19 @@ describe('ReactHooksWithNoopRenderer', () => { return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + // we explicitly wait for missing act() warnings here since + // it's a lot harder to simulate this condition inside an act scope + // todo - is this ok? + expect(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + }).toWarnDev(['Your test just caused an effect from Counter']); // A discrete event forces the passive effect to be flushed -- // updateCount(1) happens first, so 2 wins. - ReactNoop.flushDiscreteUpdates(); ReactNoop.discreteUpdates(() => { // (use batchedUpdates to silence the act() warning) @@ -890,7 +904,13 @@ describe('ReactHooksWithNoopRenderer', () => { }); }); expect(Scheduler).toHaveYielded(['Will set count to 1']); - expect(Scheduler).toFlushAndYield(['Count: 2']); + expect(() => { + expect(Scheduler).toFlushAndYield(['Count: 2']); + }).toWarnDev([ + 'Your test just caused an effect from Counter', + 'Your test just caused an effect from Counter', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]); }); @@ -921,17 +941,22 @@ describe('ReactHooksWithNoopRenderer', () => { } const tracingEvent = {id: 0, name: 'hello', timestamp: 0}; - SchedulerTracing.unstable_trace( - tracingEvent.name, - tracingEvent.timestamp, - () => { - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - }, - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + // we explicitly wait for missing act() warnings here since + // it's a lot harder to simulate this condition inside an act scope + // todo - is this ok? + expect(() => { + SchedulerTracing.unstable_trace( + tracingEvent.name, + tracingEvent.timestamp, + () => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + }, + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + }).toWarnDev(['Your test just caused an effect from Counter']); expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(0); @@ -939,11 +964,19 @@ describe('ReactHooksWithNoopRenderer', () => { // updateCount(1) happens first, so 2 wins. ReactNoop.flushDiscreteUpdates(); ReactNoop.discreteUpdates(() => { - // use batchedUpdates to silence the act warning - ReactNoop.batchedUpdates(() => _updateCount(2)); + // (use batchedUpdates to silence the act() warning) + ReactNoop.batchedUpdates(() => { + _updateCount(2); + }); }); expect(Scheduler).toHaveYielded(['Will set count to 1']); - expect(Scheduler).toFlushAndYield(['Count: 2']); + expect(() => { + expect(Scheduler).toFlushAndYield(['Count: 2']); + }).toWarnDev([ + 'Your test just caused an effect from Counter', + 'Your test just caused an effect from Counter', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]); expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1); @@ -971,12 +1004,14 @@ describe('ReactHooksWithNoopRenderer', () => { ); return ; } - ReactNoop.renderLegacySyncRoot(); - // Even in sync mode, effects are deferred until after paint - expect(Scheduler).toHaveYielded(['Count: (empty)']); - expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); - // Now fire the effects - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.renderLegacySyncRoot(); + // Even in sync mode, effects are deferred until after paint + expect(Scheduler).toFlushAndYieldThrough(['Count: (empty)']); + expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); + }); + + // effects get fored on exiting act() // There were multiple updates, but there should only be a // single render expect(Scheduler).toHaveYielded(['Count: 0']); @@ -998,18 +1033,19 @@ describe('ReactHooksWithNoopRenderer', () => { ); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough([ - 'Count: (empty)', - 'Sync effect', - ]); - expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); - - expect(() => { - ReactNoop.flushPassiveEffects(); - }).toThrow('flushSync was called from inside a lifecycle method'); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough([ + 'Count: (empty)', + 'Sync effect', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); + expect(() => { + ReactNoop.flushPassiveEffects(); + }).toThrow('flushSync was called from inside a lifecycle method'); + }); }); it('unmounts previous effect', () => { @@ -1022,20 +1058,24 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + }); + expect(Scheduler).toHaveYielded(['Did create [0]']); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + }); + expect(Scheduler).toHaveYielded(['Did destroy [0]', 'Did create [1]']); }); @@ -1049,12 +1089,14 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + }); + expect(Scheduler).toHaveYielded(['Did create [0]']); ReactNoop.render(null); @@ -1072,20 +1114,24 @@ describe('ReactHooksWithNoopRenderer', () => { }, []); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + }); + expect(Scheduler).toHaveYielded(['Did create [0]']); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + }); + expect(Scheduler).toHaveYielded([]); ReactNoop.render(null); @@ -1104,20 +1150,24 @@ describe('ReactHooksWithNoopRenderer', () => { useEffect(effect); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + }); + expect(Scheduler).toHaveYielded(['Did create']); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + }); + expect(Scheduler).toHaveYielded(['Did destroy', 'Did create']); ReactNoop.render(null); @@ -1139,42 +1189,50 @@ describe('ReactHooksWithNoopRenderer', () => { ); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + }); + expect(Scheduler).toHaveYielded(['Did create [Count: 0]']); expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - // Count changed - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + // Count changed + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + }); + expect(Scheduler).toHaveYielded([ 'Did destroy [Count: 0]', 'Did create [Count: 1]', ]); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - // Nothing changed, so no effect should have fired - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + // Nothing changed, so no effect should have fired + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + }); + expect(Scheduler).toHaveYielded([]); expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - // Label changed - expect(Scheduler).toFlushAndYieldThrough(['Total: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Total: 1')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + // Label changed + expect(Scheduler).toFlushAndYieldThrough(['Total: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Total: 1')]); + }); + expect(Scheduler).toHaveYielded([ 'Did destroy [Count: 1]', 'Did create [Total: 1]', @@ -1191,20 +1249,23 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + }); + expect(Scheduler).toHaveYielded(['Did commit 1 [0]', 'Did commit 2 [0]']); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + }); expect(Scheduler).toHaveYielded(['Did commit 1 [1]', 'Did commit 2 [1]']); }); @@ -1224,20 +1285,23 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + }); + expect(Scheduler).toHaveYielded(['Mount A [0]', 'Mount B [0]']); - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + }); expect(Scheduler).toHaveYielded([ 'Unmount A [0]', 'Unmount B [0]', @@ -1265,12 +1329,15 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops'); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops'); + }); + expect(Scheduler).toHaveYielded([ 'Mount A [0]', 'Oops!', @@ -1301,31 +1368,35 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); - expect(Scheduler).toHaveYielded(['Mount A [0]', 'Mount B [0]']); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + ReactNoop.flushPassiveEffects(); + expect(Scheduler).toHaveYielded(['Mount A [0]', 'Mount B [0]']); + }); - // This update will trigger an errror - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops'); - expect(Scheduler).toHaveYielded([ - 'Unmount A [0]', - 'Unmount B [0]', - 'Mount A [1]', - 'Oops!', - // Clean up effect A. There's no effect B to clean-up, because it - // never mounted. - 'Unmount A [1]', - ]); - expect(ReactNoop.getChildren()).toEqual([]); + act(() => { + // This update will trigger an errror + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops'); + expect(Scheduler).toHaveYielded([ + 'Unmount A [0]', + 'Unmount B [0]', + 'Mount A [1]', + 'Oops!', + // Clean up effect A. There's no effect B to clean-up, because it + // never mounted. + 'Unmount A [1]', + ]); + expect(ReactNoop.getChildren()).toEqual([]); + }); }); it('handles errors on unmount', () => { @@ -1347,27 +1418,31 @@ describe('ReactHooksWithNoopRenderer', () => { }); return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - ReactNoop.flushPassiveEffects(); - expect(Scheduler).toHaveYielded(['Mount A [0]', 'Mount B [0]']); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + ReactNoop.flushPassiveEffects(); + expect(Scheduler).toHaveYielded(['Mount A [0]', 'Mount B [0]']); + }); - // This update will trigger an errror - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); - expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops'); - expect(Scheduler).toHaveYielded([ - 'Oops!', - // B unmounts even though an error was thrown in the previous effect - 'Unmount B [0]', - ]); - expect(ReactNoop.getChildren()).toEqual([]); + act(() => { + // This update will trigger an errror + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 1', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); + expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops'); + expect(Scheduler).toHaveYielded([ + 'Oops!', + // B unmounts even though an error was thrown in the previous effect + 'Unmount B [0]', + ]); + expect(ReactNoop.getChildren()).toEqual([]); + }); }); it('works with memo', () => { @@ -1470,27 +1545,27 @@ describe('ReactHooksWithNoopRenderer', () => { return null; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough([ - 'Mount layout [current: 0]', - 'Sync effect', - ]); - expect(committedText).toEqual('0'); - - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough([ - 'Mount normal [current: 0]', - 'Unmount layout [current: 0]', - 'Mount layout [current: 1]', - 'Sync effect', - ]); - expect(committedText).toEqual('1'); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough([ + 'Mount layout [current: 0]', + 'Sync effect', + ]); + expect(committedText).toEqual('0'); + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough([ + 'Mount normal [current: 0]', + 'Unmount layout [current: 0]', + 'Mount layout [current: 1]', + 'Sync effect', + ]); + expect(committedText).toEqual('1'); + }); - ReactNoop.flushPassiveEffects(); expect(Scheduler).toHaveYielded([ 'Unmount normal [current: 1]', 'Mount normal [current: 1]', @@ -1684,8 +1759,10 @@ describe('ReactHooksWithNoopRenderer', () => { return null; } - ReactNoop.render(); - expect(Scheduler).toFlushAndYield([]); + act(() => { + ReactNoop.render(); + }); + expect(Scheduler).toHaveYielded([]); ping(1); ping(2); @@ -1963,28 +2040,33 @@ describe('ReactHooksWithNoopRenderer', () => { return null; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Sync effect']); - ReactNoop.flushPassiveEffects(); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Sync effect']); + }); + + // ReactNoop.flushPassiveEffects(); expect(Scheduler).toHaveYielded(['Mount A']); - ReactNoop.render(); - expect(() => { + act(() => { + ReactNoop.render(); expect(() => { - expect(Scheduler).toFlushAndYield([]); - }).toThrow('Rendered more hooks than during the previous render'); - }).toWarnDev([ - 'Warning: React has detected a change in the order of Hooks called by App. ' + - 'This will lead to bugs and errors if not fixed. For more information, ' + - 'read the Rules of Hooks: https://fb.me/rules-of-hooks\n\n' + - ' Previous render Next render\n' + - ' ------------------------------------------------------\n' + - '1. useEffect useEffect\n' + - '2. undefined useEffect\n' + - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', - ]); + expect(() => { + expect(Scheduler).toFlushAndYield([]); + }).toThrow('Rendered more hooks than during the previous render'); + }).toWarnDev([ + 'Warning: React has detected a change in the order of Hooks called by App. ' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the Rules of Hooks: https://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. useEffect useEffect\n' + + '2. undefined useEffect\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); + }); // Uncomment if/when we support this again // ReactNoop.flushPassiveEffects(); @@ -2026,15 +2108,17 @@ describe('ReactHooksWithNoopRenderer', () => { return count; } - ReactNoop.render(); - expect(Scheduler).toFlushAndYield([ - 'Render: -1', - 'Effect: 1', - 'Reducer: 1', - 'Reducer: 1', - 'Render: 1', - ]); - expect(ReactNoop).toMatchRenderedOutput('1'); + act(() => { + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'Render: -1', + 'Effect: 1', + 'Reducer: 1', + 'Reducer: 1', + 'Render: 1', + ]); + expect(ReactNoop).toMatchRenderedOutput('1'); + }); act(() => { setCounter(2); @@ -2109,19 +2193,19 @@ describe('ReactHooksWithNoopRenderer', () => { return ; } - ReactNoop.render(, () => - Scheduler.yieldValue('Sync effect'), - ); - expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - - // Enqueuing this update forces the passive effect to be flushed -- - // updateCount(1) happens first, so 2 wins. - // (use batchedUpdates to silence the act() warning) - ReactNoop.batchedUpdates(() => _updateCount(2)); - expect(Scheduler).toHaveYielded(['Will set count to 1']); - expect(Scheduler).toFlushAndYield(['Count: 2']); - expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]); + act(() => { + ReactNoop.render(, () => + Scheduler.yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); + // Enqueuing this update forces the passive effect to be flushed -- + // updateCount(1) happens first, so 2 wins. + act(() => _updateCount(2)); + expect(Scheduler).toHaveYielded(['Will set count to 1']); + expect(Scheduler).toFlushAndYield(['Count: 2']); + expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]); + }); }); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js index ddc62c60aa6fa..9064249f96dc4 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js @@ -111,30 +111,30 @@ describe('ReactIncrementalScheduling', () => { expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:1'); // Schedule deferred work in the reverse order - ReactNoop.batchedUpdates(() => { + ReactNoop.act(() => { ReactNoop.renderToRootWithID(, 'c'); ReactNoop.renderToRootWithID(, 'b'); - }); - // Ensure it starts in the order it was scheduled - expect(Scheduler).toFlushAndYieldThrough(['c:2']); + // Ensure it starts in the order it was scheduled + expect(Scheduler).toFlushAndYieldThrough(['c:2']); + + expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:1'); + expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:1'); + expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); + // Schedule last bit of work, it will get processed the last - expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:1'); - expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:1'); - expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); - // Schedule last bit of work, it will get processed the last - ReactNoop.batchedUpdates(() => { ReactNoop.renderToRootWithID(, 'a'); - }); - // Keep performing work in the order it was scheduled - expect(Scheduler).toFlushAndYieldThrough(['b:2']); - expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:1'); - expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:2'); - expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); - expect(Scheduler).toFlushAndYieldThrough(['a:2']); - expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:2'); - expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:2'); - expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); + // Keep performing work in the order it was scheduled + expect(Scheduler).toFlushAndYieldThrough(['b:2']); + expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:1'); + expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:2'); + expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); + + expect(Scheduler).toFlushAndYieldThrough(['a:2']); + expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:2'); + expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:2'); + expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); + }); }); it('schedules sync updates when inside componentDidMount/Update', () => { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js index 26af1eb0014aa..8e46223f7fc3e 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js @@ -476,70 +476,74 @@ describe('ReactIncrementalUpdates', () => { // First, as a sanity check, assert what happens when four low pri // updates in separate batches are all flushed in the same callback - ReactNoop.render(); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.render(); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.render(); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.render(); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - // Each update flushes in a separate commit. - // Note: This isn't necessarily the ideal behavior. It might be better to - // batch all of these updates together. The fact that they don't is an - // implementation detail. The important part of this unit test is what - // happens when they expire, in which case they really should be batched to - // avoid blocking the main thread for a long time. - expect(Scheduler).toFlushAndYield([ - 'Render: ', - 'Commit: ', - 'Render: he', - 'Commit: he', - 'Render: hell', - 'Commit: hell', - 'Render: hello', - 'Commit: hello', - ]); + ReactNoop.act(() => { + ReactNoop.render(); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.render(); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.render(); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.render(); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + // Each update flushes in a separate commit. + // Note: This isn't necessarily the ideal behavior. It might be better to + // batch all of these updates together. The fact that they don't is an + // implementation detail. The important part of this unit test is what + // happens when they expire, in which case they really should be batched to + // avoid blocking the main thread for a long time. + expect(Scheduler).toFlushAndYield([ + 'Render: ', + 'Commit: ', + 'Render: he', + 'Commit: he', + 'Render: hell', + 'Commit: hell', + 'Render: hello', + 'Commit: hello', + ]); + }); - // Now do the same thing over again, but this time, expire all the updates - // instead of flushing them normally. - ReactNoop.render(); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.render(); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.render(); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.render(); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - // All the updates should render and commit in a single batch. - Scheduler.advanceTime(10000); - expect(Scheduler).toHaveYielded(['Render: goodbye']); - // Passive effect - expect(Scheduler).toFlushAndYield(['Commit: goodbye']); + ReactNoop.act(() => { + // Now do the same thing over again, but this time, expire all the updates + // instead of flushing them normally. + ReactNoop.render(); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.render(); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.render(); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.render(); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + // All the updates should render and commit in a single batch. + Scheduler.advanceTime(10000); + expect(Scheduler).toHaveYielded(['Render: goodbye']); + // Passive effect + expect(Scheduler).toFlushAndYield(['Commit: goodbye']); + }); }); it('flushes all expired updates in a single batch across multiple roots', () => { @@ -559,92 +563,95 @@ describe('ReactIncrementalUpdates', () => { ReactNoop.renderToRootWithID(null, 'other-root'); }); } + ReactNoop.act(() => { + // First, as a sanity check, assert what happens when four low pri + // updates in separate batches are all flushed in the same callback + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + // Each update flushes in a separate commit. + // Note: This isn't necessarily the ideal behavior. It might be better to + // batch all of these updates together. The fact that they don't is an + // implementation detail. The important part of this unit test is what + // happens when they expire, in which case they really should be batched to + // avoid blocking the main thread for a long time. + expect(Scheduler).toFlushAndYield([ + 'Render: ', + 'Commit: ', + 'Render: ', + 'Commit: ', + 'Render: he', + 'Commit: he', + 'Render: he', + 'Commit: he', + 'Render: hell', + 'Commit: hell', + 'Render: hell', + 'Commit: hell', + 'Render: hello', + 'Commit: hello', + 'Render: hello', + 'Commit: hello', + ]); + }); - // First, as a sanity check, assert what happens when four low pri - // updates in separate batches are all flushed in the same callback - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - // Each update flushes in a separate commit. - // Note: This isn't necessarily the ideal behavior. It might be better to - // batch all of these updates together. The fact that they don't is an - // implementation detail. The important part of this unit test is what - // happens when they expire, in which case they really should be batched to - // avoid blocking the main thread for a long time. - expect(Scheduler).toFlushAndYield([ - 'Render: ', - 'Commit: ', - 'Render: ', - 'Commit: ', - 'Render: he', - 'Commit: he', - 'Render: he', - 'Commit: he', - 'Render: hell', - 'Commit: hell', - 'Render: hell', - 'Commit: hell', - 'Render: hello', - 'Commit: hello', - 'Render: hello', - 'Commit: hello', - ]); - - // Now do the same thing over again, but this time, expire all the updates - // instead of flushing them normally. - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - Scheduler.advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['Render: ']); - interrupt(); - - // All the updates should render and commit in a single batch. - Scheduler.advanceTime(10000); - expect(Scheduler).toHaveYielded([ - 'Render: goodbye', - 'Commit: goodbye', - 'Render: goodbye', - ]); - // Passive effect - expect(Scheduler).toFlushAndYield(['Commit: goodbye']); + ReactNoop.act(() => { + // Now do the same thing over again, but this time, expire all the updates + // instead of flushing them normally. + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + Scheduler.advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['Render: ']); + interrupt(); + + // All the updates should render and commit in a single batch. + Scheduler.advanceTime(10000); + expect(Scheduler).toHaveYielded([ + 'Render: goodbye', + 'Commit: goodbye', + 'Render: goodbye', + ]); + // Passive effect + expect(Scheduler).toFlushAndYield(['Commit: goodbye']); + }); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js index dec4bfbfc57b1..7f123fbb66238 100644 --- a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js @@ -133,21 +133,22 @@ describe('ReactSchedulerIntegration', () => { }); return null; } + ReactNoop.act(() => { + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'Render priority: Normal', + 'Passive priority: Normal', + ]); - ReactNoop.render(); - expect(Scheduler).toFlushAndYield([ - 'Render priority: Normal', - 'Passive priority: Normal', - ]); + runWithPriority(UserBlockingPriority, () => { + ReactNoop.render(); + }); - runWithPriority(UserBlockingPriority, () => { - ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'Render priority: UserBlocking', + 'Passive priority: UserBlocking', + ]); }); - - expect(Scheduler).toFlushAndYield([ - 'Render priority: UserBlocking', - 'Passive priority: UserBlocking', - ]); }); it('after completing a level of work, infers priority of the next batch based on its expiration time', () => { diff --git a/packages/react-refresh/src/__tests__/ReactFresh-test.js b/packages/react-refresh/src/__tests__/ReactFresh-test.js index 255b311779cdd..d2e2888a373e5 100644 --- a/packages/react-refresh/src/__tests__/ReactFresh-test.js +++ b/packages/react-refresh/src/__tests__/ReactFresh-test.js @@ -66,7 +66,9 @@ describe('ReactFresh', () => { function patch(version) { const Component = version(); const hotUpdate = ReactFreshRuntime.prepareUpdate(); - scheduleHotUpdate(lastRoot, hotUpdate); + act(() => { + scheduleHotUpdate(lastRoot, hotUpdate); + }); return Component; } @@ -2390,66 +2392,68 @@ describe('ReactFresh', () => { expect(el.hidden).toBe(true); expect(el.firstChild).toBe(null); // Offscreen content not flushed yet. - // Perform a hot update. - patch(() => { - function Hello() { - React.useLayoutEffect(() => { - Scheduler.yieldValue('Hello#layout'); - }); - const [val, setVal] = React.useState(0); - return ( -

setVal(val + 1)}> - {val} -

- ); - } - __register__(Hello, 'Hello'); - }); - - // It's still offscreen so we don't see anything. - expect(container.firstChild).toBe(el); - expect(el.hidden).toBe(true); - expect(el.firstChild).toBe(null); + // wrap this patch() with yet another act(), to prevent prematurely flushing + // effects/updates + act(() => { + // Perform a hot update. + patch(() => { + function Hello() { + React.useLayoutEffect(() => { + Scheduler.yieldValue('Hello#layout'); + }); + const [val, setVal] = React.useState(0); + return ( +

setVal(val + 1)}> + {val} +

+ ); + } + __register__(Hello, 'Hello'); + }); - // Process the offscreen updates. - expect(Scheduler).toFlushAndYieldThrough(['Hello#layout']); - expect(container.firstChild).toBe(el); - expect(el.firstChild.textContent).toBe('0'); - expect(el.firstChild.style.color).toBe('red'); + // It's still offscreen so we don't see anything. + expect(container.firstChild).toBe(el); + expect(el.hidden).toBe(true); + expect(el.firstChild).toBe(null); + // Process the offscreen updates. + expect(Scheduler).toFlushAndYieldThrough(['Hello#layout']); + expect(container.firstChild).toBe(el); + expect(el.firstChild.textContent).toBe('0'); + expect(el.firstChild.style.color).toBe('red'); + + el.firstChild.dispatchEvent(new MouseEvent('click', {bubbles: true})); + expect(el.firstChild.textContent).toBe('0'); + expect(el.firstChild.style.color).toBe('red'); + expect(Scheduler).toFlushAndYieldThrough(['Hello#layout']); + expect(el.firstChild.textContent).toBe('1'); + expect(el.firstChild.style.color).toBe('red'); + // Hot reload while we're offscreen. + patch(() => { + function Hello() { + React.useLayoutEffect(() => { + Scheduler.yieldValue('Hello#layout'); + }); + const [val, setVal] = React.useState(0); + return ( +

setVal(val + 1)}> + {val} +

+ ); + } + __register__(Hello, 'Hello'); + }); - el.firstChild.dispatchEvent(new MouseEvent('click', {bubbles: true})); - expect(el.firstChild.textContent).toBe('0'); - expect(el.firstChild.style.color).toBe('red'); - expect(Scheduler).toFlushAndYieldThrough(['Hello#layout']); - expect(el.firstChild.textContent).toBe('1'); - expect(el.firstChild.style.color).toBe('red'); + // It's still offscreen so we don't see the updates. + expect(container.firstChild).toBe(el); + expect(el.firstChild.textContent).toBe('1'); + expect(el.firstChild.style.color).toBe('red'); - // Hot reload while we're offscreen. - patch(() => { - function Hello() { - React.useLayoutEffect(() => { - Scheduler.yieldValue('Hello#layout'); - }); - const [val, setVal] = React.useState(0); - return ( -

setVal(val + 1)}> - {val} -

- ); - } - __register__(Hello, 'Hello'); + // Process the offscreen updates. + expect(Scheduler).toFlushAndYieldThrough(['Hello#layout']); + expect(container.firstChild).toBe(el); + expect(el.firstChild.textContent).toBe('1'); + expect(el.firstChild.style.color).toBe('orange'); }); - - // It's still offscreen so we don't see the updates. - expect(container.firstChild).toBe(el); - expect(el.firstChild.textContent).toBe('1'); - expect(el.firstChild.style.color).toBe('red'); - - // Process the offscreen updates. - expect(Scheduler).toFlushAndYieldThrough(['Hello#layout']); - expect(container.firstChild).toBe(el); - expect(el.firstChild.textContent).toBe('1'); - expect(el.firstChild.style.color).toBe('orange'); } }); diff --git a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js index 10442f72aeff3..3e90fb3c50342 100644 --- a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js +++ b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js @@ -104,34 +104,32 @@ describe('ReactDOMTracing', () => { const root = ReactDOM.unstable_createRoot(container); SchedulerTracing.unstable_trace('initialization', 0, () => { interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0]; + TestUtils.act(() => { + root.render( + + + , + ); + expect(onInteractionTraced).toHaveBeenCalledTimes(1); + expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction( + interaction, + ); + expect(Scheduler).toFlushAndYieldThrough(['App', 'App:mount']); + expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); + expect(onRender).toHaveBeenCalledTimes(1); + expect(onRender).toHaveLastRenderedWithInteractions( + new Set([interaction]), + ); + expect(Scheduler).toFlushAndYieldThrough(['Child', 'Child:mount']); + expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); + expect(onRender).toHaveBeenCalledTimes(2); + expect(onRender).toHaveLastRenderedWithInteractions( + new Set([interaction]), + ); - root.render( - - - , - ); + expect(Scheduler).toFlushAndYield(['Child', 'Child:update']); + }); }); - - expect(onInteractionTraced).toHaveBeenCalledTimes(1); - expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction( - interaction, - ); - - expect(Scheduler).toFlushAndYieldThrough(['App', 'App:mount']); - expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); - expect(onRender).toHaveBeenCalledTimes(1); - expect(onRender).toHaveLastRenderedWithInteractions( - new Set([interaction]), - ); - - expect(Scheduler).toFlushAndYieldThrough(['Child', 'Child:mount']); - expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); - expect(onRender).toHaveBeenCalledTimes(2); - expect(onRender).toHaveLastRenderedWithInteractions( - new Set([interaction]), - ); - - expect(Scheduler).toFlushAndYield(['Child', 'Child:update']); expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1); expect( onInteractionScheduledWorkCompleted, @@ -172,33 +170,34 @@ describe('ReactDOMTracing', () => { SchedulerTracing.unstable_trace('initialization', 0, () => { interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0]; - root.render( - - - , - ); - }); - - expect(onInteractionTraced).toHaveBeenCalledTimes(1); - expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction( - interaction, - ); + TestUtils.act(() => { + root.render( + + + , + ); + expect(onInteractionTraced).toHaveBeenCalledTimes(1); + expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction( + interaction, + ); - expect(Scheduler).toFlushAndYieldThrough(['App', 'App:mount']); - expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); - expect(onRender).toHaveBeenCalledTimes(1); - expect(onRender).toHaveLastRenderedWithInteractions( - new Set([interaction]), - ); + expect(Scheduler).toFlushAndYieldThrough(['App', 'App:mount']); + expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); + expect(onRender).toHaveBeenCalledTimes(1); + expect(onRender).toHaveLastRenderedWithInteractions( + new Set([interaction]), + ); - expect(wrapped).not.toBeNull(); + expect(wrapped).not.toBeNull(); - expect(Scheduler).toFlushAndYield(['Child']); - expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); - expect(onRender).toHaveBeenCalledTimes(2); - expect(onRender).toHaveLastRenderedWithInteractions( - new Set([interaction]), - ); + expect(Scheduler).toFlushAndYield(['Child']); + expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); + expect(onRender).toHaveBeenCalledTimes(2); + expect(onRender).toHaveLastRenderedWithInteractions( + new Set([interaction]), + ); + }); + }); wrapped(); expect(onInteractionTraced).toHaveBeenCalledTimes(1); @@ -249,34 +248,35 @@ describe('ReactDOMTracing', () => { const root = ReactDOM.unstable_createRoot(container); SchedulerTracing.unstable_trace('initialization', 0, () => { interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0]; + TestUtils.act(() => { + root.render( + + + , + ); + expect(onInteractionTraced).toHaveBeenCalledTimes(1); + expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction( + interaction, + ); - root.render( - - - , - ); - }); - - expect(onInteractionTraced).toHaveBeenCalledTimes(1); - expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction( - interaction, - ); + expect(Scheduler).toFlushAndYieldThrough(['App', 'App:mount']); + expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); + expect(onRender).toHaveBeenCalledTimes(1); + expect(onRender).toHaveLastRenderedWithInteractions( + new Set([interaction]), + ); - expect(Scheduler).toFlushAndYieldThrough(['App', 'App:mount']); - expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); - expect(onRender).toHaveBeenCalledTimes(1); - expect(onRender).toHaveLastRenderedWithInteractions( - new Set([interaction]), - ); + expect(Scheduler).toFlushAndYieldThrough(['Child', 'Child:mount']); + expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); + expect(onRender).toHaveBeenCalledTimes(2); + expect(onRender).toHaveLastRenderedWithInteractions( + new Set([interaction]), + ); - expect(Scheduler).toFlushAndYieldThrough(['Child', 'Child:mount']); - expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); - expect(onRender).toHaveBeenCalledTimes(2); - expect(onRender).toHaveLastRenderedWithInteractions( - new Set([interaction]), - ); + expect(Scheduler).toFlushAndYield(['Child', 'Child:update']); + }); + }); - expect(Scheduler).toFlushAndYield(['Child', 'Child:update']); expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1); expect( onInteractionScheduledWorkCompleted, diff --git a/packages/react/src/__tests__/withComponentStack-test.js b/packages/react/src/__tests__/withComponentStack-test.js index e46d7f2e54997..3b26db7357e24 100644 --- a/packages/react/src/__tests__/withComponentStack-test.js +++ b/packages/react/src/__tests__/withComponentStack-test.js @@ -178,10 +178,9 @@ describe('withComponentStack', () => { }); return null; } - - ReactTestRenderer.create(); - - scheduler.flushAll(); // Flush passive effects + ReactTestRenderer.act(() => { + ReactTestRenderer.create(); + }); expectMessageAndStack( 'logged in child render method',