diff --git a/fixtures/ssr/server/render.js b/fixtures/ssr/server/render.js index ae30e622174cb..a4fe698858ab1 100644 --- a/fixtures/ssr/server/render.js +++ b/fixtures/ssr/server/render.js @@ -22,13 +22,13 @@ export default function render(url, res) { let didError = false; const {pipe, abort} = renderToPipeableStream(, { bootstrapScripts: [assets['main.js']], - onCompleteShell() { + onShellReady() { // If something errored before we started streaming, we set the error code appropriately. res.statusCode = didError ? 500 : 200; res.setHeader('Content-type', 'text/html'); pipe(res); }, - onErrorShell(x) { + onShellError(x) { // Something errored before we could complete the shell so we emit an alternative shell. res.statusCode = 500; res.send('

Error

'); diff --git a/fixtures/ssr2/server/render.js b/fixtures/ssr2/server/render.js index ccd12af0d3638..ea2b6e4696a01 100644 --- a/fixtures/ssr2/server/render.js +++ b/fixtures/ssr2/server/render.js @@ -49,7 +49,7 @@ module.exports = function render(url, res) { res.setHeader('Content-type', 'text/html'); pipe(res); }, - onErrorShell(x) { + onShellError(x) { // Something errored before we could complete the shell so we emit an alternative shell. res.statusCode = 500; res.send('

Error

'); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 90b339bb281eb..0aacf5a755247 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -914,7 +914,7 @@ describe('ReactDOMFizzServer', () => { , { identifierPrefix: 'A_', - onCompleteShell() { + onShellReady() { writableA.write('
'); pipe(writableA); writableA.write('
'); @@ -933,7 +933,7 @@ describe('ReactDOMFizzServer', () => { , { identifierPrefix: 'B_', - onCompleteShell() { + onShellReady() { writableB.write('
'); pipe(writableB); writableB.write('
'); @@ -1168,7 +1168,7 @@ describe('ReactDOMFizzServer', () => { { namespaceURI: 'http://www.w3.org/2000/svg', - onCompleteShell() { + onShellReady() { writable.write(''); pipe(writable); writable.write(''); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 93e30aa2305be..ad6176e3fa5e6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -209,4 +209,43 @@ describe('ReactDOMFizzServer', () => { const result = await readResult(stream); expect(result).toContain('Loading'); }); + + // @gate experimental + it('should not continue rendering after the reader cancels', async () => { + let hasLoaded = false; + let resolve; + let isComplete = false; + let rendered = false; + const promise = new Promise(r => (resolve = r)); + function Wait() { + if (!hasLoaded) { + throw promise; + } + rendered = true; + return 'Done'; + } + const stream = await ReactDOMFizzServer.renderToReadableStream( +
+ Loading
}> + /> + + , + ); + + stream.allReady.then(() => (isComplete = true)); + + expect(rendered).toBe(false); + expect(isComplete).toBe(false); + + const reader = stream.getReader(); + reader.cancel(); + + hasLoaded = true; + resolve(); + + await jest.runAllTimers(); + + expect(rendered).toBe(false); + expect(isComplete).toBe(true); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index a35a250282ec4..c1844d7ef78bf 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -138,7 +138,7 @@ describe('ReactDOMFizzServer', () => { , { - onCompleteAll() { + onAllReady() { isCompleteCalls++; }, }, @@ -179,7 +179,7 @@ describe('ReactDOMFizzServer', () => { onError(x) { reportedErrors.push(x); }, - onErrorShell(x) { + onShellError(x) { reportedShellErrors.push(x); }, }, @@ -213,7 +213,7 @@ describe('ReactDOMFizzServer', () => { onError(x) { reportedErrors.push(x); }, - onErrorShell(x) { + onShellError(x) { reportedShellErrors.push(x); }, }, @@ -244,7 +244,7 @@ describe('ReactDOMFizzServer', () => { onError(x) { reportedErrors.push(x); }, - onErrorShell(x) { + onShellError(x) { reportedShellErrors.push(x); }, }, @@ -298,7 +298,7 @@ describe('ReactDOMFizzServer', () => { , { - onCompleteAll() { + onAllReady() { isCompleteCalls++; }, }, @@ -333,7 +333,7 @@ describe('ReactDOMFizzServer', () => { , { - onCompleteAll() { + onAllReady() { isCompleteCalls++; }, }, @@ -537,4 +537,49 @@ describe('ReactDOMFizzServer', () => { expect(output.result).not.toContain('context never found'); expect(output.result).toContain('OK'); }); + + // @gate experimental + it('should not continue rendering after the writable ends unexpectedly', async () => { + let hasLoaded = false; + let resolve; + let isComplete = false; + let rendered = false; + const promise = new Promise(r => (resolve = r)); + function Wait() { + if (!hasLoaded) { + throw promise; + } + rendered = true; + return 'Done'; + } + const {writable, completed} = getTestWritable(); + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( +
+ Loading
}> + + + , + { + onAllReady() { + isComplete = true; + }, + }, + ); + pipe(writable); + + expect(rendered).toBe(false); + expect(isComplete).toBe(false); + + writable.end(); + + await jest.runAllTimers(); + + hasLoaded = true; + resolve(); + + await completed; + + expect(rendered).toBe(false); + expect(isComplete).toBe(true); + }); }); diff --git a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js index 9f23d7aa96ae0..f03825c16d442 100644 --- a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js +++ b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js @@ -155,7 +155,7 @@ module.exports = function(initModules) { new Promise((resolve, reject) => { const writable = new DrainWritable(); const s = ReactDOMServer.renderToPipeableStream(reactElement, { - onErrorShell(e) { + onShellError(e) { reject(e); }, }); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index bf90682d3c6a5..219c4a1bf7b1e 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -46,25 +46,27 @@ function renderToReadableStream( ): Promise { return new Promise((resolve, reject) => { let onFatalError; - let onCompleteAll; + let onAllReady; const allReady = new Promise((res, rej) => { - onCompleteAll = res; + onAllReady = res; onFatalError = rej; }); - function onCompleteShell() { + function onShellReady() { const stream: ReactDOMServerReadableStream = (new ReadableStream({ type: 'bytes', pull(controller) { startFlowing(request, controller); }, - cancel(reason) {}, + cancel(reason) { + abort(request); + }, }): any); // TODO: Move to sub-classing ReadableStream. stream.allReady = allReady; resolve(stream); } - function onErrorShell(error: mixed) { + function onShellError(error: mixed) { reject(error); } const request = createRequest( @@ -79,9 +81,9 @@ function renderToReadableStream( createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, - onCompleteAll, - onCompleteShell, - onErrorShell, + onAllReady, + onShellReady, + onShellError, onFatalError, ); if (options && options.signal) { diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index 39126ed8d4266..306724c448b0f 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -28,6 +28,10 @@ function createDrainHandler(destination, request) { return () => startFlowing(request, destination); } +function createAbortHandler(request) { + return () => abort(request); +} + type Options = {| identifierPrefix?: string, namespaceURI?: string, @@ -36,9 +40,9 @@ type Options = {| bootstrapScripts?: Array, bootstrapModules?: Array, progressiveChunkSize?: number, - onCompleteShell?: () => void, - onErrorShell?: () => void, - onCompleteAll?: () => void, + onShellReady?: () => void, + onShellError?: () => void, + onAllReady?: () => void, onError?: (error: mixed) => void, |}; @@ -62,9 +66,9 @@ function createRequestImpl(children: ReactNodeList, options: void | Options) { createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, - options ? options.onCompleteAll : undefined, - options ? options.onCompleteShell : undefined, - options ? options.onErrorShell : undefined, + options ? options.onAllReady : undefined, + options ? options.onShellReady : undefined, + options ? options.onShellError : undefined, undefined, ); } @@ -86,6 +90,7 @@ function renderToPipeableStream( hasStartedFlowing = true; startFlowing(request, destination); destination.on('drain', createDrainHandler(destination, request)); + destination.on('close', createAbortHandler(request)); return destination; }, abort() { diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js b/packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js index 778806c9703d7..168f38fc59db3 100644 --- a/packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js @@ -53,7 +53,7 @@ function renderToStringImpl( }; let readyToStream = false; - function onCompleteShell() { + function onShellReady() { readyToStream = true; } const request = createRequest( @@ -66,7 +66,7 @@ function renderToStringImpl( Infinity, onError, undefined, - onCompleteShell, + onShellReady, undefined, undefined, ); diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerNode.js b/packages/react-dom/src/server/ReactDOMLegacyServerNode.js index 7840708c5ad50..f5c6aa1f46005 100644 --- a/packages/react-dom/src/server/ReactDOMLegacyServerNode.js +++ b/packages/react-dom/src/server/ReactDOMLegacyServerNode.js @@ -68,7 +68,7 @@ function renderToNodeStreamImpl( options: void | ServerOptions, generateStaticMarkup: boolean, ): Readable { - function onCompleteAll() { + function onAllReady() { // We wait until everything has loaded before starting to write. // That way we only end up with fully resolved HTML even if we suspend. destination.startedFlowing = true; @@ -81,7 +81,7 @@ function renderToNodeStreamImpl( createRootFormatContext(), Infinity, onError, - onCompleteAll, + onAllReady, undefined, undefined, ); diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 94815d57f9a8f..5bd7e79cf7242 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -251,8 +251,8 @@ const ReactNoopServer = ReactFizzServer({ type Options = { progressiveChunkSize?: number, - onCompleteShell?: () => void, - onCompleteAll?: () => void, + onShellReady?: () => void, + onAllReady?: () => void, onError?: (error: mixed) => void, }; @@ -272,8 +272,8 @@ function render(children: React$Element, options?: Options): Destination { null, options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, - options ? options.onCompleteAll : undefined, - options ? options.onCompleteShell : undefined, + options ? options.onAllReady : undefined, + options ? options.onShellReady : undefined, ); ReactNoopServer.startWork(request); ReactNoopServer.startFlowing(request, destination); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 306926503d756..7a238391493a3 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -194,16 +194,16 @@ export opaque 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, - // onCompleteAll is called when all pending task is done but it may not have flushed yet. + // onAllReady is called when all pending task 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. - onCompleteAll: () => void, - // onCompleteShell is called when there is at least a root fallback ready to show. + onAllReady: () => void, + // onShellReady 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. - onCompleteShell: () => void, - // onErrorShell is called when the shell didn't complete. That means you probably want to + onShellReady: () => void, + // onShellError is called when the shell didn't complete. That means you probably want to // emit a different response to the stream instead. - onErrorShell: (error: mixed) => void, + onShellError: (error: mixed) => void, onFatalError: (error: mixed) => void, }; @@ -236,9 +236,9 @@ export function createRequest( rootFormatContext: FormatContext, progressiveChunkSize: void | number, onError: void | ((error: mixed) => void), - onCompleteAll: void | (() => void), - onCompleteShell: void | (() => void), - onErrorShell: void | ((error: mixed) => void), + onAllReady: void | (() => void), + onShellReady: void | (() => void), + onShellError: void | ((error: mixed) => void), onFatalError: void | ((error: mixed) => void), ): Request { const pingedTasks = []; @@ -262,9 +262,9 @@ export function createRequest( completedBoundaries: [], partialBoundaries: [], onError: onError === undefined ? defaultErrorHandler : onError, - onCompleteAll: onCompleteAll === undefined ? noop : onCompleteAll, - onCompleteShell: onCompleteShell === undefined ? noop : onCompleteShell, - onErrorShell: onErrorShell === undefined ? noop : onErrorShell, + onAllReady: onAllReady === undefined ? noop : onAllReady, + onShellReady: onShellReady === undefined ? noop : onShellReady, + onShellError: onShellError === undefined ? noop : onShellError, onFatalError: onFatalError === undefined ? noop : onFatalError, }; // This segment represents the root fallback. @@ -422,8 +422,10 @@ function fatalError(request: Request, error: mixed): void { // This is called outside error handling code such as if the root errors outside // a suspense boundary or if the root suspense boundary's fallback errors. // It's also called if React itself or its host configs errors. - const onErrorShell = request.onErrorShell; - onErrorShell(error); + const onShellError = request.onShellError; + onShellError(error); + const onFatalError = request.onFatalError; + onFatalError(error); if (request.destination !== null) { request.status = CLOSED; closeWithError(request.destination, error); @@ -1371,8 +1373,8 @@ function erroredTask( request.allPendingTasks--; if (request.allPendingTasks === 0) { - const onCompleteAll = request.onCompleteAll; - onCompleteAll(); + const onAllReady = request.onAllReady; + onAllReady(); } } @@ -1422,8 +1424,8 @@ function abortTask(task: Task): void { request.allPendingTasks--; if (request.allPendingTasks === 0) { - const onCompleteAll = request.onCompleteAll; - onCompleteAll(); + const onAllReady = request.onAllReady; + onAllReady(); } } } @@ -1446,9 +1448,9 @@ function finishedTask( request.pendingRootTasks--; if (request.pendingRootTasks === 0) { // We have completed the shell so the shell can't error anymore. - request.onErrorShell = noop; - const onCompleteShell = request.onCompleteShell; - onCompleteShell(); + request.onShellError = noop; + const onShellReady = request.onShellReady; + onShellReady(); } } else { boundary.pendingTasks--; @@ -1499,8 +1501,8 @@ function finishedTask( if (request.allPendingTasks === 0) { // This needs to be called at the very end so that we can synchronously write the result // in the callback if needed. - const onCompleteAll = request.onCompleteAll; - onCompleteAll(); + const onAllReady = request.onAllReady; + onAllReady(); } }