Skip to content

Commit

Permalink
[Flight][Fizz] schedule work async (#29551)
Browse files Browse the repository at this point in the history 
While most builds of Flight and Fizz schedule work in new tasks some do
execute work synchronously. While this is necessary for legacy APIs like
renderToString for modern APIs there really isn't a great reason to do
this synchronously.

We could schedule works as microtasks but we actually want to yield so
the runtime can run events and other things that will unblock additional
work before starting the next work loop.

This change updates all non-legacy uses to be async using the best
availalble macrotask scheduler.

Browser now uses postMessage
Bun uses setTimeout because while it also supports setImmediate the
scheduling is not as eager as the same API in node
the FB build also uses setTimeout

This change required a number of changes to tests which were utilizing
the sync nature of work in the Browser builds to avoid having to manage
timers and tasks. I added a patch to install MessageChannel which is
required by the browser builds and made this patched version integrate
with the Scheduler mock. This way we can effectively use `act` to flush
flight and fizz work similar to how we do this on the client.
  • Loading branch information
@gnoff
gnoff authored Jun 6, 2024
1 parent fd6e130 commit b526a0a
Show file tree
Hide file tree
Showing 22 changed files with 1,419 additions and 837 deletions.
26 changes: 22 additions & 4 deletions packages/react-dom/src/__tests__/ReactClassComponentPropResolutionFizz-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
'use strict';

import {insertNodesAndExecuteScripts} from '../test-utils/FizzTestUtils';
import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel';

// Polyfills for test environment
global.ReadableStream =
Expand All @@ -21,12 +22,16 @@ let ReactDOMServer;
let Scheduler;
let assertLog;
let container;
let act;

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

React = require('react');
ReactDOMServer = require('react-dom/server.browser');
assertLog = require('internal-test-utils').assertLog;
container = document.createElement('div');
Expand All @@ -37,6 +42,17 @@ 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 All @@ -57,7 +73,7 @@ describe('ReactClassComponentPropResolutionFizz', () => {
return text;
}

test('resolves ref and default props before calling lifecycle methods', async () => {
it('resolves ref and default props before calling lifecycle methods', async () => {
function getPropKeys(props) {
return Object.keys(props).join(', ');
}
Expand All @@ -80,11 +96,13 @@ describe('ReactClassComponentPropResolutionFizz', () => {
};

// `ref` should never appear as a prop. `default` always should.

const ref = React.createRef();
const stream = await ReactDOMServer.renderToReadableStream(
<Component text="Yay" ref={ref} />,
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<Component text="Yay" ref={ref} />),
);
await readIntoContainer(stream);

assertLog([
'constructor: text, default',
'componentWillMount: text, default',
Expand Down
30 changes: 24 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
insertNodesAndExecuteScripts,
getVisibleChildren,
} from '../test-utils/FizzTestUtils';
import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel';

// Polyfills for test environment
global.ReadableStream =
Expand All @@ -33,13 +34,14 @@ let Suspense;
describe('ReactDOMFizzForm', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
Scheduler = require('scheduler');
patchMessageChannel(Scheduler);
act = require('internal-test-utils').act;
React = require('react');
ReactDOMServer = require('react-dom/server.browser');
ReactDOMClient = require('react-dom/client');
useDeferredValue = React.useDeferredValue;
Suspense = React.Suspense;
act = require('internal-test-utils').act;
assertLog = require('internal-test-utils').assertLog;
waitForPaint = require('internal-test-utils').waitForPaint;
container = document.createElement('div');
Expand All @@ -50,6 +52,17 @@ 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 All @@ -76,7 +89,9 @@ describe('ReactDOMFizzForm', () => {
return useDeferredValue('Final', 'Initial');
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
expect(container.textContent).toEqual('Initial');

Expand Down Expand Up @@ -107,7 +122,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
expect(container.textContent).toEqual('Loading...');

Expand Down Expand Up @@ -153,8 +170,9 @@ describe('ReactDOMFizzForm', () => {

const cRef = React.createRef();

// The server renders using the "initial" value for B.
const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
assertLog(['A', 'B [Initial]', 'C']);
expect(getVisibleChildren(container)).toEqual(
Expand Down
72 changes: 57 additions & 15 deletions packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
'use strict';

import {insertNodesAndExecuteScripts} from '../test-utils/FizzTestUtils';
import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel';

// Polyfills for test environment
global.ReadableStream =
Expand All @@ -24,10 +25,13 @@ let ReactDOMClient;
let useFormStatus;
let useOptimistic;
let useActionState;
let Scheduler;

describe('ReactDOMFizzForm', () => {
beforeEach(() => {
jest.resetModules();
Scheduler = require('scheduler');
patchMessageChannel(Scheduler);
React = require('react');
ReactDOMServer = require('react-dom/server.browser');
ReactDOMClient = require('react-dom/client');
Expand All @@ -48,6 +52,14 @@ 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 Expand Up @@ -96,7 +108,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
await act(async () => {
ReactDOMClient.hydrateRoot(container, <App />);
Expand Down Expand Up @@ -143,7 +157,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
await act(async () => {
ReactDOMClient.hydrateRoot(container, <App />);
Expand Down Expand Up @@ -175,7 +191,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
await expect(async () => {
await act(async () => {
Expand All @@ -197,7 +215,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
// This should ideally warn because only the client provides a function that doesn't line up.
await act(async () => {
Expand Down Expand Up @@ -231,7 +251,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
let root;
await act(async () => {
Expand Down Expand Up @@ -278,7 +300,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
let root;
await act(async () => {
Expand Down Expand Up @@ -334,7 +358,9 @@ describe('ReactDOMFizzForm', () => {
// Specifying the extra form fields are a DEV error, but we expect it
// to eventually still be patched up after an update.
await expect(async () => {
const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
}).toErrorDev([
'Cannot specify a encType or method for a form that specifies a function as the action.',
Expand Down Expand Up @@ -379,7 +405,9 @@ describe('ReactDOMFizzForm', () => {
return 'Pending: ' + pending;
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
expect(container.textContent).toBe('Pending: false');

Expand All @@ -400,7 +428,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);

// Dispatch an event before hydration
Expand Down Expand Up @@ -441,7 +471,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);

submit(container.getElementsByTagName('input')[1]);
Expand All @@ -463,7 +495,9 @@ describe('ReactDOMFizzForm', () => {
return optimisticState;
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
expect(container.textContent).toBe('hi');

Expand All @@ -484,7 +518,9 @@ describe('ReactDOMFizzForm', () => {
return state;
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);
expect(container.textContent).toBe('0');

Expand Down Expand Up @@ -521,7 +557,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);

const form = container.firstChild;
Expand Down Expand Up @@ -581,7 +619,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);

const input = container.getElementsByTagName('input')[1];
Expand Down Expand Up @@ -651,7 +691,9 @@ describe('ReactDOMFizzForm', () => {
);
}

const stream = await ReactDOMServer.renderToReadableStream(<App />);
const stream = await serverAct(() =>
ReactDOMServer.renderToReadableStream(<App />),
);
await readIntoContainer(stream);

const barField = container.querySelector('[name=bar]');
Expand Down
Loading

0 comments on commit b526a0a

Please sign in to comment.