Skip to content

Commit e0bbc26

Browse files
authored
Improve tests that deal with microtasks (#26493)
I rewrote some of our tests that deal with microtasks with the aim of making them less coupled to implementation details. This is related to an upcoming change to move update processing into a microtask.
1 parent 8faf751 commit e0bbc26

File tree

4 files changed

+92
-13
lines changed

4 files changed

+92
-13
lines changed

packages/internal-test-utils/ReactInternalTestUtils.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,30 @@ ${diff(expectedLog, actualLog)}
218218
throw error;
219219
}
220220

221+
export async function waitForDiscrete(expectedLog) {
222+
assertYieldsWereCleared(SchedulerMock);
223+
224+
// Create the error object before doing any async work, to get a better
225+
// stack trace.
226+
const error = new Error();
227+
Error.captureStackTrace(error, waitForDiscrete);
228+
229+
// Wait until end of current task/microtask.
230+
await waitForMicrotasks();
231+
232+
const actualLog = SchedulerMock.unstable_clearLog();
233+
if (equals(actualLog, expectedLog)) {
234+
return;
235+
}
236+
237+
error.message = `
238+
Expected sequence of events did not occur.
239+
240+
${diff(expectedLog, actualLog)}
241+
`;
242+
throw error;
243+
}
244+
221245
export function assertLog(expectedLog) {
222246
const actualLog = SchedulerMock.unstable_clearLog();
223247
if (equals(actualLog, expectedLog)) {

packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,33 @@ let React;
1313

1414
let ReactDOMClient;
1515
let act;
16+
let assertLog;
17+
let Scheduler;
1618

1719
describe('ReactDOMSafariMicrotaskBug-test', () => {
1820
let container;
19-
let flushMicrotasksPrematurely;
21+
let overrideQueueMicrotask;
22+
let flushFakeMicrotasks;
2023

2124
beforeEach(() => {
2225
// In Safari, microtasks don't always run on clean stack.
2326
// This setup crudely approximates it.
2427
// In reality, the sync flush happens when an iframe is added to the page.
2528
// https://github.com/facebook/react/issues/22459
26-
let queue = [];
27-
window.queueMicrotask = function (cb) {
28-
queue.push(cb);
29+
const originalQueueMicrotask = queueMicrotask;
30+
overrideQueueMicrotask = false;
31+
const fakeMicrotaskQueue = [];
32+
global.queueMicrotask = cb => {
33+
if (overrideQueueMicrotask) {
34+
fakeMicrotaskQueue.push(cb);
35+
} else {
36+
originalQueueMicrotask(cb);
37+
}
2938
};
30-
flushMicrotasksPrematurely = function () {
31-
while (queue.length > 0) {
32-
const prevQueue = queue;
33-
queue = [];
34-
prevQueue.forEach(cb => cb());
39+
flushFakeMicrotasks = () => {
40+
while (fakeMicrotaskQueue.length > 0) {
41+
const cb = fakeMicrotaskQueue.shift();
42+
cb();
3543
}
3644
};
3745

@@ -40,6 +48,8 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
4048
React = require('react');
4149
ReactDOMClient = require('react-dom/client');
4250
act = require('internal-test-utils').act;
51+
assertLog = require('internal-test-utils').assertLog;
52+
Scheduler = require('scheduler');
4353

4454
document.body.appendChild(container);
4555
});
@@ -55,10 +65,14 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
5565
return (
5666
<div
5767
ref={() => {
68+
overrideQueueMicrotask = true;
5869
if (!ran) {
5970
ran = true;
6071
setState(1);
61-
flushMicrotasksPrematurely();
72+
flushFakeMicrotasks();
73+
Scheduler.log(
74+
'Content at end of ref callback: ' + container.textContent,
75+
);
6276
}
6377
}}>
6478
{state}
@@ -69,6 +83,7 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
6983
await act(() => {
7084
root.render(<Foo />);
7185
});
86+
assertLog(['Content at end of ref callback: 0']);
7287
expect(container.textContent).toBe('1');
7388
});
7489

@@ -78,8 +93,12 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
7893
return (
7994
<button
8095
onClick={() => {
96+
overrideQueueMicrotask = true;
8197
setState(1);
82-
flushMicrotasksPrematurely();
98+
flushFakeMicrotasks();
99+
Scheduler.log(
100+
'Content at end of click handler: ' + container.textContent,
101+
);
83102
}}>
84103
{state}
85104
</button>
@@ -95,6 +114,11 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
95114
new MouseEvent('click', {bubbles: true}),
96115
);
97116
});
117+
// This causes the update to flush earlier than usual. This isn't the ideal
118+
// behavior but we use this test to document it. The bug is Safari's, not
119+
// ours, so we just do our best to not crash even though the behavior isn't
120+
// completely correct.
121+
assertLog(['Content at end of click handler: 1']);
98122
expect(container.textContent).toBe('1');
99123
});
100124
});

packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ let ReactFeatureFlags;
1616
let Scheduler;
1717
let act;
1818
let waitForAll;
19+
let waitForDiscrete;
1920
let assertLog;
2021

2122
const setUntrackedChecked = Object.getOwnPropertyDescriptor(
@@ -65,6 +66,7 @@ describe('ChangeEventPlugin', () => {
6566

6667
const InternalTestUtils = require('internal-test-utils');
6768
waitForAll = InternalTestUtils.waitForAll;
69+
waitForDiscrete = InternalTestUtils.waitForDiscrete;
6870
assertLog = InternalTestUtils.assertLog;
6971

7072
container = document.createElement('div');
@@ -730,8 +732,7 @@ describe('ChangeEventPlugin', () => {
730732
);
731733

732734
// Flush microtask queue.
733-
await null;
734-
assertLog(['render: ']);
735+
await waitForDiscrete(['render: ']);
735736
expect(input.value).toBe('');
736737
});
737738

packages/react-reconciler/src/__tests__/ReactFlushSyncNoAggregateError-test.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ let act;
55
let assertLog;
66
let waitForThrow;
77

8+
let overrideQueueMicrotask;
9+
let flushFakeMicrotasks;
10+
811
// TODO: Migrate tests to React DOM instead of React Noop
912

1013
describe('ReactFlushSync (AggregateError not available)', () => {
@@ -13,6 +16,26 @@ describe('ReactFlushSync (AggregateError not available)', () => {
1316

1417
global.AggregateError = undefined;
1518

19+
// When AggregateError is not available, the errors are rethrown in a
20+
// microtask. This is an implementation detail but we want to test it here
21+
// so override the global one.
22+
const originalQueueMicrotask = queueMicrotask;
23+
overrideQueueMicrotask = false;
24+
const fakeMicrotaskQueue = [];
25+
global.queueMicrotask = cb => {
26+
if (overrideQueueMicrotask) {
27+
fakeMicrotaskQueue.push(cb);
28+
} else {
29+
originalQueueMicrotask(cb);
30+
}
31+
};
32+
flushFakeMicrotasks = () => {
33+
while (fakeMicrotaskQueue.length > 0) {
34+
const cb = fakeMicrotaskQueue.shift();
35+
cb();
36+
}
37+
};
38+
1639
React = require('react');
1740
ReactNoop = require('react-noop-renderer');
1841
Scheduler = require('scheduler');
@@ -47,6 +70,8 @@ describe('ReactFlushSync (AggregateError not available)', () => {
4770
const aahh = new Error('AAHH!');
4871
const nooo = new Error('Noooooooooo!');
4972

73+
// Override the global queueMicrotask so we can test the behavior.
74+
overrideQueueMicrotask = true;
5075
let error;
5176
try {
5277
ReactNoop.flushSync(() => {
@@ -70,10 +95,15 @@ describe('ReactFlushSync (AggregateError not available)', () => {
7095
// AggregateError is not available, React throws the first error, then
7196
// throws the remaining errors in separate tasks.
7297
expect(error).toBe(aahh);
98+
7399
// TODO: Currently the remaining error is rethrown in an Immediate Scheduler
74100
// task, but this may change to a timer or microtask in the future. The
75101
// exact mechanism is an implementation detail; they just need to be logged
76102
// in the order the occurred.
103+
104+
// This will start throwing if we change it to rethrow in a microtask.
105+
flushFakeMicrotasks();
106+
77107
await waitForThrow(nooo);
78108
});
79109
});

0 commit comments

Comments
 (0)