Skip to content

Commit

Permalink
Cleanup tests using runWithPriority. (facebook#20958)
Browse files Browse the repository at this point in the history
* Remove Scheduler.runWithPriority from some tests

* Mark experimental test experimental
  • Loading branch information
rickhanlonii authored and koto committed Jun 15, 2021
1 parent c6b51bb commit 4be4c7d
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(<App text="Hi" className="hi" />);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3166,10 +3166,7 @@ describe('ReactHooksWithNoopRenderer', () => {
]);

await act(async () => {
Scheduler.unstable_runWithPriority(
Scheduler.unstable_UserBlockingPriority,
transition,
);
transition();

expect(Scheduler).toFlushAndYield([
'Before... Pending: true',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand All @@ -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([
Expand Down Expand Up @@ -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');
}
Expand All @@ -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([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});
});

Expand All @@ -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());
});
});

Expand Down Expand Up @@ -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]']);
Expand Down Expand Up @@ -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));
});
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
16 changes: 7 additions & 9 deletions packages/react/src/__tests__/ReactProfiler-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -767,15 +768,12 @@ describe('Profiler', () => {
const onRender = jest.fn();

// Schedule low-priority work.
Scheduler.unstable_runWithPriority(
Scheduler.unstable_LowPriority,
() => {
ReactNoop.render(
<React.Profiler id="root" onRender={onRender}>
<Component />
</React.Profiler>,
);
},
React.unstable_startTransition(() =>
ReactNoop.render(
<React.Profiler id="root" onRender={onRender}>
<Component />
</React.Profiler>,
),
);

// Flush sync work with a nested update
Expand Down

0 comments on commit 4be4c7d

Please sign in to comment.