From b989ee82695ac46fcb8e2c6371caef197373f021 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Mon, 8 Mar 2021 21:07:29 -0700 Subject: [PATCH 1/2] Remove Scheduler.runWithPriority from some tests --- ...DOMServerPartialHydration-test.internal.js | 7 +++- ...MServerSelectiveHydration-test.internal.js | 7 +++- .../src/__tests__/ReactExpiration-test.js | 6 +--- .../ReactHooksWithNoopRenderer-test.js | 5 +-- .../__tests__/ReactIncrementalUpdates-test.js | 32 +++---------------- .../ReactSuspenseWithNoopRenderer-test.js | 20 +++--------- .../useMutableSource-test.internal.js | 29 ++++++----------- .../ReactDOMTracing-test.internal.js | 6 +--- .../__tests__/ReactProfiler-test.internal.js | 15 ++++----- 9 files changed, 39 insertions(+), 88 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 2ad221d5fbcb6..91f6e8f022727 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -18,6 +18,11 @@ let Suspense; let SuspenseList; let act; +// Copied from ReactFiberLanes. Don't do this! +// This is hard coded directly to avoid needing to import, and +// we'll remove this as we replace runWithPriority with React APIs. +export const IdleLanePriority = 2; + function dispatchMouseEvent(to, from) { if (!to) { to = null; @@ -623,7 +628,7 @@ describe('ReactDOMServerPartialHydration', () => { expect(span.textContent).toBe('Hello'); // Schedule an update at idle priority - Scheduler.unstable_runWithPriority(Scheduler.unstable_IdlePriority, () => { + ReactDOM.unstable_runWithPriority(IdleLanePriority, () => { root.render(); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 19cf6065e6b9d..96adf85ba35a8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -19,6 +19,11 @@ let Scheduler; let Suspense; let act; +// Copied from ReactFiberLanes. Don't do this! +// This is hard coded directly to avoid needing to import, and +// we'll remove this as we replace runWithPriority with React APIs. +export const IdleLanePriority = 2; + function dispatchMouseHoverEvent(to, from) { if (!to) { to = null; @@ -96,7 +101,7 @@ function dispatchClickEvent(target) { // and there's no native DOM event that maps to idle priority, so this is a // temporary workaround. Need something like ReactDOM.unstable_IdleUpdates. function TODO_scheduleIdleDOMSchedulerTask(fn) { - Scheduler.unstable_runWithPriority(Scheduler.unstable_IdlePriority, () => { + ReactDOM.unstable_runWithPriority(IdleLanePriority, () => { const prevEvent = window.event; window.event = {type: 'message'}; try { diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index 48ff50880f943..ed49965318212 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -550,11 +550,7 @@ describe('ReactExpiration', () => { function App() { const [highPri, setHighPri] = useState(0); const [normalPri, setNormalPri] = useState(0); - updateHighPri = () => - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => setHighPri(n => n + 1), - ); + updateHighPri = () => ReactNoop.flushSync(() => setHighPri(n => n + 1)); updateNormalPri = () => setNormalPri(n => n + 1); return ( <> diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 4c7d308a9c909..cf4e4a3f3b619 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3166,10 +3166,7 @@ describe('ReactHooksWithNoopRenderer', () => { ]); await act(async () => { - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - transition, - ); + transition(); expect(Scheduler).toFlushAndYield([ 'Before... Pending: true', diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js index e24e2576dde2f..95acb7d8fc778 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js @@ -530,14 +530,8 @@ describe('ReactIncrementalUpdates', () => { Scheduler.unstable_yieldValue('Committed: ' + log); if (log === 'B') { // Right after B commits, schedule additional updates. - // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority. ReactNoop.unstable_runWithPriority(InputContinuousLanePriority, () => - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => { - pushToLog('C'); - }, - ), + pushToLog('C'), ); setLog(prevLog => prevLog + 'D'); } @@ -556,14 +550,8 @@ describe('ReactIncrementalUpdates', () => { await ReactNoop.act(async () => { pushToLog('A'); - // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority. ReactNoop.unstable_runWithPriority(InputContinuousLanePriority, () => - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => { - pushToLog('B'); - }, - ), + pushToLog('B'), ); }); expect(Scheduler).toHaveYielded([ @@ -595,14 +583,8 @@ describe('ReactIncrementalUpdates', () => { Scheduler.unstable_yieldValue('Committed: ' + this.state.log); if (this.state.log === 'B') { // Right after B commits, schedule additional updates. - // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority. ReactNoop.unstable_runWithPriority(InputContinuousLanePriority, () => - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => { - this.pushToLog('C'); - }, - ), + this.pushToLog('C'), ); this.pushToLog('D'); } @@ -622,14 +604,8 @@ describe('ReactIncrementalUpdates', () => { await ReactNoop.act(async () => { pushToLog('A'); - // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority. ReactNoop.unstable_runWithPriority(InputContinuousLanePriority, () => - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => { - pushToLog('B'); - }, - ), + pushToLog('B'), ); }); expect(Scheduler).toHaveYielded([ diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 8ec621be0d015..d7da84e67b752 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -1929,10 +1929,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { // TODO: assert toErrorDev() when the warning is implemented again. ReactNoop.act(() => { - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => _setShow(true), - ); + ReactNoop.flushSync(() => _setShow(true)); }); }); @@ -1959,10 +1956,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { // TODO: assert toErrorDev() when the warning is implemented again. ReactNoop.act(() => { - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => show(), - ); + ReactNoop.flushSync(() => show()); }); }); @@ -1991,10 +1985,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop).toMatchRenderedOutput('Loading...'); ReactNoop.act(() => { - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => showB(), - ); + ReactNoop.flushSync(() => showB()); }); expect(Scheduler).toHaveYielded(['Suspend! [A]', 'Suspend! [B]']); @@ -2025,10 +2016,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { // TODO: assert toErrorDev() when the warning is implemented again. ReactNoop.act(() => { - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => _setShow(true), - ); + ReactNoop.flushSync(() => _setShow(true)); }); }, ); diff --git a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js index cf1db2891e11d..e9fc689850233 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js +++ b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js @@ -1476,13 +1476,10 @@ describe('useMutableSource', () => { // read during render will happen to match the latest value. But it should // still entangle the updates to prevent the previous update (a1) from // rendering by itself. - Scheduler.unstable_runWithPriority( - Scheduler.unstable_IdlePriority, - () => { - mutateA('a0'); - mutateB('b0'); - }, - ); + React.unstable_startTransition(() => { + mutateA('a0'); + mutateB('b0'); + }); // Finish the current render expect(Scheduler).toFlushUntilNextPaint(['c']); // a0 will re-render because of the mutation update. But it should show @@ -1601,12 +1598,9 @@ describe('useMutableSource', () => { // Mutate the config. This is at lower priority so that 1) to make sure // it doesn't happen to get batched with the in-progress render, and 2) // so it doesn't interrupt the in-progress render. - Scheduler.unstable_runWithPriority( - Scheduler.unstable_IdlePriority, - () => { - source.valueB = '3'; - }, - ); + React.unstable_startTransition(() => { + source.valueB = '3'; + }); expect(Scheduler).toFlushAndYieldThrough([ // The partial render completes @@ -1698,12 +1692,9 @@ describe('useMutableSource', () => { expect(source.listenerCount).toBe(1); // Mutate -> schedule update for ComponentA - Scheduler.unstable_runWithPriority( - Scheduler.unstable_IdlePriority, - () => { - source.value = 'two'; - }, - ); + React.unstable_startTransition(() => { + source.value = 'two'; + }); // Commit ComponentB -> notice the change and schedule an update for ComponentB expect(Scheduler).toFlushAndYield(['a:two', 'b:two']); diff --git a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js index 114b0cfffee53..479e229d91153 100644 --- a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js +++ b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js @@ -246,12 +246,8 @@ describe('ReactDOMTracing', () => { Scheduler.unstable_yieldValue('Child:update'); } else { Scheduler.unstable_yieldValue('Child:mount'); - // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority. ReactDOM.unstable_runWithPriority(IdleLanePriority, () => - Scheduler.unstable_runWithPriority( - Scheduler.unstable_IdlePriority, - () => setDidMount(true), - ), + setDidMount(true), ); } }, [didMount]); diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index d6e44ab820960..1ad9bb551258b 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -767,15 +767,12 @@ describe('Profiler', () => { const onRender = jest.fn(); // Schedule low-priority work. - Scheduler.unstable_runWithPriority( - Scheduler.unstable_LowPriority, - () => { - ReactNoop.render( - - - , - ); - }, + React.unstable_startTransition(() => + ReactNoop.render( + + + , + ), ); // Flush sync work with a nested update From 6eddfd90006772eca555964de1ca20ad80b0c339 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Tue, 9 Mar 2021 15:23:31 -0700 Subject: [PATCH 2/2] Mark experimental test experimental --- packages/react/src/__tests__/ReactProfiler-test.internal.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 1ad9bb551258b..93347b1ac2f9f 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -748,6 +748,7 @@ describe('Profiler', () => { expect(onRender.mock.calls[2][1]).toBe('update'); }); + // @gate experimental it('is properly distinguish updates and nested-updates when there is more than sync remaining work', () => { loadModules({ enableSchedulerTracing,