Skip to content

Unify serverAct helpers #33327

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

Merged
merged 6 commits into from
May 21, 2025
+94 −255
Merged

Unify serverAct helpers #33327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5de1268
Don't include undefined prop
sebmarkbage May 21, 2025
d8dcc2e
Use internal-test-utils serverAct instead
sebmarkbage May 21, 2025
47521da
Don't pass Scheduler to patchMessageChannel
sebmarkbage May 21, 2025
1eef184
Don't patch setImmediate using Scheduler neither
sebmarkbage May 21, 2025
6dcba95
Flush work before awaiting the result in case we need to do work to p…
sebmarkbage May 21, 2025
30dafe0
Run process.nextTicks between events
sebmarkbage May 21, 2025
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
76 changes: 45 additions & 31 deletions packages/internal-test-utils/internalAct.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export async function act<T>(scope: () => Thenable<T>): Thenable<T> {
// those will also fire now, too, which is not ideal. (The public
// version of `act` doesn't do this.) For this reason, we should try
// to avoid using timers in our internal tests.
j.runAllTicks();
j.runOnlyPendingTimers();
// If a committing a fallback triggers another update, it might not
// get scheduled until a microtask. So wait one more time.
Expand Down Expand Up @@ -194,6 +195,39 @@ export async function act<T>(scope: () => Thenable<T>): Thenable<T> {
}
}

async function waitForTasksAndTimers(error: Error) {
do {
// Wait until end of current task/microtask.
await waitForMicrotasks();

// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
if (jest.isEnvironmentTornDown()) {
error.message =
'The Jest environment was torn down before `act` completed. This ' +
'probably means you forgot to `await` an `act` call.';
throw error;
}

// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
const j = jest;
if (j.getTimerCount() > 0) {
// There's a pending timer. Flush it now. We only do this in order to
// force Suspense fallbacks to display; the fact that it's a timer
// is an implementation detail. If there are other timers scheduled,
// those will also fire now, too, which is not ideal. (The public
// version of `act` doesn't do this.) For this reason, we should try
// to avoid using timers in our internal tests.
j.runAllTicks();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getTimerCount() includes process.nextTick mocks but runOnlyPendingTimers() alone doesn't run them. We don't use it ourselves but JSDOM uses it internally. This was driving me insane debugging this infinite loop.

Copy link
Collaborator

Choose a reason for hiding this comment

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

should we waitForMicrotasks here before running timers. that probably more closely approximates a real loop

Copy link
Collaborator

Choose a reason for hiding this comment

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

could continue the loop if runAllTicks changes the timer count. It's unfortunate you can't know if ticks ran. unless there isa. getTickCount or something

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically yes, but we also don't wait between multiple timers or multiple ticks.

What we really want is the runAllTimersAsync() but for whatever reason that's not available on the legacy mocks. I guess we'd have to upgrade to newer mock timers.

j.runOnlyPendingTimers();
// If a committing a fallback triggers another update, it might not
// get scheduled until a microtask. So wait one more time.
await waitForMicrotasks();
} else {
break;
}
} while (true);
}

