From 67fee58b1f72754cc77488c40c44e786572ef954 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 26 Sep 2024 13:51:45 -0700 Subject: [PATCH] [Fizz] Start initial work immediately (#31079) In a recent update we make Flight start working immediately rather than waitin for a new task. This commit updates fizz to have similar mechanics. We start the render in the currently running task but we do so in a microtask to avoid reentrancy. This aligns Fizz with Flight. ref: https://github.com/facebook/react/pull/30961 --- .../src/__tests__/ReactDOMFizzServer-test.js | 4 +- .../__tests__/ReactDOMFizzServerNode-test.js | 237 +++++++++--------- packages/react-server/src/ReactFizzServer.js | 72 +++--- 3 files changed, 160 insertions(+), 153 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index a109011b35e98..a2ca222daf04b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -8119,7 +8119,7 @@ describe('ReactDOMFizzServer', () => { prerendering = false; - const resumed = await ReactDOMFizzServer.resumeToPipeableStream( + const resumed = ReactDOMFizzServer.resumeToPipeableStream( , JSON.parse(JSON.stringify(prerendered.postponed)), { @@ -8187,7 +8187,7 @@ describe('ReactDOMFizzServer', () => { function onPostpone(reason) { postpones.push(reason); } - const result = await renderToPipeableStream(, { + const result = renderToPipeableStream(, { onError, onShellError, onPostpone, diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 1e93c2420b8c8..f890840f32b32 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -14,6 +14,7 @@ let Stream; let React; let ReactDOMFizzServer; let Suspense; +let act; describe('ReactDOMFizzServerNode', () => { beforeEach(() => { @@ -22,6 +23,7 @@ describe('ReactDOMFizzServerNode', () => { ReactDOMFizzServer = require('react-dom/server'); Stream = require('stream'); Suspense = React.Suspense; + act = require('internal-test-utils').act; }); function getTestWritable() { @@ -54,54 +56,59 @@ describe('ReactDOMFizzServerNode', () => { throw theInfinitePromise; } - it('should call renderToPipeableStream', () => { + it('should call renderToPipeableStream', async () => { const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream( -
hello world
, - ); - pipe(writable); - jest.runAllTimers(); + await act(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( +
hello world
, + ); + pipe(writable); + }); expect(output.result).toMatchInlineSnapshot(`"
hello world
"`); }); - it('should emit DOCTYPE at the root of the document', () => { + it('should emit DOCTYPE at the root of the document', async () => { const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream( - - hello world - , - ); - pipe(writable); - jest.runAllTimers(); + await act(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + + hello world + , + ); + pipe(writable); + }); // with Float, we emit empty heads if they are elided when rendering expect(output.result).toMatchInlineSnapshot( `"hello world"`, ); }); - it('should emit bootstrap script src at the end', () => { + it('should emit bootstrap script src at the end', async () => { const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream( -
hello world
, - { - bootstrapScriptContent: 'INIT();', - bootstrapScripts: ['init.js'], - bootstrapModules: ['init.mjs'], - }, - ); - pipe(writable); - jest.runAllTimers(); + await act(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( +
hello world
, + { + bootstrapScriptContent: 'INIT();', + bootstrapScripts: ['init.js'], + bootstrapModules: ['init.mjs'], + }, + ); + pipe(writable); + }); expect(output.result).toMatchInlineSnapshot( `"
hello world
"`, ); }); - it('should start writing after pipe', () => { + it('should start writing after pipe', async () => { const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream( -
hello world
, - ); - jest.runAllTimers(); + let pipe; + await act(() => { + pipe = ReactDOMFizzServer.renderToPipeableStream( +
hello world
, + ).pipe; + }); // First we write our header. output.result += 'test'; @@ -281,24 +288,26 @@ describe('ReactDOMFizzServerNode', () => { let isCompleteCalls = 0; const errors = []; const {writable, output, completed} = getTestWritable(); - const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream( -
- Loading
}> - - - , - { - onError(x) { - errors.push(x.message); - }, - onAllReady() { - isCompleteCalls++; + let abort; + await act(() => { + const pipeable = ReactDOMFizzServer.renderToPipeableStream( +
+ Loading
}> + + + , + { + onError(x) { + errors.push(x.message); + }, + onAllReady() { + isCompleteCalls++; + }, }, - }, - ); - pipe(writable); - - jest.runAllTimers(); + ); + pipeable.pipe(writable); + abort = pipeable.abort; + }); expect(output.result).toContain('Loading'); expect(isCompleteCalls).toBe(0); @@ -360,26 +369,28 @@ describe('ReactDOMFizzServerNode', () => { let isCompleteCalls = 0; const errors = []; const {writable, output, completed} = getTestWritable(); - const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream( -
- - }> - + let abort; + await act(() => { + const pipeable = ReactDOMFizzServer.renderToPipeableStream( +
+ + }> + + - -
, - { - onError(x) { - errors.push(x.message); - }, - onAllReady() { - isCompleteCalls++; +
, + { + onError(x) { + errors.push(x.message); + }, + onAllReady() { + isCompleteCalls++; + }, }, - }, - ); - pipe(writable); - - jest.runAllTimers(); + ); + pipeable.pipe(writable); + abort = pipeable.abort; + }); expect(output.result).toContain('Loading'); expect(isCompleteCalls).toBe(0); @@ -428,15 +439,15 @@ describe('ReactDOMFizzServerNode', () => { const client = new DelayClient(); const {writable, output, completed} = getTestWritable(); - ReactDOMFizzServer.renderToPipeableStream( - - - - - , - ).pipe(writable); - - jest.runAllTimers(); + await act(() => { + ReactDOMFizzServer.renderToPipeableStream( + + + + + , + ).pipe(writable); + }); expect(output.error).toBe(undefined); expect(output.result).toContain('loading'); @@ -481,29 +492,28 @@ describe('ReactDOMFizzServerNode', () => { output: output0, completed: completed0, } = getTestWritable(); - ReactDOMFizzServer.renderToPipeableStream( - - - - - , - ).pipe(writable0); - const client1 = new DelayClient(); const { writable: writable1, output: output1, completed: completed1, } = getTestWritable(); - ReactDOMFizzServer.renderToPipeableStream( - - - - - , - ).pipe(writable1); - - jest.runAllTimers(); + await act(() => { + ReactDOMFizzServer.renderToPipeableStream( + + + + + , + ).pipe(writable0); + ReactDOMFizzServer.renderToPipeableStream( + + + + + , + ).pipe(writable1); + }); expect(output0.error).toBe(undefined); expect(output0.result).toContain('loading'); @@ -552,22 +562,22 @@ describe('ReactDOMFizzServerNode', () => { const client = new DelayClient(); const {writable, output, completed} = getTestWritable(); - ReactDOMFizzServer.renderToPipeableStream( - <> - - - - - - - - - - - , - ).pipe(writable); - - jest.runAllTimers(); + await act(() => { + ReactDOMFizzServer.renderToPipeableStream( + <> + + + + + + + + + + + , + ).pipe(writable); + }); expect(output.error).toBe(undefined); expect(output.result).toContain('loading'); @@ -630,13 +640,14 @@ describe('ReactDOMFizzServerNode', () => { expect(isComplete).toBe(true); }); - it('should encode multibyte characters correctly without nulls (#24985)', () => { + it('should encode multibyte characters correctly without nulls (#24985)', async () => { const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream( -
{Array(700).fill('ののの')}
, - ); - pipe(writable); - jest.runAllTimers(); + await act(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( +
{Array(700).fill('ののの')}
, + ); + pipe(writable); + }); expect(output.result.indexOf('\u0000')).toBe(-1); expect(output.result).toEqual( '
' + Array(700).fill('ののの').join('') + '
', diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 0f863d24b9c57..f384908720a33 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -316,10 +316,11 @@ type Segment = { textEmbedded: boolean, }; -const OPEN = 0; -const ABORTING = 1; -const CLOSING = 2; -const CLOSED = 3; +const OPENING = 10; +const OPEN = 11; +const ABORTING = 12; +const CLOSING = 13; +const CLOSED = 14; export opaque type Request = { destination: null | Destination, @@ -328,7 +329,7 @@ export opaque type Request = { +renderState: RenderState, +rootFormatContext: FormatContext, +progressiveChunkSize: number, - status: 0 | 1 | 2 | 3, + status: 10 | 11 | 12 | 13 | 14, fatalError: mixed, nextSegmentId: number, allPendingTasks: number, // when it reaches zero, we can close the connection. @@ -424,7 +425,7 @@ function RequestInstance( progressiveChunkSize === undefined ? DEFAULT_PROGRESSIVE_CHUNK_SIZE : progressiveChunkSize; - this.status = OPEN; + this.status = OPENING; this.fatalError = null; this.nextSegmentId = 0; this.allPendingTasks = 0; @@ -688,7 +689,7 @@ function pingTask(request: Request, task: Task): void { pingedTasks.push(task); if (request.pingedTasks.length === 1) { request.flushScheduled = request.destination !== null; - if (request.trackedPostpones !== null) { + if (request.trackedPostpones !== null || request.status === OPENING) { scheduleMicrotask(() => performWork(request)); } else { scheduleWork(() => performWork(request)); @@ -4977,43 +4978,38 @@ function flushCompletedQueues( export function startWork(request: Request): void { request.flushScheduled = request.destination !== null; - if (request.trackedPostpones !== null) { - // When prerendering we use microtasks for pinging work - if (supportsRequestStorage) { - scheduleMicrotask(() => - requestStorage.run(request, performWork, request), - ); - } else { - scheduleMicrotask(() => performWork(request)); - } + // When prerendering we use microtasks for pinging work + if (supportsRequestStorage) { + scheduleMicrotask(() => requestStorage.run(request, performWork, request)); } else { - // When rendering/resuming we use regular tasks and we also emit early preloads - if (supportsRequestStorage) { - scheduleWork(() => requestStorage.run(request, performWork, request)); - } else { - scheduleWork(() => performWork(request)); + scheduleMicrotask(() => performWork(request)); + } + scheduleWork(() => { + if (request.status === OPENING) { + request.status = OPEN; } - // this is either a regular render or a resume. For regular render we want - // to call emitEarlyPreloads after the first performWork because we want - // are responding to a live request and need to balance sending something early - // (i.e. don't want for the shell to finish) but we need something to send. - // The only implementation of this is for DOM at the moment and during resumes nothing - // actually emits but the code paths here are the same. - // During a prerender we don't want to be too aggressive in emitting early preloads - // because we aren't responding to a live request and we can wait for the prerender to - // postpone before we emit anything. - if (supportsRequestStorage) { - scheduleWork(() => + + if (request.trackedPostpones === null) { + // this is either a regular render or a resume. For regular render we want + // to call emitEarlyPreloads after the first performWork because we want + // are responding to a live request and need to balance sending something early + // (i.e. don't want for the shell to finish) but we need something to send. + // The only implementation of this is for DOM at the moment and during resumes nothing + // actually emits but the code paths here are the same. + // During a prerender we don't want to be too aggressive in emitting early preloads + // because we aren't responding to a live request and we can wait for the prerender to + // postpone before we emit anything. + if (supportsRequestStorage) { requestStorage.run( request, enqueueEarlyPreloadsAfterInitialWork, request, - ), - ); - } else { - scheduleWork(() => enqueueEarlyPreloadsAfterInitialWork(request)); + ); + } else { + enqueueEarlyPreloadsAfterInitialWork(request); + } } - } + }); } function enqueueEarlyPreloadsAfterInitialWork(request: Request) { @@ -5095,7 +5091,7 @@ export function stopFlowing(request: Request): void { // This is called to early terminate a request. It puts all pending boundaries in client rendered state. export function abort(request: Request, reason: mixed): void { - if (request.status === OPEN) { + if (request.status === OPEN || request.status === OPENING) { request.status = ABORTING; }