From 043aef45cf36557980e95996dc4a07d5f44dbd75 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 29 May 2019 17:58:58 +0100 Subject: [PATCH 1/6] 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 | 42 +- .../ReactDOMTracing-test.internal.js | 148 ++-- .../src/__tests__/withComponentStack-test.js | 7 +- 15 files changed, 931 insertions(+), 724 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 7f9372bdc7159..1ed7ecb7247f7 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -35,6 +35,7 @@ import { computeExpirationForFiber, flushPassiveEffects, requestCurrentTime, + warnIfNotCurrentlyActingEffectsInDEV, warnIfNotCurrentlyActingUpdatesInDev, warnIfNotScopedWithMatchingAct, markRenderEventTimeAndConfig, @@ -898,6 +899,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, @@ -910,6 +919,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 57b0913dd8d91..d2a60a636fad1 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 f56a3bd5bf03e..41e1771e68284 100644 --- a/packages/react-refresh/src/__tests__/ReactFresh-test.js +++ b/packages/react-refresh/src/__tests__/ReactFresh-test.js @@ -2676,15 +2676,18 @@ describe('ReactFresh', () => { expect(container.firstChild.nextSibling.nextSibling).toBe(secondP); // Perform a hot update that fixes the error. - patch(() => { - function Hello() { - const [x] = React.useState(''); - React.useEffect(() => {}, []); // Removes the bad effect code. - x.slice(); // Doesn't throw initially. - return

Fixed!

; - } - $RefreshReg$(Hello, 'Hello'); - }); + act(() => { + patch(() => { + function Hello() { + const [x] = React.useState(''); + React.useEffect(() => {}, []); // Removes the bad effect code. + x.slice(); // Doesn't throw initially. + return

Fixed!

; + } + $RefreshReg$(Hello, 'Hello'); + }); + }) + // This should remount the error boundary (but not anything above it). expect(container.innerHTML).toBe('

A

Fixed!

B