export async function serverAct<T>(scope: () => Thenable<T>): Thenable<T> {
// We require every `act` call to assert console logs
// with one of the assertion helpers. Fails if not empty.
Expand Down Expand Up @@ -233,37 +267,17 @@ export async function serverAct<T>(scope: () => Thenable<T>): Thenable<T> {
}

try {
const result = await scope();

do {
// Wait until end of current task/microtask.
await waitForMicrotasks();

// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
if (jest.isEnvironmentTornDown()) {
error.message =
'The Jest environment was torn down before `act` completed. This ' +
'probably means you forgot to `await` an `act` call.';
throw error;
}

// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
const j = jest;
if (j.getTimerCount() > 0) {
// There's a pending timer. Flush it now. We only do this in order to
// force Suspense fallbacks to display; the fact that it's a timer
// is an implementation detail. If there are other timers scheduled,
// those will also fire now, too, which is not ideal. (The public
// version of `act` doesn't do this.) For this reason, we should try
// to avoid using timers in our internal tests.
j.runOnlyPendingTimers();
// If a committing a fallback triggers another update, it might not
// get scheduled until a microtask. So wait one more time.
await waitForMicrotasks();
} else {
break;
}
} while (true);
const promise = scope();
// $FlowFixMe[prop-missing]
if (promise && typeof promise.catch === 'function') {
// $FlowFixMe[incompatible-use]
promise.catch(() => {}); // Handle below
}
// See if we need to do some work to unblock the promise first.
await waitForTasksAndTimers(error);
const result = await promise;
// Then wait to flush the result.
await waitForTasksAndTimers(error);

if (thrownErrors.length > 0) {
// Rethrow any errors logged by the global error handling.
Expand Down
17 changes: 3 additions & 14 deletions packages/react-dom/src/__tests__/ReactClassComponentPropResolutionFizz-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ let ReactDOMServer;
let Scheduler;
let assertLog;
let container;
let act;
let serverAct;

describe('ReactClassComponentPropResolutionFizz', () => {
beforeEach(() => {
jest.resetModules();
Scheduler = require('scheduler');
patchMessageChannel(Scheduler);
act = require('internal-test-utils').act;
patchMessageChannel();

React = require('react');
ReactDOMServer = require('react-dom/server.browser');
assertLog = require('internal-test-utils').assertLog;
serverAct = require('internal-test-utils').serverAct;
container = document.createElement('div');
document.body.appendChild(container);
});
Expand All @@ -42,17 +42,6 @@ describe('ReactClassComponentPropResolutionFizz', () => {
document.body.removeChild(container);
});

async function serverAct(callback) {
let maybePromise;
await act(() => {
maybePromise = callback();
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
});
return maybePromise;
}

async function readIntoContainer(stream) {
const reader = stream.getReader();
let result = '';
Expand Down
15 changes: 3 additions & 12 deletions packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ global.ReadableStream =
global.TextEncoder = require('util').TextEncoder;

let act;
let serverAct;
let assertLog;
let waitForPaint;
let container;
Expand All @@ -35,8 +36,9 @@ describe('ReactDOMFizzForm', () => {
beforeEach(() => {
jest.resetModules();
Scheduler = require('scheduler');
patchMessageChannel(Scheduler);
patchMessageChannel();
act = require('internal-test-utils').act;
serverAct = require('internal-test-utils').serverAct;
React = require('react');
ReactDOMServer = require('react-dom/server.browser');
ReactDOMClient = require('react-dom/client');
Expand All @@ -52,17 +54,6 @@ describe('ReactDOMFizzForm', () => {
document.body.removeChild(container);
});

async function serverAct(callback) {
let maybePromise;
await act(() => {
maybePromise = callback();
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
});
return maybePromise;
}

async function readIntoContainer(stream) {
const reader = stream.getReader();
let result = '';
Expand Down
14 changes: 3 additions & 11 deletions packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,27 @@ global.ReadableStream =
global.TextEncoder = require('util').TextEncoder;

let act;
let serverAct;
let container;
let React;
let ReactDOMServer;
let ReactDOMClient;
let useFormStatus;
let useOptimistic;
let useActionState;
let Scheduler;
let assertConsoleErrorDev;

describe('ReactDOMFizzForm', () => {
beforeEach(() => {
jest.resetModules();
Scheduler = require('scheduler');
patchMessageChannel(Scheduler);
patchMessageChannel();
React = require('react');
ReactDOMServer = require('react-dom/server.browser');
ReactDOMClient = require('react-dom/client');
useFormStatus = require('react-dom').useFormStatus;
useOptimistic = require('react').useOptimistic;
act = require('internal-test-utils').act;
serverAct = require('internal-test-utils').serverAct;
assertConsoleErrorDev =
require('internal-test-utils').assertConsoleErrorDev;
container = document.createElement('div');
Expand All @@ -55,14 +55,6 @@ describe('ReactDOMFizzForm', () => {
document.body.removeChild(container);
});

async function serverAct(callback) {
let maybePromise;
await act(() => {
maybePromise = callback();
});
return maybePromise;
}

function submit(submitter) {
const form = submitter.form || submitter;
if (!submitter.form) {
Expand Down
19 changes: 3 additions & 16 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,33 +19,20 @@ global.TextEncoder = require('util').TextEncoder;
let React;
let ReactDOMFizzServer;
let Suspense;
let Scheduler;
let act;
let serverAct;

describe('ReactDOMFizzServerBrowser', () => {
beforeEach(() => {
jest.resetModules();

Scheduler = require('scheduler');
patchMessageChannel(Scheduler);
act = require('internal-test-utils').act;
patchMessageChannel();
serverAct = require('internal-test-utils').serverAct;

React = require('react');
ReactDOMFizzServer = require('react-dom/server.browser');
Suspense = React.Suspense;
});

async function serverAct(callback) {
let maybePromise;
await act(() => {
maybePromise = callback();
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
});
return maybePromise;
}

const theError = new Error('This is an error');
function Throw() {
throw theError;
Expand Down
19 changes: 3 additions & 16 deletions packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ let ReactDOMFizzServer;
let ReactDOMFizzStatic;
let Suspense;
let container;
let Scheduler;
let act;
let serverAct;

describe('ReactDOMFizzStaticBrowser', () => {
beforeEach(() => {
Expand All @@ -41,9 +40,8 @@ describe('ReactDOMFizzStaticBrowser', () => {
// We need the mocked version of setTimeout inside the document.
window.setTimeout = setTimeout;

Scheduler = require('scheduler');
patchMessageChannel(Scheduler);
act = require('internal-test-utils').act;
patchMessageChannel();
serverAct = require('internal-test-utils').serverAct;

React = require('react');
ReactDOM = require('react-dom');
Expand All @@ -61,17 +59,6 @@ describe('ReactDOMFizzStaticBrowser', () => {
document.body.removeChild(container);
});

async function serverAct(callback) {
let maybePromise;
await act(() => {
maybePromise = callback();
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
});
return maybePromise;
}

const theError = new Error('This is an error');
function Throw() {
throw theError;
Expand Down
17 changes: 3 additions & 14 deletions packages/react-dom/src/__tests__/ReactDOMFizzStaticFloat-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ let ReactDOMFizzServer;
let ReactDOMFizzStatic;
let Suspense;
let container;
let Scheduler;
let act;
let serverAct;

describe('ReactDOMFizzStaticFloat', () => {
beforeEach(() => {
jest.resetModules();
Scheduler = require('scheduler');
patchMessageChannel(Scheduler);
patchMessageChannel();
act = require('internal-test-utils').act;
serverAct = require('internal-test-utils').serverAct;

React = require('react');
ReactDOM = require('react-dom');
Expand All @@ -52,17 +52,6 @@ describe('ReactDOMFizzStaticFloat', () => {
document.body.removeChild(container);
});

async function serverAct(callback) {
let maybePromise;
await act(() => {
maybePromise = callback();
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
});
return maybePromise;
}

async function readIntoContainer(stream) {
const reader = stream.getReader();
let result = '';
Expand Down
5 changes: 4 additions & 1 deletion packages/react-dom/src/test-utils/FizzTestUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ function getVisibleChildren(element: Element): React$Node {
}
props[attributes[i].name] = attributes[i].value;
}
props.children = getVisibleChildren(node);
const nestedChildren = getVisibleChildren(node);
if (nestedChildren !== undefined) {
props.children = nestedChildren;
}
children.push(
require('react').createElement(node.tagName.toLowerCase(), props),
);
Expand Down
19 changes: 3 additions & 16 deletions packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOM-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ global.TextEncoder = require('util').TextEncoder;
global.TextDecoder = require('util').TextDecoder;

let act;
let serverAct;
let use;
let clientExports;
let clientExportsESM;
Expand All @@ -28,8 +29,6 @@ let ReactDOMClient;
let ReactServerDOMServer;
let ReactServerDOMClient;
let Suspense;
let ReactServerScheduler;
let reactServerAct;
let ErrorBoundary;

describe('ReactFlightTurbopackDOM', () => {
Expand All @@ -39,9 +38,8 @@ describe('ReactFlightTurbopackDOM', () => {
// condition
jest.resetModules();

ReactServerScheduler = require('scheduler');
patchSetImmediate(ReactServerScheduler);
reactServerAct = require('internal-test-utils').act;
patchSetImmediate();
serverAct = require('internal-test-utils').serverAct;

// Simulate the condition resolution
jest.mock('react-server-dom-turbopack/server', () =>
Expand Down Expand Up @@ -84,17 +82,6 @@ describe('ReactFlightTurbopackDOM', () => {
};
});

async function serverAct(callback) {
let maybePromise;
await reactServerAct(() => {
maybePromise = callback();
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
});
return maybePromise;
}

function getTestStream() {
const writable = new Stream.PassThrough();
const readable = new ReadableStream({
Expand Down
Loading
Loading