From 7ed3e5a78d57cd6dd5fb85a908f0ef77805098b4 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 22 Mar 2021 16:53:19 -0400 Subject: [PATCH 1/4] Report errors to a global handler This allows you to log errors or set things like status codes. --- .../ReactDOMFizzServerBrowser-test.js | 21 +++++++++++++++++ .../__tests__/ReactDOMFizzServerNode-test.js | 23 +++++++++++++++++++ .../src/server/ReactDOMFizzServerBrowser.js | 2 ++ .../src/server/ReactDOMFizzServerNode.js | 2 ++ .../src/ReactNoopServer.js | 2 ++ packages/react-server/src/ReactFizzServer.js | 10 +++++++- 6 files changed, 59 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 0167103db57cf..340898ad5cd86 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -60,10 +60,16 @@ describe('ReactDOMFizzServer', () => { // @gate experimental it('should error the stream when an error is thrown at the root', async () => { + const reportedErrors = []; const stream = ReactDOMFizzServer.renderToReadableStream(
, + { + onError(x) { + reportedErrors.push(x); + }, + }, ); let caughtError = null; @@ -75,16 +81,23 @@ describe('ReactDOMFizzServer', () => { } expect(caughtError).toBe(theError); expect(result).toBe(''); + expect(reportedErrors).toEqual([theError]); }); // @gate experimental it('should error the stream when an error is thrown inside a fallback', async () => { + const reportedErrors = []; const stream = ReactDOMFizzServer.renderToReadableStream(
}>
, + { + onError(x) { + reportedErrors.push(x); + }, + }, ); let caughtError = null; @@ -96,20 +109,28 @@ describe('ReactDOMFizzServer', () => { } expect(caughtError).toBe(theError); expect(result).toBe(''); + expect(reportedErrors).toEqual([theError]); }); // @gate experimental it('should not error the stream when an error is thrown inside suspense boundary', async () => { + const reportedErrors = []; const stream = ReactDOMFizzServer.renderToReadableStream(
Loading
}> , + { + onError(x) { + reportedErrors.push(x); + }, + }, ); const result = await readResult(stream); expect(result).toContain('Loading'); + expect(reportedErrors).toEqual([theError]); }); // @gate experimental diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index a7ea970bece51..e77585cdef297 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -88,12 +88,18 @@ describe('ReactDOMFizzServer', () => { // @gate experimental it('should error the stream when an error is thrown at the root', async () => { + const reportedErrors = []; const {writable, output, completed} = getTestWritable(); ReactDOMFizzServer.pipeToNodeWritable(
, writable, + { + onError(x) { + reportedErrors.push(x); + }, + }, ); // The stream is errored even if we haven't started writing. @@ -102,10 +108,13 @@ describe('ReactDOMFizzServer', () => { expect(output.error).toBe(theError); expect(output.result).toBe(''); + // This type of error is reported to the error callback too. + expect(reportedErrors).toEqual([theError]); }); // @gate experimental it('should error the stream when an error is thrown inside a fallback', async () => { + const reportedErrors = []; const {writable, output, completed} = getTestWritable(); const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
@@ -114,6 +123,11 @@ describe('ReactDOMFizzServer', () => {
, writable, + { + onError(x) { + reportedErrors.push(x); + }, + }, ); startWriting(); @@ -121,10 +135,12 @@ describe('ReactDOMFizzServer', () => { expect(output.error).toBe(theError); expect(output.result).toBe(''); + expect(reportedErrors).toEqual([theError]); }); // @gate experimental it('should not error the stream when an error is thrown inside suspense boundary', async () => { + const reportedErrors = []; const {writable, output, completed} = getTestWritable(); const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
@@ -133,6 +149,11 @@ describe('ReactDOMFizzServer', () => {
, writable, + { + onError(x) { + reportedErrors.push(x); + }, + }, ); startWriting(); @@ -140,6 +161,8 @@ describe('ReactDOMFizzServer', () => { expect(output.error).toBe(undefined); expect(output.result).toContain('Loading'); + // While no error is reported to the stream, the error is reported to the callback. + expect(reportedErrors).toEqual([theError]); }); // @gate experimental diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index 902989fefca0f..c419b2936b9af 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -22,6 +22,7 @@ type Options = { identifierPrefix?: string, progressiveChunkSize?: number, signal?: AbortSignal, + onError?: (error: mixed) => void, }; function renderToReadableStream( @@ -44,6 +45,7 @@ function renderToReadableStream( controller, createResponseState(options ? options.identifierPrefix : undefined), options ? options.progressiveChunkSize : undefined, + options ? options.onError : undefined, ); startWork(request); }, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index 8de76f7095f9d..0b5be8a89a909 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -26,6 +26,7 @@ function createDrainHandler(destination, request) { type Options = { identifierPrefix?: string, progressiveChunkSize?: number, + onError?: (error: mixed) => void, }; type Controls = { @@ -44,6 +45,7 @@ function pipeToNodeWritable( destination, createResponseState(options ? options.identifierPrefix : undefined), options ? options.progressiveChunkSize : undefined, + options ? options.onError : undefined, ); let hasStartedFlowing = false; startWork(request); diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index d2a4bf032933c..f53d8274ad176 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -217,6 +217,7 @@ const ReactNoopServer = ReactFizzServer({ type Options = { progressiveChunkSize?: number, + onError?: (error: mixed) => void, }; function render(children: React$Element, options?: Options): Destination { @@ -234,6 +235,7 @@ function render(children: React$Element, options?: Options): Destination { destination, null, options ? options.progressiveChunkSize : undefined, + options ? options.onError : undefined, ); ReactNoopServer.startWork(request); ReactNoopServer.startFlowing(request); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index a52079c1587ce..01dcc70e98b4a 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -110,6 +110,7 @@ type Request = { clientRenderedBoundaries: Array, // Errored or client rendered but not yet flushed. completedBoundaries: Array, // Completed but not yet fully flushed boundaries to show. partialBoundaries: Array, // Partially completed boundaries that can flush its segments early. + onError: (error: mixed) => void, }; // This is a default heuristic for how to split up the HTML content into progressive @@ -134,6 +135,7 @@ export function createRequest( destination: Destination, responseState: ResponseState, progressiveChunkSize: number = DEFAULT_PROGRESSIVE_CHUNK_SIZE, + onError: (error: mixed) => void = noop, ): Request { const pingedWork = []; const abortSet: Set = new Set(); @@ -151,6 +153,7 @@ export function createRequest( clientRenderedBoundaries: [], completedBoundaries: [], partialBoundaries: [], + onError, }; // This segment represents the root fallback. const rootSegment = createPendingSegment(request, 0, null); @@ -235,7 +238,9 @@ function createPendingSegment( } function reportError(request: Request, error: mixed): void { - // TODO: Report errors on the server. + // If this callback errors, we intentionally let that error bubble up to become a fatal error + // so that someone fixes the error reporting instead of hiding it. + request.onError(error); } function fatalError(request: Request, error: mixed): void { @@ -573,6 +578,7 @@ function performWork(request: Request): void { flushCompletedQueues(request); } } catch (error) { + reportError(request, error); fatalError(request, error); } finally { ReactCurrentDispatcher.current = prevDispatcher; @@ -920,6 +926,7 @@ export function startFlowing(request: Request): void { try { flushCompletedQueues(request); } catch (error) { + reportError(request, error); fatalError(request, error); } } @@ -934,6 +941,7 @@ export function abort(request: Request): void { flushCompletedQueues(request); } } catch (error) { + reportError(request, error); fatalError(request, error); } } From 4d22b70d3fa7c04758d2bde790894eef31734e26 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 22 Mar 2021 18:38:42 -0400 Subject: [PATCH 2/4] Add complete callback --- .../ReactDOMFizzServerBrowser-test.js | 38 ++++++ .../__tests__/ReactDOMFizzServerNode-test.js | 48 ++++++++ .../src/server/ReactDOMFizzServerBrowser.js | 13 +- .../src/server/ReactDOMFizzServerNode.js | 2 + .../src/ReactNoopServer.js | 2 + packages/react-server/src/ReactFizzServer.js | 115 ++++++++++-------- 6 files changed, 166 insertions(+), 52 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 340898ad5cd86..83a31a6f6a0e6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -58,6 +58,44 @@ describe('ReactDOMFizzServer', () => { expect(result).toBe('
hello world
'); }); + // @gate experimental + it('emits all HTML as one unit if we wait until the end to start', async () => { + let hasLoaded = false; + let resolve; + const promise = new Promise(r => (resolve = r)); + function Wait() { + if (!hasLoaded) { + throw promise; + } + return 'Done'; + } + let isComplete = false; + const stream = ReactDOMFizzServer.renderToReadableStream( +
+ + + +
, + { + onComplete() { + isComplete = true; + }, + }, + ); + await jest.runAllTimers(); + expect(isComplete).toBe(false); + // Resolve the loading. + hasLoaded = true; + await resolve(); + + await jest.runAllTimers(); + + expect(isComplete).toBe(true); + + const result = await readResult(stream); + expect(result).toBe('
Done
'); + }); + // @gate experimental it('should error the stream when an error is thrown at the root', async () => { const reportedErrors = []; diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index e77585cdef297..468d16767612f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -86,6 +86,54 @@ describe('ReactDOMFizzServer', () => { ); }); + // @gate experimental + it('emits all HTML as one unit if we wait until the end to start', async () => { + let hasLoaded = false; + let resolve; + const promise = new Promise(r => (resolve = r)); + function Wait() { + if (!hasLoaded) { + throw promise; + } + return 'Done'; + } + let isComplete = false; + const {writable, output} = getTestWritable(); + const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable( +
+ + + +
, + writable, + { + onComplete() { + isComplete = true; + }, + }, + ); + await jest.runAllTimers(); + expect(output.result).toBe(''); + expect(isComplete).toBe(false); + // Resolve the loading. + hasLoaded = true; + await resolve(); + + await jest.runAllTimers(); + + expect(output.result).toBe(''); + expect(isComplete).toBe(true); + + // First we write our header. + output.result += + 'test'; + // Then React starts writing. + startWriting(); + expect(output.result).toBe( + 'test
Done
', + ); + }); + // @gate experimental it('should error the stream when an error is thrown at the root', async () => { const reportedErrors = []; diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index c419b2936b9af..e9159bbfea70b 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -22,6 +22,7 @@ type Options = { identifierPrefix?: string, progressiveChunkSize?: number, signal?: AbortSignal, + onComplete?: () => void, onError?: (error: mixed) => void, }; @@ -38,7 +39,7 @@ function renderToReadableStream( }; signal.addEventListener('abort', listener); } - return new ReadableStream({ + const stream = new ReadableStream({ start(controller) { request = createRequest( children, @@ -46,14 +47,22 @@ function renderToReadableStream( createResponseState(options ? options.identifierPrefix : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, + options ? options.onComplete : undefined, ); startWork(request); }, pull(controller) { - startFlowing(request); + // Pull is called immediately even if the stream is not passed to anything. + // That's buffering too early. We want to start buffering once the stream + // is actually used by something so we can give it the best result possible + // at that point. + if (stream.locked) { + startFlowing(request); + } }, cancel(reason) {}, }); + return stream; } export {renderToReadableStream}; diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index 0b5be8a89a909..b9399bb58a233 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -26,6 +26,7 @@ function createDrainHandler(destination, request) { type Options = { identifierPrefix?: string, progressiveChunkSize?: number, + onComplete?: () => void, onError?: (error: mixed) => void, }; @@ -46,6 +47,7 @@ function pipeToNodeWritable( createResponseState(options ? options.identifierPrefix : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, + options ? options.onComplete : undefined, ); let hasStartedFlowing = false; startWork(request); diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index f53d8274ad176..3538b565ceec7 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -217,6 +217,7 @@ const ReactNoopServer = ReactFizzServer({ type Options = { progressiveChunkSize?: number, + onComplete?: () => void, onError?: (error: mixed) => void, }; @@ -236,6 +237,7 @@ function render(children: React$Element, options?: Options): Destination { null, options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, + options ? options.onComplete : undefined, ); ReactNoopServer.startWork(request); ReactNoopServer.startFlowing(request); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 01dcc70e98b4a..3f42088a1931d 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -110,7 +110,11 @@ type Request = { clientRenderedBoundaries: Array, // Errored or client rendered but not yet flushed. completedBoundaries: Array, // Completed but not yet fully flushed boundaries to show. partialBoundaries: Array, // Partially completed boundaries that can flush its segments early. + // onError is called when an error happens anywhere in the tree. It might recover. onError: (error: mixed) => void, + // onComplete is called when all pending work is done but it may not have flushed yet. + // This is a good time to start writing if you want only HTML and no intermediate steps. + onComplete: () => void, }; // This is a default heuristic for how to split up the HTML content into progressive @@ -136,6 +140,7 @@ export function createRequest( responseState: ResponseState, progressiveChunkSize: number = DEFAULT_PROGRESSIVE_CHUNK_SIZE, onError: (error: mixed) => void = noop, + onComplete: () => void = noop, ): Request { const pingedWork = []; const abortSet: Set = new Set(); @@ -154,6 +159,7 @@ export function createRequest( completedBoundaries: [], partialBoundaries: [], onError, + onComplete, }; // This segment represents the root fallback. const rootSegment = createPendingSegment(request, 0, null); @@ -394,28 +400,31 @@ function erroredWork( segment: Segment, error: mixed, ) { - request.allPendingWork--; - if (boundary !== null) { - boundary.pendingWork--; - } - // Report the error to a global handler. reportError(request, error); if (boundary === null) { fatalError(request, error); - } else if (!boundary.forceClientRender) { - boundary.forceClientRender = true; - - // Regardless of what happens next, this boundary won't be displayed, - // so we can flush it, if the parent already flushed. - if (boundary.parentFlushed) { - // We don't have a preference where in the queue this goes since it's likely - // to error on the client anyway. However, intentionally client-rendered - // boundaries should be flushed earlier so that they can start on the client. - // We reuse the same queue for errors. - request.clientRenderedBoundaries.push(boundary); + } else { + boundary.pendingWork--; + if (!boundary.forceClientRender) { + boundary.forceClientRender = true; + + // Regardless of what happens next, this boundary won't be displayed, + // so we can flush it, if the parent already flushed. + if (boundary.parentFlushed) { + // We don't have a preference where in the queue this goes since it's likely + // to error on the client anyway. However, intentionally client-rendered + // boundaries should be flushed earlier so that they can start on the client. + // We reuse the same queue for errors. + request.clientRenderedBoundaries.push(boundary); + } } } + + request.allPendingWork--; + if (request.allPendingWork === 0) { + request.onComplete(); + } } function abortWorkSoft(suspendedWork: SuspendedWork): void { @@ -459,6 +468,10 @@ function abortWork(suspendedWork: SuspendedWork): void { request.clientRenderedBoundaries.push(boundary); } } + + if (request.allPendingWork === 0) { + request.onComplete(); + } } } @@ -467,8 +480,6 @@ function finishedWork( boundary: Root | SuspenseBoundary, segment: Segment, ) { - request.allPendingWork--; - if (boundary === null) { request.pendingRootWork--; if (segment.parentFlushed) { @@ -478,43 +489,47 @@ function finishedWork( ); request.completedRootSegment = segment; } - return; - } - - boundary.pendingWork--; - if (boundary.forceClientRender) { - // This already errored. - return; - } - if (boundary.pendingWork === 0) { - // This must have been the last segment we were waiting on. This boundary is now complete. - // We can now cancel any pending work on the fallback since we won't need to show it anymore. - boundary.fallbackAbortableWork.forEach(abortWorkSoft, request); - boundary.fallbackAbortableWork.clear(); - if (segment.parentFlushed) { - // Our parent segment already flushed, so we need to schedule this segment to be emitted. - boundary.completedSegments.push(segment); - } - if (boundary.parentFlushed) { - // The segment might be part of a segment that didn't flush yet, but if the boundary's - // parent flushed, we need to schedule the boundary to be emitted. - request.completedBoundaries.push(boundary); - } } else { - if (segment.parentFlushed) { - // Our parent already flushed, so we need to schedule this segment to be emitted. - const completedSegments = boundary.completedSegments; - completedSegments.push(segment); - if (completedSegments.length === 1) { - // This is the first time since we last flushed that we completed anything. - // We can schedule this boundary to emit its partially completed segments early - // in case the parent has already been flushed. - if (boundary.parentFlushed) { - request.partialBoundaries.push(boundary); + boundary.pendingWork--; + if (boundary.forceClientRender) { + // This already errored. + } else if (boundary.pendingWork === 0) { + // This must have been the last segment we were waiting on. This boundary is now complete. + // We can now cancel any pending work on the fallback since we won't need to show it anymore. + boundary.fallbackAbortableWork.forEach(abortWorkSoft, request); + boundary.fallbackAbortableWork.clear(); + if (segment.parentFlushed) { + // Our parent segment already flushed, so we need to schedule this segment to be emitted. + boundary.completedSegments.push(segment); + } + if (boundary.parentFlushed) { + // The segment might be part of a segment that didn't flush yet, but if the boundary's + // parent flushed, we need to schedule the boundary to be emitted. + request.completedBoundaries.push(boundary); + } + } else { + if (segment.parentFlushed) { + // Our parent already flushed, so we need to schedule this segment to be emitted. + const completedSegments = boundary.completedSegments; + completedSegments.push(segment); + if (completedSegments.length === 1) { + // This is the first time since we last flushed that we completed anything. + // We can schedule this boundary to emit its partially completed segments early + // in case the parent has already been flushed. + if (boundary.parentFlushed) { + request.partialBoundaries.push(boundary); + } } } } } + + request.allPendingWork--; + if (request.allPendingWork === 0) { + // This needs to be called at the very end so that we can synchronously write the result + // in the callback if needed. + request.onComplete(); + } } function retryWork(request: Request, work: SuspendedWork): void { From 0509b5734365a4d9b58aad9a753880ea460cb6f0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 22 Mar 2021 23:05:13 -0400 Subject: [PATCH 3/4] onReadyToStream callback This is typically not needed because if you want to stream when the root is ready you can just start writing immediately. --- .../src/__tests__/ReactDOMFizzServer-test.js | 24 ++++++++++++------- .../src/server/ReactDOMFizzServerBrowser.js | 2 ++ .../src/server/ReactDOMFizzServerNode.js | 2 ++ .../src/ReactNoopServer.js | 2 ++ packages/react-server/src/ReactFizzServer.js | 11 ++++++++- 5 files changed, 32 insertions(+), 9 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 3a7d7205dcb27..9021c23c6ce74 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -336,7 +336,6 @@ describe('ReactDOMFizzServer', () => { writable.write(chunk, encoding, next); }; - writable.write('
'); await act(async () => { const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable( }> @@ -346,13 +345,17 @@ describe('ReactDOMFizzServer', () => {
, writableA, - {identifierPrefix: 'A_'}, + { + identifierPrefix: 'A_', + onReadyToStream() { + writableA.write('
'); + startWriting(); + writableA.write('
'); + }, + }, ); - startWriting(); }); - writable.write(''); - writable.write('
'); await act(async () => { const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable( }> @@ -362,11 +365,16 @@ describe('ReactDOMFizzServer', () => {
, writableB, - {identifierPrefix: 'B_'}, + { + identifierPrefix: 'B_', + onReadyToStream() { + writableB.write('
'); + startWriting(); + writableB.write('
'); + }, + }, ); - startWriting(); }); - writable.write(''); expect(getVisibleChildren(container)).toEqual([
Loading A...
, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index e9159bbfea70b..6ea65b5544ead 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -22,6 +22,7 @@ type Options = { identifierPrefix?: string, progressiveChunkSize?: number, signal?: AbortSignal, + onReadyToStream?: () => void, onComplete?: () => void, onError?: (error: mixed) => void, }; @@ -48,6 +49,7 @@ function renderToReadableStream( options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, options ? options.onComplete : undefined, + options ? options.onReadyToStream : undefined, ); startWork(request); }, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index b9399bb58a233..25bdf21160c1d 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -26,6 +26,7 @@ function createDrainHandler(destination, request) { type Options = { identifierPrefix?: string, progressiveChunkSize?: number, + onReadyToStream?: () => void, onComplete?: () => void, onError?: (error: mixed) => void, }; @@ -48,6 +49,7 @@ function pipeToNodeWritable( options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, options ? options.onComplete : undefined, + options ? options.onReadyToStream : undefined, ); let hasStartedFlowing = false; startWork(request); diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 3538b565ceec7..9a6b560e7f57d 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -217,6 +217,7 @@ const ReactNoopServer = ReactFizzServer({ type Options = { progressiveChunkSize?: number, + onReadyToStream?: () => void, onComplete?: () => void, onError?: (error: mixed) => void, }; @@ -238,6 +239,7 @@ function render(children: React$Element, options?: Options): Destination { options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, options ? options.onComplete : undefined, + options ? options.onReadyToStream : undefined, ); ReactNoopServer.startWork(request); ReactNoopServer.startFlowing(request); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 3f42088a1931d..a990253634dc3 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -115,6 +115,10 @@ type Request = { // onComplete is called when all pending work is done but it may not have flushed yet. // This is a good time to start writing if you want only HTML and no intermediate steps. onComplete: () => void, + // onReadyToStream is called when there is at least a root fallback ready to show. + // Typically you don't need this callback because it's best practice to always have a + // root fallback ready so there's no need to wait. + onReadyToStream: () => void, }; // This is a default heuristic for how to split up the HTML content into progressive @@ -141,6 +145,7 @@ export function createRequest( progressiveChunkSize: number = DEFAULT_PROGRESSIVE_CHUNK_SIZE, onError: (error: mixed) => void = noop, onComplete: () => void = noop, + onReadyToStream: () => void = noop, ): Request { const pingedWork = []; const abortSet: Set = new Set(); @@ -160,6 +165,7 @@ export function createRequest( partialBoundaries: [], onError, onComplete, + onReadyToStream, }; // This segment represents the root fallback. const rootSegment = createPendingSegment(request, 0, null); @@ -481,7 +487,6 @@ function finishedWork( segment: Segment, ) { if (boundary === null) { - request.pendingRootWork--; if (segment.parentFlushed) { invariant( request.completedRootSegment === null, @@ -489,6 +494,10 @@ function finishedWork( ); request.completedRootSegment = segment; } + request.pendingRootWork--; + if (request.pendingRootWork === 0) { + request.onReadyToStream(); + } } else { boundary.pendingWork--; if (boundary.forceClientRender) { From b28626fa21ee4fcf61ec790bcc237effd0db89fd Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 23 Mar 2021 13:21:54 -0400 Subject: [PATCH 4/4] Rename onComplete -> onCompleteAll --- .../__tests__/ReactDOMFizzServerBrowser-test.js | 2 +- .../src/__tests__/ReactDOMFizzServerNode-test.js | 2 +- .../src/server/ReactDOMFizzServerBrowser.js | 4 ++-- .../react-dom/src/server/ReactDOMFizzServerNode.js | 4 ++-- .../react-noop-renderer/src/ReactNoopServer.js | 4 ++-- packages/react-server/src/ReactFizzServer.js | 14 +++++++------- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 83a31a6f6a0e6..b55bb446f6fce 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -77,7 +77,7 @@ describe('ReactDOMFizzServer', () => { , { - onComplete() { + onCompleteAll() { isComplete = true; }, }, diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 468d16767612f..5377607a5795b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -107,7 +107,7 @@ describe('ReactDOMFizzServer', () => { , writable, { - onComplete() { + onCompleteAll() { isComplete = true; }, }, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index 6ea65b5544ead..95a58f0688dd8 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -23,7 +23,7 @@ type Options = { progressiveChunkSize?: number, signal?: AbortSignal, onReadyToStream?: () => void, - onComplete?: () => void, + onCompleteAll?: () => void, onError?: (error: mixed) => void, }; @@ -48,7 +48,7 @@ function renderToReadableStream( createResponseState(options ? options.identifierPrefix : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, - options ? options.onComplete : undefined, + options ? options.onCompleteAll : undefined, options ? options.onReadyToStream : undefined, ); startWork(request); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index 25bdf21160c1d..51a091948d4b1 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -27,7 +27,7 @@ type Options = { identifierPrefix?: string, progressiveChunkSize?: number, onReadyToStream?: () => void, - onComplete?: () => void, + onCompleteAll?: () => void, onError?: (error: mixed) => void, }; @@ -48,7 +48,7 @@ function pipeToNodeWritable( createResponseState(options ? options.identifierPrefix : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, - options ? options.onComplete : undefined, + options ? options.onCompleteAll : undefined, options ? options.onReadyToStream : undefined, ); let hasStartedFlowing = false; diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 9a6b560e7f57d..feec884743efc 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -218,7 +218,7 @@ const ReactNoopServer = ReactFizzServer({ type Options = { progressiveChunkSize?: number, onReadyToStream?: () => void, - onComplete?: () => void, + onCompleteAll?: () => void, onError?: (error: mixed) => void, }; @@ -238,7 +238,7 @@ function render(children: React$Element, options?: Options): Destination { null, options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, - options ? options.onComplete : undefined, + options ? options.onCompleteAll : undefined, options ? options.onReadyToStream : undefined, ); ReactNoopServer.startWork(request); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index a990253634dc3..b936376e85eae 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -112,9 +112,9 @@ type Request = { partialBoundaries: Array, // Partially completed boundaries that can flush its segments early. // onError is called when an error happens anywhere in the tree. It might recover. onError: (error: mixed) => void, - // onComplete is called when all pending work is done but it may not have flushed yet. + // onCompleteAll is called when all pending work is done but it may not have flushed yet. // This is a good time to start writing if you want only HTML and no intermediate steps. - onComplete: () => void, + onCompleteAll: () => void, // onReadyToStream is called when there is at least a root fallback ready to show. // Typically you don't need this callback because it's best practice to always have a // root fallback ready so there's no need to wait. @@ -144,7 +144,7 @@ export function createRequest( responseState: ResponseState, progressiveChunkSize: number = DEFAULT_PROGRESSIVE_CHUNK_SIZE, onError: (error: mixed) => void = noop, - onComplete: () => void = noop, + onCompleteAll: () => void = noop, onReadyToStream: () => void = noop, ): Request { const pingedWork = []; @@ -164,7 +164,7 @@ export function createRequest( completedBoundaries: [], partialBoundaries: [], onError, - onComplete, + onCompleteAll, onReadyToStream, }; // This segment represents the root fallback. @@ -429,7 +429,7 @@ function erroredWork( request.allPendingWork--; if (request.allPendingWork === 0) { - request.onComplete(); + request.onCompleteAll(); } } @@ -476,7 +476,7 @@ function abortWork(suspendedWork: SuspendedWork): void { } if (request.allPendingWork === 0) { - request.onComplete(); + request.onCompleteAll(); } } } @@ -537,7 +537,7 @@ function finishedWork( if (request.allPendingWork === 0) { // This needs to be called at the very end so that we can synchronously write the result // in the callback if needed. - request.onComplete(); + request.onCompleteAll(); } }