'); @@ -2693,15 +2696,18 @@ describe('ReactFresh', () => { // Verify next hot reload doesn't remount anything. const helloNode = container.firstChild.nextSibling; - patch(() => { - function Hello() { - const [x] = React.useState(''); - React.useEffect(() => {}, []); - x.slice(); - return

Nice.

; - } - $RefreshReg$(Hello, 'Hello'); - }); + act(() => { + patch(() => { + function Hello() { + const [x] = React.useState(''); + React.useEffect(() => {}, []); + x.slice(); + return

Nice.

; + } + $RefreshReg$(Hello, 'Hello'); + }); + }) + expect(container.firstChild.nextSibling).toBe(helloNode); expect(helloNode.textContent).toBe('Nice.'); } 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', From bad37b8032a38e816f990ae702b2a9d578919864 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 29 May 2019 18:33:27 +0100 Subject: [PATCH 2/6] pass lint --- packages/react-dom/src/__tests__/ReactDOMHooks-test.js | 2 -- .../src/__tests__/ReactErrorBoundaries-test.internal.js | 2 -- packages/react/src/__tests__/withComponentStack-test.js | 3 --- 3 files changed, 7 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js index c703d5e86a1c2..f5f8a61b4171c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js @@ -11,7 +11,6 @@ let React; let ReactDOM; -let act; let Scheduler; describe('ReactDOMHooks', () => { @@ -22,7 +21,6 @@ describe('ReactDOMHooks', () => { React = require('react'); ReactDOM = require('react-dom'); - act = require('react-dom/test-utils').act; Scheduler = require('scheduler'); container = document.createElement('div'); diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 81c333e4b44f5..478d30d136538 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -14,7 +14,6 @@ let React; let ReactDOM; let act; let ReactFeatureFlags; -let Scheduler; describe('ReactErrorBoundaries', () => { let log; @@ -47,7 +46,6 @@ describe('ReactErrorBoundaries', () => { ReactDOM = require('react-dom'); React = require('react'); act = require('react-dom/test-utils').act; - Scheduler = require('scheduler'); log = []; diff --git a/packages/react/src/__tests__/withComponentStack-test.js b/packages/react/src/__tests__/withComponentStack-test.js index 3b26db7357e24..3f9a709816e85 100644 --- a/packages/react/src/__tests__/withComponentStack-test.js +++ b/packages/react/src/__tests__/withComponentStack-test.js @@ -44,16 +44,13 @@ describe('withComponentStack', () => { let React = null; let ReactTestRenderer = null; let error = null; - let scheduler = null; let warn = null; beforeEach(() => { jest.resetModules(); - jest.mock('scheduler', () => require('scheduler/unstable_mock')); React = require('react'); ReactTestRenderer = require('react-test-renderer'); - scheduler = require('scheduler'); error = React.error; warn = React.warn; From 46c0741a1cc960a00acde51f8d974155fa9f1ec9 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sun, 23 Jun 2019 15:27:52 +0100 Subject: [PATCH 3/6] pass failing test, fixes another - a test was failing in ReactDOMServerIntegrationHooks while testing an effect; the behaviour of yields was different from browser and server when wrapped with act(). further, because of how we initialized modules, act() around renders wasn't working corrrectly. solved by passing in ReactTestUtils in initModules, and checking on the finally yielded values in the specific test. - in ReactUpdates, while testing an infinite recursion detection, the test needed to be wrapped in an act(), which would have caused the recusrsion error to throw. solived by rethrowing the error from inside the act(). --- ...eactDOMServerIntegrationAttributes-test.js | 3 ++ .../ReactDOMServerIntegrationBasic-test.js | 3 ++ .../ReactDOMServerIntegrationCheckbox-test.js | 3 ++ ...MServerIntegrationClassContextType-test.js | 3 ++ .../ReactDOMServerIntegrationElements-test.js | 3 ++ .../ReactDOMServerIntegrationFragment-test.js | 3 ++ ...DOMServerIntegrationHooks-test.internal.js | 29 +++++++++++++------ .../ReactDOMServerIntegrationInput-test.js | 3 ++ ...tDOMServerIntegrationLegacyContext-test.js | 3 ++ .../ReactDOMServerIntegrationModes-test.js | 3 ++ ...eactDOMServerIntegrationNewContext-test.js | 3 ++ ...ctDOMServerIntegrationReconnecting-test.js | 3 ++ .../ReactDOMServerIntegrationRefs-test.js | 3 ++ .../ReactDOMServerIntegrationSelect-test.js | 3 ++ ...ctDOMServerIntegrationSpecialTypes-test.js | 3 ++ .../ReactDOMServerIntegrationTextarea-test.js | 3 ++ ...erIntegrationUntrustedURL-test.internal.js | 5 ++++ ...OMServerIntegrationUserInteraction-test.js | 3 ++ .../ReactDOMserverIntegrationProgress-test.js | 3 ++ .../src/__tests__/ReactUpdates-test.js | 25 +++++++++------- .../ReactDOMServerIntegrationTestUtils.js | 11 +++++-- ...eactHooksWithNoopRenderer-test.internal.js | 1 - .../src/__tests__/ReactFresh-test.js | 11 ++++--- 23 files changed, 103 insertions(+), 30 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js index 50fd7ae16bc8e..c60ed8770b310 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js @@ -13,6 +13,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; +let ReactTestUtils; let ReactDOMServer; function initModules() { @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationBasic-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationBasic-test.js index 114907ef7d177..72ac5ff06b49f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationBasic-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationBasic-test.js @@ -16,6 +16,7 @@ const TEXT_NODE_TYPE = 3; let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -23,11 +24,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationCheckbox-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationCheckbox-test.js index 03e7c4b984480..3280ed5e0626c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationCheckbox-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationCheckbox-test.js @@ -16,6 +16,7 @@ const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags'); let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -23,11 +24,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationClassContextType-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationClassContextType-test.js index 19839aa7cd6c2..a24e969d0f527 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationClassContextType-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationClassContextType-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js index 535191d66e8a7..8b66321f8692a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js @@ -16,17 +16,20 @@ const TEXT_NODE_TYPE = 3; let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { jest.resetModuleRegistry(); React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationFragment-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationFragment-test.js index 8c93ee9eea556..d1144aebdaedf 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationFragment-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationFragment-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index 97d304c0647dd..b4f99453cd328 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -17,7 +17,7 @@ let React; let ReactFeatureFlags; let ReactDOM; let ReactDOMServer; -let act; +let ReactTestUtils; let useState; let useReducer; let useEffect; @@ -42,7 +42,7 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); - act = require('react-dom/test-utils').act; + ReactTestUtils = require('react-dom/test-utils'); useState = React.useState; useReducer = React.useReducer; useEffect = React.useEffect; @@ -69,6 +69,7 @@ function initModules() { return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } @@ -541,19 +542,29 @@ describe('ReactDOMServerHooks', () => { }); describe('useEffect', () => { + const yields = []; itRenders('should ignore effects on the server', async render => { function Counter(props) { useEffect(() => { - yieldValue('should not be invoked'); + yieldValue('invoked on client'); }); return ; } - await act(async () => { - const domNode = await render(); - expect(clearYields()).toEqual(['Count: 0']); - expect(domNode.tagName).toEqual('SPAN'); - expect(domNode.textContent).toEqual('Count: 0'); - }); + + const domNode = await render(); + yields.push(clearYields()); + expect(domNode.tagName).toEqual('SPAN'); + expect(domNode.textContent).toEqual('Count: 0'); + }); + + it('verifies yields in order', () => { + expect(yields).toEqual([ + ['Count: 0'], // server render + ['Count: 0'], // server stream + ['Count: 0', 'invoked on client'], // clean render + ['Count: 0', 'invoked on client'], // hydrated render + // nothing yielded for bad markup + ]); }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationInput-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationInput-test.js index d637755d02c0b..9ca1c92d8d427 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationInput-test.js @@ -16,6 +16,7 @@ const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags'); let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -23,11 +24,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js index e801902011b61..4c78919bd8480 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js @@ -15,6 +15,7 @@ let PropTypes; let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -23,11 +24,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationModes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationModes-test.js index b9f8a5c48c8a1..188ab02991868 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationModes-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationModes-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js index 071dd8715751c..6089daa80af29 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationReconnecting-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationReconnecting-test.js index b6428b1fa3824..aba74c2f34fcf 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationReconnecting-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationReconnecting-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -22,11 +23,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js index ccb9a60607833..4d39fce80c885 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js index 486b2a81a6925..3871ad975e743 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSpecialTypes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSpecialTypes-test.js index b56d36b626a76..2cc696ae6e4e8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSpecialTypes-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSpecialTypes-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; let forwardRef; let memo; let yieldedValues; @@ -26,6 +27,7 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); forwardRef = React.forwardRef; memo = React.memo; @@ -43,6 +45,7 @@ function initModules() { return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationTextarea-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationTextarea-test.js index 2b0faddcd2c44..261b6c041495a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationTextarea-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationTextarea-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.internal.js index 21a4cc1b6dc87..897e2d51183b2 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.internal.js @@ -16,6 +16,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function runTests(itRenders, itRejectsRendering, expectToReject) { itRenders('a http link with the word javascript in it', async render => { @@ -152,11 +153,13 @@ describe('ReactDOMServerIntegration - Untrusted URLs', () => { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } @@ -185,11 +188,13 @@ describe('ReactDOMServerIntegration - Untrusted URLs - disableJavaScriptURLs', ( React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUserInteraction-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUserInteraction-test.js index 558aad7648b40..c0e2733c54169 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUserInteraction-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUserInteraction-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMserverIntegrationProgress-test.js b/packages/react-dom/src/__tests__/ReactDOMserverIntegrationProgress-test.js index 4a8eb06faace6..5ff530b10a858 100644 --- a/packages/react-dom/src/__tests__/ReactDOMserverIntegrationProgress-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMserverIntegrationProgress-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -21,11 +22,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index 4eea6a6b3d746..6d7435fe4cde7 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -1621,20 +1621,23 @@ describe('ReactUpdates', () => { let stack = null; let originalConsoleError = console.error; console.error = (e, s) => { - // skip the missing act() error - if (e.slice(0, 40) !== 'Warning: Your test just caused an effect') { - error = e; - stack = s; - } + error = e; + stack = s; }; try { const container = document.createElement('div'); - ReactDOM.render(, container); - while (error === null) { - Scheduler.unstable_flushNumberOfYields(1); - } - expect(error).toContain('Warning: Maximum update depth exceeded.'); - expect(stack).toContain('in NonTerminating'); + expect(() => { + act(() => { + ReactDOM.render(, container); + while (error === null) { + Scheduler.unstable_flushNumberOfYields(1); + } + expect(error).toContain('Warning: Maximum update depth exceeded.'); + expect(stack).toContain('in NonTerminating'); + // rethrow error to prevent going into an infinite loop when act() exits + throw error; + }); + }).toThrow('Maximum update depth exceeded.'); } finally { console.error = originalConsoleError; } diff --git a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js index 55350b42a378e..c4f7f7bada09f 100644 --- a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js +++ b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js @@ -14,9 +14,10 @@ const stream = require('stream'); module.exports = function(initModules) { let ReactDOM; let ReactDOMServer; + let ReactTestUtils; function resetModules() { - ({ReactDOM, ReactDOMServer} = initModules()); + ({ReactDOM, ReactDOMServer, ReactTestUtils} = initModules()); } function shouldUseDocument(reactElement) { @@ -48,9 +49,13 @@ module.exports = function(initModules) { function asyncReactDOMRender(reactElement, domElement, forceHydrate) { return new Promise(resolve => { if (forceHydrate) { - ReactDOM.hydrate(reactElement, domElement); + ReactTestUtils.act(() => { + ReactDOM.hydrate(reactElement, domElement); + }); } else { - ReactDOM.render(reactElement, domElement); + ReactTestUtils.act(() => { + ReactDOM.render(reactElement, domElement); + }); } // We can't use the callback for resolution because that will not catch // errors. They're thrown. diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index 049111e17095c..d55781e0af2fe 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -2047,7 +2047,6 @@ describe('ReactHooksWithNoopRenderer', () => { expect(Scheduler).toFlushAndYieldThrough(['Sync effect']); }); - // ReactNoop.flushPassiveEffects(); expect(Scheduler).toHaveYielded(['Mount A']); act(() => { diff --git a/packages/react-refresh/src/__tests__/ReactFresh-test.js b/packages/react-refresh/src/__tests__/ReactFresh-test.js index 41e1771e68284..790a418013b6b 100644 --- a/packages/react-refresh/src/__tests__/ReactFresh-test.js +++ b/packages/react-refresh/src/__tests__/ReactFresh-test.js @@ -2685,9 +2685,8 @@ describe('ReactFresh', () => { return

Fixed!

; } $RefreshReg$(Hello, 'Hello'); - }); - }) - + }); + }); // This should remount the error boundary (but not anything above it). expect(container.innerHTML).toBe('

A

Fixed!

B

'); @@ -2705,9 +2704,9 @@ describe('ReactFresh', () => { return

Nice.

; } $RefreshReg$(Hello, 'Hello'); - }); - }) - + }); + }); + expect(container.firstChild.nextSibling).toBe(helloNode); expect(helloNode.textContent).toBe('Nice.'); } From 17a9cad12a00e5e8de124806d0e50ae991402827 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sun, 23 Jun 2019 20:03:49 +0100 Subject: [PATCH 4/6] pass ReactDOMServerSuspense --- .../src/__tests__/ReactDOMServerSuspense-test.internal.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js index bab90592430de..434fc6c8a507f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js @@ -15,6 +15,7 @@ let React; let ReactDOM; let ReactDOMServer; let ReactFeatureFlags; +let ReactTestUtils; function initModules() { // Reset warning cache. @@ -26,11 +27,13 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. return { ReactDOM, ReactDOMServer, + ReactTestUtils, }; } From c777608a910e0fd9097920e4a1818c550daa5898 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sun, 23 Jun 2019 20:05:38 +0100 Subject: [PATCH 5/6] stray todo --- packages/react-dom/src/__tests__/ReactDOMHooks-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js index f5f8a61b4171c..1af896f1631e2 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js @@ -55,7 +55,6 @@ describe('ReactDOMHooks', () => { // 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'); From 8bcfcc351cc66a2dcebbdc737f9d9c97bc22f762 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 24 Jun 2019 10:45:17 +0100 Subject: [PATCH 6/6] a better message, consistent with the state update one. --- .../src/__tests__/ReactDOMHooks-test.js | 8 ++++---- .../react-reconciler/src/ReactFiberWorkLoop.js | 2 +- .../src/__tests__/ReactHooks-test.internal.js | 2 +- .../ReactHooksWithNoopRenderer-test.internal.js | 16 +++++++--------- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js index 1af896f1631e2..ec23c88fa981f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js @@ -74,10 +74,10 @@ describe('ReactDOMHooks', () => { 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', + 'An update to Example1 ran an effect', + 'An update to Example2 ran an effect', + 'An update to Example1 ran an effect', + 'An update to Example2 ran an effect', ]); }); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index d2a60a636fad1..ebf98cf617b41 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2434,7 +2434,7 @@ export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void { if (ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil) { warningWithoutStack( false, - 'Your test just caused an effect from %s, but was not wrapped in act(...).\n\n' + + 'An update to %s ran an effect, 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' + diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 922aa8c89110a..ea6e3103edfe1 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1800,7 +1800,7 @@ describe('ReactHooks', () => { globalListener(); globalListener(); }).toWarnDev([ - 'Your test just caused an effect from C', + 'An update to C ran an effect', '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. diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index d55781e0af2fe..8c49c606a87de 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -674,7 +674,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(Scheduler).toFlushAndYield(['Passive effect']); expect(ReactNoop.getChildren()).toEqual([span('Passive')]); }); - // exiting act calls flushePassiveEffects(), but there none are left with flush. + // exiting act calls flushPassiveEffects(), but there are none left to flush. expect(Scheduler).toHaveYielded([]); }); @@ -885,14 +885,13 @@ describe('ReactHooksWithNoopRenderer', () => { // 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']); + }).toWarnDev(['An update to Counter ran an effect']); // A discrete event forces the passive effect to be flushed -- // updateCount(1) happens first, so 2 wins. @@ -907,8 +906,8 @@ describe('ReactHooksWithNoopRenderer', () => { expect(() => { expect(Scheduler).toFlushAndYield(['Count: 2']); }).toWarnDev([ - 'Your test just caused an effect from Counter', - 'Your test just caused an effect from Counter', + 'An update to Counter ran an effect', + 'An update to Counter ran an effect', ]); expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]); @@ -943,7 +942,6 @@ describe('ReactHooksWithNoopRenderer', () => { const tracingEvent = {id: 0, name: 'hello', timestamp: 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, @@ -956,7 +954,7 @@ describe('ReactHooksWithNoopRenderer', () => { ); expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']); expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - }).toWarnDev(['Your test just caused an effect from Counter']); + }).toWarnDev(['An update to Counter ran an effect']); expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(0); @@ -973,8 +971,8 @@ describe('ReactHooksWithNoopRenderer', () => { expect(() => { expect(Scheduler).toFlushAndYield(['Count: 2']); }).toWarnDev([ - 'Your test just caused an effect from Counter', - 'Your test just caused an effect from Counter', + 'An update to Counter ran an effect', + 'An update to Counter ran an effect', ]); expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]);