Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cleanup tests using runWithPriority. #20958

Merged
merged 2 commits into from
Mar 10, 2021
Merged

Cleanup tests using runWithPriority. #20958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b989ee8
Remove Scheduler.runWithPriority from some tests
rickhanlonii Mar 9, 2021
6eddfd9
Mark experimental test experimental
rickhanlonii Mar 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Open in desktop
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.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this but it's the way we did it elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we have Offscreen we could use that instead.

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
7 changes: 6 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js
View file
Open in desktop
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
6 changes: 1 addition & 5 deletions packages/react-reconciler/src/__tests__/ReactExpiration-test.js
View file
Open in desktop
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
5 changes: 1 addition & 4 deletions packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
View file
Open in desktop
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
32 changes: 4 additions & 28 deletions packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
View file
Open in desktop
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
20 changes: 4 additions & 16 deletions packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
View file
Open in desktop
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));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted as many of these that I could to either flushSync or startTransition.

After the default event changes land we can probably clean up a lot more.

});
});

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
29 changes: 10 additions & 19 deletions packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js
View file
Open in desktop
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
6 changes: 1 addition & 5 deletions packages/react/src/__tests__/ReactDOMTracing-test.internal.js
View file
Open in desktop
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
View file
Open in desktop
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