From 0f1977ad0e9e90260593ca5bac485b577168ad36 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 8 May 2019 18:05:27 +0100 Subject: [PATCH 01/15] s/flushPassiveEffects/unstable_flushWithoutYielding a first crack at flushing the scheduler manually from inside act(). uses unstable_flushWithoutYielding(). The tests that changes, mostly replaced toFlushAndYield(...) with toHaveYielded(). For some tests that tested the state of the tree before flushing effects (but still after updates), I replaced act() with bacthedUpdates(). --- .../src/__tests__/ReactTestUtilsAct-test.js | 22 ++++++ packages/react-dom/src/client/ReactDOM.js | 4 +- .../src/test-utils/ReactTestUtils.js | 2 +- .../src/test-utils/ReactTestUtilsAct.js | 9 +-- .../src/createReactNoop.js | 10 +-- .../src/ReactFiberReconciler.js | 2 + .../src/ReactFiberScheduler.js | 4 ++ .../src/__tests__/ReactHooks-test.internal.js | 72 ++++++++++--------- ...eactHooksWithNoopRenderer-test.internal.js | 52 +++++++------- ...eactIncrementalScheduling-test.internal.js | 6 +- .../__tests__/ReactNoopRendererAct-test.js | 9 +-- .../src/ReactTestRendererAct.js | 12 ++-- 12 files changed, 117 insertions(+), 87 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js index 9969fa721125..3e6f96afad15 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js @@ -13,6 +13,7 @@ let ReactTestUtils; let SchedulerTracing; let act; let container; +let Scheduler; jest.useRealTimers(); @@ -31,6 +32,7 @@ describe('ReactTestUtils.act()', () => { ReactDOM = require('react-dom'); ReactTestUtils = require('react-dom/test-utils'); SchedulerTracing = require('scheduler/tracing'); + Scheduler = require('scheduler'); act = ReactTestUtils.act; container = document.createElement('div'); document.body.appendChild(container); @@ -503,4 +505,24 @@ describe('ReactTestUtils.act()', () => { }); } }); + describe('concurrent mode', () => { + it('flushes renders', () => { + function App() { + let [state, setState] = React.useState(0); + React.useEffect( + () => { + setState(x => x + 1); + }, + [Math.min(state, 4)], + ); + return state; + } + const el = document.createElement('div'); + act(() => { + ReactDOM.unstable_createRoot(el).render(); + }); + // Scheduler.flushAll(); + expect(el.innerHTML).toBe('5'); + }); + }); }); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 728f775adf5e..f473b09057b0 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -35,7 +35,7 @@ import { getPublicRootInstance, findHostInstance, findHostInstanceWithWarning, - flushPassiveEffects, + hasPendingEffects, } from 'react-reconciler/inline.dom'; import {createPortal as createPortalImpl} from 'shared/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -821,7 +821,7 @@ const ReactDOM: Object = { restoreStateIfNeeded, dispatchEvent, runEventsInBatch, - flushPassiveEffects, + hasPendingEffects, ], }, }; diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index f463cc4aa6a3..a8abfc58f807 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -43,7 +43,7 @@ const [ dispatchEvent, runEventsInBatch, // eslint-disable-next-line no-unused-vars - flushPassiveEffects, + hasPendingEffects, ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; function Event(suffix) {} diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js index 99cb73ede7c1..4a5e5ac1ae4f 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -13,6 +13,7 @@ import warningWithoutStack from 'shared/warningWithoutStack'; import ReactDOM from 'react-dom'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import enqueueTask from 'shared/enqueueTask'; +import * as Scheduler from 'scheduler'; // Keep in sync with ReactDOMUnstableNativeDependencies.js // ReactDOM.js, and ReactTestUtils.js: @@ -30,7 +31,7 @@ const [ dispatchEvent, runEventsInBatch, /* eslint-enable no-unused-vars */ - flushPassiveEffects, + hasPendingEffects, ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; const batchedUpdates = ReactDOM.unstable_batchedUpdates; @@ -46,9 +47,9 @@ let actingUpdatesScopeDepth = 0; function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { try { - flushPassiveEffects(); + Scheduler.unstable_flushWithoutYielding(); enqueueTask(() => { - if (flushPassiveEffects()) { + if (hasPendingEffects()) { flushEffectsAndMicroTasks(onDone); } else { onDone(); @@ -147,7 +148,7 @@ function act(callback: () => Thenable) { // flush effects until none remain, and cleanup try { - while (flushPassiveEffects()) {} + Scheduler.unstable_flushWithoutYielding(); onDone(); } catch (err) { onDone(); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 3ac1c7021de5..1f79342a088b 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -645,18 +645,20 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { const roots = new Map(); const DEFAULT_ROOT_ID = ''; - const {flushPassiveEffects, batchedUpdates} = NoopRenderer; + const {hasPendingEffects, batchedUpdates} = NoopRenderer; // this act() implementation should be exactly the same in // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js + // we track the 'depth' of the act() calls with this counter, + // so we can tell if any async act() calls try to run in parallel. let actingUpdatesScopeDepth = 0; function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { try { - flushPassiveEffects(); + Scheduler.unstable_flushWithoutYielding(); enqueueTask(() => { - if (flushPassiveEffects()) { + if (hasPendingEffects()) { flushEffectsAndMicroTasks(onDone); } else { onDone(); @@ -755,7 +757,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // flush effects until none remain, and cleanup try { - while (flushPassiveEffects()) {} + Scheduler.unstable_flushWithoutYielding(); onDone(); } catch (err) { onDone(); diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 77fadd31e9b2..dcfe248cedd0 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -53,6 +53,7 @@ import { interactiveUpdates, flushInteractiveUpdates, flushPassiveEffects, + hasPendingEffects, } from './ReactFiberScheduler'; import {createUpdate, enqueueUpdate} from './ReactUpdateQueue'; import ReactFiberInstrumentation from './ReactFiberInstrumentation'; @@ -309,6 +310,7 @@ export { flushControlled, flushSync, flushPassiveEffects, + hasPendingEffects, }; export function getPublicRootInstance( diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index f748228f8eb0..55e1f0b05e55 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -1665,6 +1665,10 @@ export function flushPassiveEffects() { return true; } +export function hasPendingEffects() { + return rootWithPendingPassiveEffects !== null; +} + export function isAlreadyFailedLegacyErrorBoundary(instance: mixed): boolean { return ( legacyErrorBoundariesThatAlreadyFailed !== null && diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index e2e5178ba37f..1e4b1939ef05 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -95,7 +95,7 @@ describe('ReactHooks', () => { setCounter2(1); }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Parent: 1, 1', 'Child: 1, 1', 'Effect: 1, 1', @@ -103,7 +103,7 @@ describe('ReactHooks', () => { // Update that bails out. act(() => setCounter1(1)); - expect(Scheduler).toFlushAndYield(['Parent: 1, 1']); + expect(Scheduler).toHaveYielded(['Parent: 1, 1']); // This time, one of the state updates but the other one doesn't. So we // can't bail out. @@ -112,7 +112,7 @@ describe('ReactHooks', () => { setCounter2(2); }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Parent: 1, 2', 'Child: 1, 2', 'Effect: 1, 2', @@ -130,14 +130,15 @@ describe('ReactHooks', () => { // Because the final values are the same as the current values, the // component bails out. - expect(Scheduler).toFlushAndYield(['Parent: 1, 2']); + expect(Scheduler).toHaveYielded(['Parent: 1, 2']); // prepare to check SameValue act(() => { setCounter1(0 / -1); setCounter2(NaN); }); - expect(Scheduler).toFlushAndYield([ + + expect(Scheduler).toHaveYielded([ 'Parent: 0, NaN', 'Child: 0, NaN', 'Effect: 0, NaN', @@ -151,13 +152,13 @@ describe('ReactHooks', () => { setCounter2(NaN); }); - expect(Scheduler).toFlushAndYield(['Parent: 0, NaN']); + expect(Scheduler).toHaveYielded(['Parent: 0, NaN']); // check if changing negative 0 to positive 0 does not bail out act(() => { setCounter1(0); }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Parent: 0, NaN', 'Child: 0, NaN', 'Effect: 0, NaN', @@ -201,14 +202,14 @@ describe('ReactHooks', () => { setCounter2(1); }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Parent: 1, 1 (light)', 'Child: 1, 1 (light)', ]); // Update that bails out. act(() => setCounter1(1)); - expect(Scheduler).toFlushAndYield(['Parent: 1, 1 (light)']); + expect(Scheduler).toHaveYielded(['Parent: 1, 1 (light)']); // This time, one of the state updates but the other one doesn't. So we // can't bail out. @@ -217,7 +218,7 @@ describe('ReactHooks', () => { setCounter2(2); }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Parent: 1, 2 (light)', 'Child: 1, 2 (light)', ]); @@ -227,10 +228,10 @@ describe('ReactHooks', () => { act(() => { setCounter1(1); setCounter2(2); + root.update(); }); - root.update(); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Parent: 1, 2 (dark)', 'Child: 1, 2 (dark)', ]); @@ -239,10 +240,10 @@ describe('ReactHooks', () => { act(() => { setCounter1(1); setCounter2(2); + root.update(); }); - root.update(); - expect(Scheduler).toFlushAndYield(['Parent: 1, 2 (dark)']); + expect(Scheduler).toHaveYielded(['Parent: 1, 2 (dark)']); }); it('warns about setState second argument', () => { @@ -275,7 +276,7 @@ describe('ReactHooks', () => { 'declare it in the component body with useEffect().', {withoutStack: true}, ); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(root).toMatchRenderedOutput('1'); }); @@ -309,7 +310,7 @@ describe('ReactHooks', () => { 'declare it in the component body with useEffect().', {withoutStack: true}, ); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(root).toMatchRenderedOutput('1'); }); @@ -347,14 +348,17 @@ describe('ReactHooks', () => { }); return ; } + let root; + act(() => { + root = ReactTestRenderer.create(null, {unstable_isConcurrent: true}); + root.update( + + + , + ); + }); - const root = ReactTestRenderer.create(null, {unstable_isConcurrent: true}); - root.update( - - - , - ); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Theme: light', 'Parent: 0 (light)', 'Child: 0 (light)', @@ -370,7 +374,7 @@ describe('ReactHooks', () => { // Normal update act(() => setCounter(1)); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Parent: 1 (light)', 'Child: 1 (light)', 'Effect: 1 (light)', @@ -379,7 +383,7 @@ describe('ReactHooks', () => { // Update that doesn't change state, so it bails out act(() => setCounter(1)); - expect(Scheduler).toFlushAndYield(['Parent: 1 (light)']); + expect(Scheduler).toHaveYielded(['Parent: 1 (light)']); expect(root).toMatchRenderedOutput('1 (light)'); // Update that doesn't change state, but the context changes, too, so it @@ -389,7 +393,7 @@ describe('ReactHooks', () => { setTheme('dark'); }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Theme: dark', 'Parent: 1 (dark)', 'Child: 1 (dark)', @@ -424,7 +428,7 @@ describe('ReactHooks', () => { // Normal update act(() => setCounter(1)); - expect(Scheduler).toFlushAndYield(['Parent: 1', 'Child: 1', 'Effect: 1']); + expect(Scheduler).toHaveYielded(['Parent: 1', 'Child: 1', 'Effect: 1']); expect(root).toMatchRenderedOutput('1'); // Update to the same state. React doesn't know if the queue is empty @@ -432,7 +436,7 @@ describe('ReactHooks', () => { // enter the render phase before we can bail out. But we bail out before // rendering the child, and we don't fire any effects. act(() => setCounter(1)); - expect(Scheduler).toFlushAndYield(['Parent: 1']); + expect(Scheduler).toHaveYielded(['Parent: 1']); expect(root).toMatchRenderedOutput('1'); // Update to the same state again. This times, neither fiber has pending @@ -443,14 +447,14 @@ describe('ReactHooks', () => { // This changes the state to something different so it renders normally. act(() => setCounter(2)); - expect(Scheduler).toFlushAndYield(['Parent: 2', 'Child: 2', 'Effect: 2']); + expect(Scheduler).toHaveYielded(['Parent: 2', 'Child: 2', 'Effect: 2']); expect(root).toMatchRenderedOutput('2'); // prepare to check SameValue act(() => { setCounter(0); }); - expect(Scheduler).toFlushAndYield(['Parent: 0', 'Child: 0', 'Effect: 0']); + expect(Scheduler).toHaveYielded(['Parent: 0', 'Child: 0', 'Effect: 0']); expect(root).toMatchRenderedOutput('0'); // Update to the same state for the first time to flush the queue @@ -458,7 +462,7 @@ describe('ReactHooks', () => { setCounter(0); }); - expect(Scheduler).toFlushAndYield(['Parent: 0']); + expect(Scheduler).toHaveYielded(['Parent: 0']); expect(root).toMatchRenderedOutput('0'); // Update again to the same state. Should bail out. @@ -472,7 +476,7 @@ describe('ReactHooks', () => { act(() => { setCounter(0 / -1); }); - expect(Scheduler).toFlushAndYield(['Parent: 0', 'Child: 0', 'Effect: 0']); + expect(Scheduler).toHaveYielded(['Parent: 0', 'Child: 0', 'Effect: 0']); expect(root).toMatchRenderedOutput('0'); }); @@ -503,7 +507,7 @@ describe('ReactHooks', () => { return value; }); }; - act(() => { + ReactTestRenderer.unstable_batchedUpdates(() => { update(0); update(0); update(0); @@ -564,7 +568,7 @@ describe('ReactHooks', () => { }; // Update at normal priority - act(() => update(n => n * 100)); + ReactTestRenderer.unstable_batchedUpdates(() => update(n => n * 100)); // The new state is eagerly computed. expect(Scheduler).toHaveYielded(['Compute state (1 -> 100)']); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index a1e12ca46375..7a6be5385373 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -80,7 +80,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); // Schedule some updates - act(() => { + ReactNoop.batchedUpdates(() => { counter.current.updateCount(1); counter.current.updateCount(count => count + 10); }); @@ -189,11 +189,11 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); act(() => counter.current.updateCount(1)); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); act(() => counter.current.updateCount(count => count + 10)); - expect(Scheduler).toFlushAndYield(['Count: 11']); + expect(Scheduler).toHaveYielded(['Count: 11']); expect(ReactNoop.getChildren()).toEqual([span('Count: 11')]); }); @@ -213,7 +213,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 42')]); act(() => counter.current.updateCount(7)); - expect(Scheduler).toFlushAndYield(['Count: 7']); + expect(Scheduler).toHaveYielded(['Count: 7']); expect(ReactNoop.getChildren()).toEqual([span('Count: 7')]); }); @@ -231,10 +231,10 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); act(() => counter.current.updateCount(7)); - expect(Scheduler).toFlushAndYield(['Count: 7']); + expect(Scheduler).toHaveYielded(['Count: 7']); act(() => counter.current.updateLabel('Total')); - expect(Scheduler).toFlushAndYield(['Total: 7']); + expect(Scheduler).toHaveYielded(['Total: 7']); }); it('returns the same updater function every time', () => { @@ -249,11 +249,11 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); act(() => updaters[0](1)); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); act(() => updaters[0](count => count + 10)); - expect(Scheduler).toFlushAndYield(['Count: 11']); + expect(Scheduler).toHaveYielded(['Count: 11']); expect(ReactNoop.getChildren()).toEqual([span('Count: 11')]); expect(updaters).toEqual([updaters[0], updaters[0], updaters[0]]); @@ -298,7 +298,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); act(() => _updateCount(1)); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); }); }); @@ -484,7 +484,7 @@ describe('ReactHooksWithNoopRenderer', () => { counter.current.dispatch('reset'); }); ReactNoop.render(); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Render: 0', 'Render: 1', 'Render: 11', @@ -524,7 +524,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); act(() => counter.current.dispatch(INCREMENT)); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); act(() => { counter.current.dispatch(DECREMENT); @@ -532,7 +532,7 @@ describe('ReactHooksWithNoopRenderer', () => { counter.current.dispatch(DECREMENT); }); - expect(Scheduler).toFlushAndYield(['Count: -2']); + expect(Scheduler).toHaveYielded(['Count: -2']); expect(ReactNoop.getChildren()).toEqual([span('Count: -2')]); }); @@ -566,7 +566,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 10')]); act(() => counter.current.dispatch(INCREMENT)); - expect(Scheduler).toFlushAndYield(['Count: 11']); + expect(Scheduler).toHaveYielded(['Count: 11']); expect(ReactNoop.getChildren()).toEqual([span('Count: 11')]); act(() => { @@ -575,7 +575,7 @@ describe('ReactHooksWithNoopRenderer', () => { counter.current.dispatch(DECREMENT); }); - expect(Scheduler).toFlushAndYield(['Count: 8']); + expect(Scheduler).toHaveYielded(['Count: 8']); expect(ReactNoop.getChildren()).toEqual([span('Count: 8')]); }); @@ -600,7 +600,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(Scheduler).toFlushAndYield(['Count: 0']); expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]); - act(() => { + ReactNoop.batchedUpdates(() => { counter.current.dispatch(INCREMENT); counter.current.dispatch(INCREMENT); counter.current.dispatch(INCREMENT); @@ -882,7 +882,7 @@ describe('ReactHooksWithNoopRenderer', () => { // Enqueuing this update forces the passive effect to be flushed -- // updateCount(1) happens first, so 2 wins. - act(() => _updateCount(2)); + ReactNoop.batchedUpdates(() => _updateCount(2)); expect(Scheduler).toHaveYielded(['Will set count to 1']); expect(Scheduler).toFlushAndYield(['Count: 2']); expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]); @@ -931,7 +931,7 @@ describe('ReactHooksWithNoopRenderer', () => { // Enqueuing this update forces the passive effect to be flushed -- // updateCount(1) happens first, so 2 wins. - act(() => _updateCount(2)); + ReactNoop.batchedUpdates(() => _updateCount(2)); expect(Scheduler).toHaveYielded(['Will set count to 1']); expect(Scheduler).toFlushAndYield(['Count: 2']); expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]); @@ -1521,7 +1521,7 @@ describe('ReactHooksWithNoopRenderer', () => { ]); act(button.current.increment); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ // Button should not re-render, because its props haven't changed // 'Increment', 'Count: 1', @@ -1545,7 +1545,7 @@ describe('ReactHooksWithNoopRenderer', () => { // Callback should have updated act(button.current.increment); - expect(Scheduler).toFlushAndYield(['Count: 11']); + expect(Scheduler).toHaveYielded(['Count: 11']); expect(ReactNoop.getChildren()).toEqual([ span('Increment'), span('Count: 11'), @@ -1748,7 +1748,7 @@ describe('ReactHooksWithNoopRenderer', () => { act(() => { counter.current.dispatch(INCREMENT); }); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); // Intentionally not updated because of [] deps: expect(counter.current.count).toBe(0); @@ -1778,7 +1778,7 @@ describe('ReactHooksWithNoopRenderer', () => { act(() => { counter.current.dispatch(INCREMENT); }); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); expect(counter.current.count).toBe(1); }); @@ -1815,7 +1815,7 @@ describe('ReactHooksWithNoopRenderer', () => { act(() => { counter.current.dispatch(INCREMENT); }); - expect(Scheduler).toFlushAndYield(['Count: 1']); + expect(Scheduler).toHaveYielded(['Count: 1']); expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); expect(counter.current.count).toBe(1); expect(totalRefUpdates).toBe(2); @@ -1862,7 +1862,7 @@ describe('ReactHooksWithNoopRenderer', () => { updateB(3); }); - expect(Scheduler).toFlushAndYield(['A: 2, B: 3, C: [not loaded]']); + expect(Scheduler).toHaveYielded(['A: 2, B: 3, C: [not loaded]']); expect(ReactNoop.getChildren()).toEqual([ span('A: 2, B: 3, C: [not loaded]'), ]); @@ -1923,7 +1923,7 @@ describe('ReactHooksWithNoopRenderer', () => { updateB(3); updateC(4); }); - expect(Scheduler).toFlushAndYield(['A: 2, B: 3, C: 4']); + expect(Scheduler).toHaveYielded(['A: 2, B: 3, C: 4']); expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 4')]); ReactNoop.render(); expect(Scheduler).toFlushAndThrow( @@ -2029,7 +2029,7 @@ describe('ReactHooksWithNoopRenderer', () => { act(() => { setCounter(2); }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'Render: 1', 'Effect: 2', 'Reducer: 2', @@ -2068,7 +2068,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop).toMatchRenderedOutput('0'); act(() => dispatch()); - expect(Scheduler).toFlushAndYield(['Step: 5, Shadow: 5']); + expect(Scheduler).toHaveYielded(['Step: 5, Shadow: 5']); expect(ReactNoop).toMatchRenderedOutput('5'); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js index 76158a89fe68..ddc62c60aa6f 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js @@ -104,14 +104,14 @@ describe('ReactIncrementalScheduling', () => { ReactNoop.renderToRootWithID(, 'b'); ReactNoop.renderToRootWithID(, 'c'); }); - expect(Scheduler).toFlushAndYield(['a:1', 'b:1', 'c:1']); + expect(Scheduler).toHaveYielded(['a:1', 'b:1', 'c:1']); expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:1'); expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:1'); expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:1'); // Schedule deferred work in the reverse order - ReactNoop.act(() => { + ReactNoop.batchedUpdates(() => { ReactNoop.renderToRootWithID(, 'c'); ReactNoop.renderToRootWithID(, 'b'); }); @@ -122,7 +122,7 @@ describe('ReactIncrementalScheduling', () => { 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.act(() => { + ReactNoop.batchedUpdates(() => { ReactNoop.renderToRootWithID(, 'a'); }); // Keep performing work in the order it was scheduled diff --git a/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js b/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js index f3aba9b99881..04f7febe843e 100644 --- a/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js +++ b/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js @@ -42,6 +42,7 @@ describe('ReactNoop.act()', () => { Scheduler.yieldValue('stage 1'); await null; Scheduler.yieldValue('stage 2'); + await null; setCtr(1); } React.useEffect(() => { @@ -50,13 +51,9 @@ describe('ReactNoop.act()', () => { return ctr; } await ReactNoop.act(async () => { - ReactNoop.act(() => { - ReactNoop.render(); - }); - await null; - expect(Scheduler).toFlushAndYield(['stage 1']); + ReactNoop.render(); }); - expect(Scheduler).toHaveYielded(['stage 2']); + expect(Scheduler).toHaveYielded(['stage 1', 'stage 2']); expect(Scheduler).toFlushWithoutYielding(); expect(ReactNoop.getChildren()).toEqual([{text: '1', hidden: false}]); }); diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js index 37ced3fb04c0..12d9b5a094cd 100644 --- a/packages/react-test-renderer/src/ReactTestRendererAct.js +++ b/packages/react-test-renderer/src/ReactTestRendererAct.js @@ -8,13 +8,11 @@ */ import type {Thenable} from 'react-reconciler/src/ReactFiberScheduler'; -import { - batchedUpdates, - flushPassiveEffects, -} from 'react-reconciler/inline.test'; +import {batchedUpdates, hasPendingEffects} from 'react-reconciler/inline.test'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import warningWithoutStack from 'shared/warningWithoutStack'; import enqueueTask from 'shared/enqueueTask'; +import * as Scheduler from 'scheduler'; const {ReactShouldWarnActingUpdates} = ReactSharedInternals; @@ -27,9 +25,9 @@ let actingUpdatesScopeDepth = 0; function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { try { - flushPassiveEffects(); + Scheduler.unstable_flushWithoutYielding(); enqueueTask(() => { - if (flushPassiveEffects()) { + if (hasPendingEffects()) { flushEffectsAndMicroTasks(onDone); } else { onDone(); @@ -128,7 +126,7 @@ function act(callback: () => Thenable) { // flush effects until none remain, and cleanup try { - while (flushPassiveEffects()) {} + Scheduler.unstable_flushWithoutYielding(); onDone(); } catch (err) { onDone(); From 5e5a68bb16ec1105d158918f59204dbf416f5563 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 8 May 2019 18:23:47 +0100 Subject: [PATCH 02/15] ugh lint --- packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js index 3e6f96afad15..c7d73fd357cb 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js @@ -13,7 +13,6 @@ let ReactTestUtils; let SchedulerTracing; let act; let container; -let Scheduler; jest.useRealTimers(); @@ -32,7 +31,6 @@ describe('ReactTestUtils.act()', () => { ReactDOM = require('react-dom'); ReactTestUtils = require('react-dom/test-utils'); SchedulerTracing = require('scheduler/tracing'); - Scheduler = require('scheduler'); act = ReactTestUtils.act; container = document.createElement('div'); document.body.appendChild(container); @@ -521,7 +519,6 @@ describe('ReactTestUtils.act()', () => { act(() => { ReactDOM.unstable_createRoot(el).render(); }); - // Scheduler.flushAll(); expect(el.innerHTML).toBe('5'); }); }); From 37989a5de5866766e999a805a07f98b971446fb8 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 8 May 2019 20:28:41 +0100 Subject: [PATCH 03/15] pass build, flushPassiveEffects returns nothing now --- packages/react-reconciler/src/ReactFiberScheduler.js | 4 +--- packages/shared/forks/Scheduler.umd.js | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 55e1f0b05e55..10a3c2c6706e 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -1604,7 +1604,7 @@ function commitLayoutEffects( export function flushPassiveEffects() { if (rootWithPendingPassiveEffects === null) { - return false; + return; } const root = rootWithPendingPassiveEffects; const expirationTime = pendingPassiveEffectsExpirationTime; @@ -1661,8 +1661,6 @@ export function flushPassiveEffects() { // exceeds the limit, we'll fire a warning. nestedPassiveUpdateCount = rootWithPendingPassiveEffects === null ? 0 : nestedPassiveUpdateCount + 1; - - return true; } export function hasPendingEffects() { diff --git a/packages/shared/forks/Scheduler.umd.js b/packages/shared/forks/Scheduler.umd.js index b1a77ce9232d..5a9fc0ef8738 100644 --- a/packages/shared/forks/Scheduler.umd.js +++ b/packages/shared/forks/Scheduler.umd.js @@ -28,6 +28,7 @@ const { unstable_LowPriority, unstable_IdlePriority, unstable_forceFrameRate, + unstable_flushWithoutYielding, } = ReactInternals.Scheduler; export { @@ -47,4 +48,5 @@ export { unstable_LowPriority, unstable_IdlePriority, unstable_forceFrameRate, + unstable_flushWithoutYielding, }; From f5fd513627f2f232d80db3c1ae3ba535f9f4fb39 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 9 May 2019 10:50:44 +0100 Subject: [PATCH 04/15] pass test-fire --- packages/react-dom/src/fire/ReactFire.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/fire/ReactFire.js b/packages/react-dom/src/fire/ReactFire.js index e4f555ebd627..28e8d32fd63c 100644 --- a/packages/react-dom/src/fire/ReactFire.js +++ b/packages/react-dom/src/fire/ReactFire.js @@ -40,7 +40,7 @@ import { getPublicRootInstance, findHostInstance, findHostInstanceWithWarning, - flushPassiveEffects, + hasPendingEffects, } from 'react-reconciler/inline.fire'; import {createPortal as createPortalImpl} from 'shared/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -827,7 +827,7 @@ const ReactDOM: Object = { restoreStateIfNeeded, dispatchEvent, runEventsInBatch, - flushPassiveEffects, + hasPendingEffects, ], }, }; From 5003233a62054b70a837f1cfb93b2311d658eea6 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 9 May 2019 12:47:37 +0100 Subject: [PATCH 05/15] flush all work (not just effects), add a compatibility mode of note, unstable_flushWithoutYielding now returns a boolean much like flushPassiveEffects --- packages/react-dom/src/client/ReactDOM.js | 4 +- packages/react-dom/src/fire/ReactFire.js | 4 +- .../src/test-utils/ReactTestUtils.js | 2 +- .../src/test-utils/ReactTestUtilsAct.js | 41 ++++++++++++----- .../src/createReactNoop.js | 40 ++++++++++++----- .../src/ReactFiberReconciler.js | 2 - .../src/ReactFiberScheduler.js | 6 +-- .../src/ReactTestRendererAct.js | 44 ++++++++++++++----- .../src/forks/SchedulerHostConfig.mock.js | 6 ++- 9 files changed, 107 insertions(+), 42 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index f473b09057b0..728f775adf5e 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -35,7 +35,7 @@ import { getPublicRootInstance, findHostInstance, findHostInstanceWithWarning, - hasPendingEffects, + flushPassiveEffects, } from 'react-reconciler/inline.dom'; import {createPortal as createPortalImpl} from 'shared/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -821,7 +821,7 @@ const ReactDOM: Object = { restoreStateIfNeeded, dispatchEvent, runEventsInBatch, - hasPendingEffects, + flushPassiveEffects, ], }, }; diff --git a/packages/react-dom/src/fire/ReactFire.js b/packages/react-dom/src/fire/ReactFire.js index 28e8d32fd63c..e4f555ebd627 100644 --- a/packages/react-dom/src/fire/ReactFire.js +++ b/packages/react-dom/src/fire/ReactFire.js @@ -40,7 +40,7 @@ import { getPublicRootInstance, findHostInstance, findHostInstanceWithWarning, - hasPendingEffects, + flushPassiveEffects, } from 'react-reconciler/inline.fire'; import {createPortal as createPortalImpl} from 'shared/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -827,7 +827,7 @@ const ReactDOM: Object = { restoreStateIfNeeded, dispatchEvent, runEventsInBatch, - hasPendingEffects, + flushPassiveEffects, ], }, }; diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index a8abfc58f807..f463cc4aa6a3 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -43,7 +43,7 @@ const [ dispatchEvent, runEventsInBatch, // eslint-disable-next-line no-unused-vars - hasPendingEffects, + flushPassiveEffects, ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; function Event(suffix) {} diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js index 4a5e5ac1ae4f..61f54c3745a9 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -31,7 +31,7 @@ const [ dispatchEvent, runEventsInBatch, /* eslint-enable no-unused-vars */ - hasPendingEffects, + flushPassiveEffects, ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; const batchedUpdates = ReactDOM.unstable_batchedUpdates; @@ -41,16 +41,32 @@ const {ReactShouldWarnActingUpdates} = ReactSharedInternals; // this implementation should be exactly the same in // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js -// we track the 'depth' of the act() calls with this counter, -// so we can tell if any async act() calls try to run in parallel. -let actingUpdatesScopeDepth = 0; +let hasWarnedAboutMissingMockScheduler = false; +const flushWork = + Scheduler.unstable_flushWithoutYielding || + function() { + if (!hasWarnedAboutMissingMockScheduler) { + warningWithoutStack( + null, + 'Starting from React v17, the "scheduler" module will need to be mocked ' + + 'to guarantee consistent behaviour across tests and browsers. To fix this, add the following ' + + "to the top of your tests, or in your framework's global config file -\n\n" + + 'As an example, for jest - \n' + + "jest.mock('scheduler', () => require.requireActual('scheduler/unstable_mock'));\n\n" + + 'For more info, visit https://fb.me/react-mock-scheduler', + ); + hasWarnedAboutMissingMockScheduler = true; + } -function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { + while (flushPassiveEffects()) {} + }; + +function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) { try { - Scheduler.unstable_flushWithoutYielding(); + flushWork(); enqueueTask(() => { - if (hasPendingEffects()) { - flushEffectsAndMicroTasks(onDone); + if (flushWork()) { + flushWorkAndMicroTasks(onDone); } else { onDone(); } @@ -60,6 +76,11 @@ function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { } } +// we track the 'depth' of the act() calls with this counter, +// so we can tell if any async act() calls try to run in parallel. + +let actingUpdatesScopeDepth = 0; + function act(callback: () => Thenable) { let previousActingUpdatesScopeDepth; if (__DEV__) { @@ -120,7 +141,7 @@ function act(callback: () => Thenable) { called = true; result.then( () => { - flushEffectsAndMicroTasks((err: ?Error) => { + flushWorkAndMicroTasks((err: ?Error) => { onDone(); if (err) { reject(err); @@ -148,7 +169,7 @@ function act(callback: () => Thenable) { // flush effects until none remain, and cleanup try { - Scheduler.unstable_flushWithoutYielding(); + flushWork(); onDone(); } catch (err) { onDone(); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 1f79342a088b..8b02df86dc6f 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -645,21 +645,36 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { const roots = new Map(); const DEFAULT_ROOT_ID = ''; - const {hasPendingEffects, batchedUpdates} = NoopRenderer; + const {flushPassiveEffects, batchedUpdates} = NoopRenderer; // this act() implementation should be exactly the same in // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js + let hasWarnedAboutMissingMockScheduler = false; + const flushWork = + Scheduler.unstable_flushWithoutYielding || + function() { + if (!hasWarnedAboutMissingMockScheduler) { + warningWithoutStack( + null, + 'Starting from React v17, the "scheduler" module will need to be mocked ' + + 'to guarantee consistent behaviour across tests and browsers. To fix this, add the following ' + + "to the top of your tests, or in your framework's global config file -\n\n" + + 'As an example, for jest - \n' + + "jest.mock('scheduler', () => require.requireActual('scheduler/unstable_mock'));\n\n" + + 'For more info, visit https://fb.me/react-mock-scheduler', + ); + hasWarnedAboutMissingMockScheduler = true; + } - // we track the 'depth' of the act() calls with this counter, - // so we can tell if any async act() calls try to run in parallel. - let actingUpdatesScopeDepth = 0; + while (flushPassiveEffects()) {} + }; - function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { + function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) { try { - Scheduler.unstable_flushWithoutYielding(); + flushWork(); enqueueTask(() => { - if (hasPendingEffects()) { - flushEffectsAndMicroTasks(onDone); + if (flushWork()) { + flushWorkAndMicroTasks(onDone); } else { onDone(); } @@ -669,6 +684,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { } } + // we track the 'depth' of the act() calls with this counter, + // so we can tell if any async act() calls try to run in parallel. + + let actingUpdatesScopeDepth = 0; + function act(callback: () => Thenable) { let previousActingUpdatesScopeDepth; if (__DEV__) { @@ -729,7 +749,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { called = true; result.then( () => { - flushEffectsAndMicroTasks((err: ?Error) => { + flushWorkAndMicroTasks((err: ?Error) => { onDone(); if (err) { reject(err); @@ -757,7 +777,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // flush effects until none remain, and cleanup try { - Scheduler.unstable_flushWithoutYielding(); + flushWork(); onDone(); } catch (err) { onDone(); diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index dcfe248cedd0..77fadd31e9b2 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -53,7 +53,6 @@ import { interactiveUpdates, flushInteractiveUpdates, flushPassiveEffects, - hasPendingEffects, } from './ReactFiberScheduler'; import {createUpdate, enqueueUpdate} from './ReactUpdateQueue'; import ReactFiberInstrumentation from './ReactFiberInstrumentation'; @@ -310,7 +309,6 @@ export { flushControlled, flushSync, flushPassiveEffects, - hasPendingEffects, }; export function getPublicRootInstance( diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 10a3c2c6706e..f748228f8eb0 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -1604,7 +1604,7 @@ function commitLayoutEffects( export function flushPassiveEffects() { if (rootWithPendingPassiveEffects === null) { - return; + return false; } const root = rootWithPendingPassiveEffects; const expirationTime = pendingPassiveEffectsExpirationTime; @@ -1661,10 +1661,8 @@ export function flushPassiveEffects() { // exceeds the limit, we'll fire a warning. nestedPassiveUpdateCount = rootWithPendingPassiveEffects === null ? 0 : nestedPassiveUpdateCount + 1; -} -export function hasPendingEffects() { - return rootWithPendingPassiveEffects !== null; + return true; } export function isAlreadyFailedLegacyErrorBoundary(instance: mixed): boolean { diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js index 12d9b5a094cd..a7c3524030cf 100644 --- a/packages/react-test-renderer/src/ReactTestRendererAct.js +++ b/packages/react-test-renderer/src/ReactTestRendererAct.js @@ -8,7 +8,10 @@ */ import type {Thenable} from 'react-reconciler/src/ReactFiberScheduler'; -import {batchedUpdates, hasPendingEffects} from 'react-reconciler/inline.test'; +import { + batchedUpdates, + flushPassiveEffects, +} from 'react-reconciler/inline.test'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import warningWithoutStack from 'shared/warningWithoutStack'; import enqueueTask from 'shared/enqueueTask'; @@ -19,16 +22,32 @@ const {ReactShouldWarnActingUpdates} = ReactSharedInternals; // this implementation should be exactly the same in // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js -// we track the 'depth' of the act() calls with this counter, -// so we can tell if any async act() calls try to run in parallel. -let actingUpdatesScopeDepth = 0; +let hasWarnedAboutMissingMockScheduler = false; +const flushWork = + Scheduler.unstable_flushWithoutYielding || + function() { + if (!hasWarnedAboutMissingMockScheduler) { + warningWithoutStack( + null, + 'Starting from React v17, the "scheduler" module will need to be mocked ' + + 'to guarantee consistent behaviour across tests and browsers. To fix this, add the following ' + + "to the top of your tests, or in your framework's global config file -\n\n" + + 'As an example, for jest - \n' + + "jest.mock('scheduler', () => require.requireActual('scheduler/unstable_mock'));\n\n" + + 'For more info, visit https://fb.me/react-mock-scheduler', + ); + hasWarnedAboutMissingMockScheduler = true; + } -function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { + while (flushPassiveEffects()) {} + }; + +function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) { try { - Scheduler.unstable_flushWithoutYielding(); + flushWork(); enqueueTask(() => { - if (hasPendingEffects()) { - flushEffectsAndMicroTasks(onDone); + if (flushWork()) { + flushWorkAndMicroTasks(onDone); } else { onDone(); } @@ -38,6 +57,11 @@ function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { } } +// we track the 'depth' of the act() calls with this counter, +// so we can tell if any async act() calls try to run in parallel. + +let actingUpdatesScopeDepth = 0; + function act(callback: () => Thenable) { let previousActingUpdatesScopeDepth; if (__DEV__) { @@ -98,7 +122,7 @@ function act(callback: () => Thenable) { called = true; result.then( () => { - flushEffectsAndMicroTasks((err: ?Error) => { + flushWorkAndMicroTasks((err: ?Error) => { onDone(); if (err) { reject(err); @@ -126,7 +150,7 @@ function act(callback: () => Thenable) { // flush effects until none remain, and cleanup try { - Scheduler.unstable_flushWithoutYielding(); + flushWork(); onDone(); } catch (err) { onDone(); diff --git a/packages/scheduler/src/forks/SchedulerHostConfig.mock.js b/packages/scheduler/src/forks/SchedulerHostConfig.mock.js index 0e2641c2c3be..4ecd682ff847 100644 --- a/packages/scheduler/src/forks/SchedulerHostConfig.mock.js +++ b/packages/scheduler/src/forks/SchedulerHostConfig.mock.js @@ -103,12 +103,15 @@ export function unstable_flushExpired() { } } -export function unstable_flushWithoutYielding(): void { +export function unstable_flushWithoutYielding(): boolean { if (isFlushing) { throw new Error('Already flushing work.'); } isFlushing = true; try { + if (scheduledCallback === null) { + return false; + } while (scheduledCallback !== null) { const cb = scheduledCallback; scheduledCallback = null; @@ -117,6 +120,7 @@ export function unstable_flushWithoutYielding(): void { scheduledCallbackExpiration <= currentTime; cb(didTimeout); } + return true; } finally { expectedNumberOfYields = -1; didStop = false; From 7de2164077b12850c2793dc2034d983b0e023206 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 9 May 2019 14:57:04 +0100 Subject: [PATCH 06/15] umd build for scheduler/unstable_mock, pass the fixture with it --- fixtures/dom/.gitignore | 2 ++ fixtures/dom/package.json | 2 +- fixtures/dom/public/act-dom.html | 4 ++++ packages/react-dom/src/test-utils/ReactTestUtilsAct.js | 2 +- packages/react-noop-renderer/src/createReactNoop.js | 3 ++- packages/react-test-renderer/src/ReactTestRendererAct.js | 2 +- scripts/rollup/bundles.js | 9 ++++++++- 7 files changed, 19 insertions(+), 5 deletions(-) diff --git a/fixtures/dom/.gitignore b/fixtures/dom/.gitignore index 724d1459422c..3473f8a18efa 100644 --- a/fixtures/dom/.gitignore +++ b/fixtures/dom/.gitignore @@ -8,6 +8,8 @@ coverage # production build +public/scheduler-unstable_mock.development.js +public/scheduler-unstable_mock.production.min.js public/react.development.js public/react.production.min.js public/react-dom.development.js diff --git a/fixtures/dom/package.json b/fixtures/dom/package.json index f5f84257f7c8..3282c85fa58a 100644 --- a/fixtures/dom/package.json +++ b/fixtures/dom/package.json @@ -18,7 +18,7 @@ }, "scripts": { "start": "react-scripts start", - "prestart": "cp ../../build/node_modules/react/umd/react.development.js ../../build/node_modules/react-dom/umd/react-dom.development.js ../../build/node_modules/react/umd/react.production.min.js ../../build/node_modules/react-dom/umd/react-dom.production.min.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.production.min.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.development.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.production.min.js public/", + "prestart": "cp ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.development.js ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.production.min.js ../../build/node_modules/react/umd/react.development.js ../../build/node_modules/react-dom/umd/react-dom.development.js ../../build/node_modules/react/umd/react.production.min.js ../../build/node_modules/react-dom/umd/react-dom.production.min.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.production.min.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.development.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.production.min.js public/", "build": "react-scripts build && cp build/index.html build/200.html", "test": "react-scripts test --env=jsdom", "eject": "react-scripts eject" diff --git a/fixtures/dom/public/act-dom.html b/fixtures/dom/public/act-dom.html index 2fb4a437721d..2100026a5ba4 100644 --- a/fixtures/dom/public/act-dom.html +++ b/fixtures/dom/public/act-dom.html @@ -7,7 +7,11 @@ this page tests whether act runs properly in a browser.
your console should say "5" + +