Skip to content

Commit 8b02df1

Browse files
committed
[Flight] use microtask for scheduling during prerenders
In #29491 I updated the work scheduler for Flight to use microtasks to perform work when something pings. This is useful but it does have some downsides in terms of our ability to do task prioritization. Additionally the initial work is not instantiated using a microtask which is inconsistent with how pings work. In this change I update the scheduling logic to use microtasks consistently for prerenders and use regular tasks for renders both for the initial work and pings.
1 parent 85180b8 commit 8b02df1

File tree

4 files changed

+109
-13
lines changed

4 files changed

+109
-13
lines changed

packages/internal-test-utils/ReactInternalTestUtils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
clearErrors,
1717
createLogAssertion,
1818
} from './consoleMock';
19-
export {act} from './internalAct';
19+
export {act, serverAct} from './internalAct';
2020
const {assertConsoleLogsCleared} = require('internal-test-utils/consoleMock');
2121

2222
import {thrownErrors, actingUpdatesScopeDepth} from './internalAct';

packages/internal-test-utils/internalAct.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,90 @@ export async function act<T>(scope: () => Thenable<T>): Thenable<T> {
192192
}
193193
}
194194
}
195+
196+
export async function serverAct<T>(scope: () => Thenable<T>): Thenable<T> {
197+
// We require every `act` call to assert console logs
198+
// with one of the assertion helpers. Fails if not empty.
199+
assertConsoleLogsCleared();
200+
201+
// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
202+
if (!jest.isMockFunction(setTimeout)) {
203+
throw Error(
204+
"This version of `act` requires Jest's timer mocks " +
205+
'(i.e. jest.useFakeTimers).',
206+
);
207+
}
208+
209+
// Create the error object before doing any async work, to get a better
210+
// stack trace.
211+
const error = new Error();
212+
Error.captureStackTrace(error, act);
213+
214+
// Call the provided scope function after an async gap. This is an extra
215+
// precaution to ensure that our tests do not accidentally rely on the act
216+
// scope adding work to the queue synchronously. We don't do this in the
217+
// public version of `act`, though we maybe should in the future.
218+
await waitForMicrotasks();
219+
220+
const errorHandlerNode = function (err: mixed) {
221+
thrownErrors.push(err);
222+
};
223+
// We track errors that were logged globally as if they occurred in this scope and then rethrow them.
224+
if (typeof process === 'object') {
225+
// Node environment
226+
process.on('uncaughtException', errorHandlerNode);
227+
} else if (
228+
typeof window === 'object' &&
229+
typeof window.addEventListener === 'function'
230+
) {
231+
throw new Error('serverAct is not supported in JSDOM environments');
232+
}
233+
234+
try {
235+
const result = await scope();
236+
237+
do {
238+
// Wait until end of current task/microtask.
239+
await waitForMicrotasks();
240+
241+
// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
242+
if (jest.isEnvironmentTornDown()) {
243+
error.message =
244+
'The Jest environment was torn down before `act` completed. This ' +
245+
'probably means you forgot to `await` an `act` call.';
246+
throw error;
247+
}
248+
249+
// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
250+
const j = jest;
251+
if (j.getTimerCount() > 0) {
252+
// There's a pending timer. Flush it now. We only do this in order to
253+
// force Suspense fallbacks to display; the fact that it's a timer
254+
// is an implementation detail. If there are other timers scheduled,
255+
// those will also fire now, too, which is not ideal. (The public
256+
// version of `act` doesn't do this.) For this reason, we should try
257+
// to avoid using timers in our internal tests.
258+
j.runOnlyPendingTimers();
259+
// If a committing a fallback triggers another update, it might not
260+
// get scheduled until a microtask. So wait one more time.
261+
await waitForMicrotasks();
262+
} else {
263+
break;
264+
}
265+
} while (true);
266+
267+
if (thrownErrors.length > 0) {
268+
// Rethrow any errors logged by the global error handling.
269+
const thrownError = aggregateErrors(thrownErrors);
270+
thrownErrors.length = 0;
271+
throw thrownError;
272+
}
273+
274+
return result;
275+
} finally {
276+
if (typeof process === 'object') {
277+
// Node environment
278+
process.off('uncaughtException', errorHandlerNode);
279+
}
280+
}
281+
}

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@ if (typeof File === 'undefined' || typeof FormData === 'undefined') {
2323
// Patch for Edge environments for global scope
2424
global.AsyncLocalStorage = require('async_hooks').AsyncLocalStorage;
2525

26-
const {
27-
patchMessageChannel,
28-
} = require('../../../../scripts/jest/patchMessageChannel');
29-
3026
let serverExports;
3127
let clientExports;
3228
let webpackMap;
@@ -39,7 +35,6 @@ let ReactServerDOMServer;
3935
let ReactServerDOMStaticServer;
4036
let ReactServerDOMClient;
4137
let use;
42-
let ReactServerScheduler;
4338
let reactServerAct;
4439

4540
function normalizeCodeLocInfo(str) {
@@ -55,9 +50,7 @@ describe('ReactFlightDOMEdge', () => {
5550
beforeEach(() => {
5651
jest.resetModules();
5752

58-
ReactServerScheduler = require('scheduler');
59-
patchMessageChannel(ReactServerScheduler);
60-
reactServerAct = require('internal-test-utils').act;
53+
reactServerAct = require('internal-test-utils').serverAct;
6154

6255
// Simulate the condition resolution
6356
jest.mock('react', () => require('react/react.react-server'));

packages/react-server/src/ReactFlightServer.js

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1794,7 +1794,11 @@ function pingTask(request: Request, task: Task): void {
17941794
pingedTasks.push(task);
17951795
if (pingedTasks.length === 1) {
17961796
request.flushScheduled = request.destination !== null;
1797-
scheduleMicrotask(() => performWork(request));
1797+
if (request.type === PRERENDER) {
1798+
scheduleMicrotask(() => performWork(request));
1799+
} else {
1800+
scheduleWork(() => performWork(request));
1801+
}
17981802
}
17991803
}
18001804

@@ -4056,10 +4060,20 @@ function flushCompletedChunks(
40564060

40574061
export function startWork(request: Request): void {
40584062
request.flushScheduled = request.destination !== null;
4059-
if (supportsRequestStorage) {
4060-
scheduleWork(() => requestStorage.run(request, performWork, request));
4063+
if (request.type === PRERENDER) {
4064+
if (supportsRequestStorage) {
4065+
scheduleMicrotask(() => {
4066+
requestStorage.run(request, performWork, request);
4067+
});
4068+
} else {
4069+
scheduleMicrotask(() => performWork(request));
4070+
}
40614071
} else {
4062-
scheduleWork(() => performWork(request));
4072+
if (supportsRequestStorage) {
4073+
scheduleWork(() => requestStorage.run(request, performWork, request));
4074+
} else {
4075+
scheduleWork(() => performWork(request));
4076+
}
40634077
}
40644078
}
40654079

@@ -4073,6 +4087,8 @@ function enqueueFlush(request: Request): void {
40734087
request.destination !== null
40744088
) {
40754089
request.flushScheduled = true;
4090+
// Unlike startWork and pingTask we intetionally use scheduleWork
4091+
// here even during prerenders to allow as much batching as possible
40764092
scheduleWork(() => {
40774093
request.flushScheduled = false;
40784094
const destination = request.destination;

0 commit comments

Comments
 (0)