From 6b31297ddabc81506718658d6076e468b9a2f15e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 16 Aug 2023 15:53:52 -0400 Subject: [PATCH] Fizz implementation This is semantically the same as just throwing an error. It just triggers client rendering. The difference is in how it gets logged. On the server we log to onPostpone instead of onError. On the client this doesn't trigger a recoverable error. It's just silent since it was intentional. --- .../src/__tests__/ReactDOMFizzServer-test.js | 91 +++++++++++++++++++ .../ReactDOMFizzServerBrowser-test.js | 39 ++++++++ .../src/server/ReactDOMFizzServerBrowser.js | 2 + .../src/server/ReactDOMFizzServerBun.js | 2 + .../src/server/ReactDOMFizzServerEdge.js | 2 + .../src/server/ReactDOMFizzServerNode.js | 2 + .../src/server/ReactDOMFizzStaticBrowser.js | 2 + .../src/server/ReactDOMFizzStaticEdge.js | 2 + .../src/server/ReactDOMFizzStaticNode.js | 2 + .../src/server/ReactDOMLegacyServerImpl.js | 1 + .../server/ReactDOMLegacyServerNodeStream.js | 1 + .../src/ReactFiberBeginWork.js | 35 ++++--- packages/react-server/src/ReactFizzServer.js | 45 ++++++++- 13 files changed, 210 insertions(+), 16 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 188674ac6bfe8..e16b2189b37f6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -6040,4 +6040,95 @@ describe('ReactDOMFizzServer', () => { console.error = originalConsoleError; } }); + + // @gate enablePostpone + it('client renders postponed boundaries without erroring', async () => { + function Postponed({isClient}) { + if (!isClient) { + React.unstable_postpone('testing postpone'); + } + return 'client only'; + } + + function App({isClient}) { + return ( +
+ + + +
+ ); + } + + const errors = []; + + await act(() => { + const {pipe} = renderToPipeableStream(, { + onError(error) { + errors.push(error.message); + }, + }); + pipe(writable); + }); + + expect(getVisibleChildren(container)).toEqual(
loading...
); + + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + errors.push(error.message); + }, + }); + await waitForAll([]); + // Postponing should not be logged as a recoverable error since it's intentional. + expect(errors).toEqual([]); + expect(getVisibleChildren(container)).toEqual(
client only
); + }); + + // @gate enablePostpone + it('errors if trying to postpone outside a Suspense boundary', async () => { + function Postponed() { + React.unstable_postpone('testing postpone'); + return 'client only'; + } + + function App() { + return ( +
+ +
+ ); + } + + const errors = []; + const fatalErrors = []; + const postponed = []; + let written = false; + + const testWritable = new Stream.Writable(); + testWritable._write = (chunk, encoding, next) => { + written = true; + }; + + await act(() => { + const {pipe} = renderToPipeableStream(, { + onPostpone(reason) { + postponed.push(reason); + }, + onError(error) { + errors.push(error.message); + }, + onShellError(error) { + fatalErrors.push(error.message); + }, + }); + pipe(testWritable); + }); + + expect(written).toBe(false); + // Postponing is not logged as an error but as a postponed reason. + expect(errors).toEqual([]); + expect(postponed).toEqual(['testing postpone']); + // However, it does error the shell. + expect(fatalErrors).toEqual(['testing postpone']); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 9a4b798e8c609..1af024566a45c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -503,4 +503,43 @@ describe('ReactDOMFizzServerBrowser', () => { `"
hello world
"`, ); }); + + // @gate enablePostpone + it('errors if trying to postpone outside a Suspense boundary', async () => { + function Postponed() { + React.unstable_postpone('testing postpone'); + return 'client only'; + } + + function App() { + return ( +
+ +
+ ); + } + + const errors = []; + const postponed = []; + + let caughtError = null; + try { + await ReactDOMFizzServer.renderToReadableStream(, { + onError(error) { + errors.push(error.message); + }, + onPostpone(reason) { + postponed.push(reason); + }, + }); + } catch (error) { + caughtError = error; + } + + // Postponing is not logged as an error but as a postponed reason. + expect(errors).toEqual([]); + expect(postponed).toEqual(['testing postpone']); + // However, it does error the shell. + expect(caughtError.message).toEqual('testing postpone'); + }); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index 6bda14046fadb..97a1041d17539 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -35,6 +35,7 @@ type Options = { progressiveChunkSize?: number, signal?: AbortSignal, onError?: (error: mixed) => ?string, + onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, }; @@ -100,6 +101,7 @@ function renderToReadableStream( onShellReady, onShellError, onFatalError, + options ? options.onPostpone : undefined, ); if (options && options.signal) { const signal = options.signal; diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBun.js b/packages/react-dom/src/server/ReactDOMFizzServerBun.js index 73dc8cfc3b0f8..a43451eab2595 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBun.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBun.js @@ -35,6 +35,7 @@ type Options = { progressiveChunkSize?: number, signal?: AbortSignal, onError?: (error: mixed) => ?string, + onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, }; @@ -101,6 +102,7 @@ function renderToReadableStream( onShellReady, onShellError, onFatalError, + options ? options.onPostpone : undefined, ); if (options && options.signal) { const signal = options.signal; diff --git a/packages/react-dom/src/server/ReactDOMFizzServerEdge.js b/packages/react-dom/src/server/ReactDOMFizzServerEdge.js index 6bda14046fadb..97a1041d17539 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerEdge.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerEdge.js @@ -35,6 +35,7 @@ type Options = { progressiveChunkSize?: number, signal?: AbortSignal, onError?: (error: mixed) => ?string, + onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, }; @@ -100,6 +101,7 @@ function renderToReadableStream( onShellReady, onShellError, onFatalError, + options ? options.onPostpone : undefined, ); if (options && options.signal) { const signal = options.signal; diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index e1512dea07132..ecd4ca7f6fba4 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -49,6 +49,7 @@ type Options = { onShellError?: (error: mixed) => void, onAllReady?: () => void, onError?: (error: mixed) => ?string, + onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, }; @@ -80,6 +81,7 @@ function createRequestImpl(children: ReactNodeList, options: void | Options) { options ? options.onShellReady : undefined, options ? options.onShellError : undefined, undefined, + options ? options.onPostpone : undefined, ); } diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js b/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js index 5c9b637649591..f3990d2a6be0a 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js @@ -34,6 +34,7 @@ type Options = { progressiveChunkSize?: number, signal?: AbortSignal, onError?: (error: mixed) => ?string, + onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, }; @@ -85,6 +86,7 @@ function prerender( undefined, undefined, onFatalError, + options ? options.onPostpone : undefined, ); if (options && options.signal) { const signal = options.signal; diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js b/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js index 5c9b637649591..f3990d2a6be0a 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js @@ -34,6 +34,7 @@ type Options = { progressiveChunkSize?: number, signal?: AbortSignal, onError?: (error: mixed) => ?string, + onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, }; @@ -85,6 +86,7 @@ function prerender( undefined, undefined, onFatalError, + options ? options.onPostpone : undefined, ); if (options && options.signal) { const signal = options.signal; diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticNode.js b/packages/react-dom/src/server/ReactDOMFizzStaticNode.js index 156b947d8d751..9ea4e2bd7dfcd 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticNode.js @@ -36,6 +36,7 @@ type Options = { progressiveChunkSize?: number, signal?: AbortSignal, onError?: (error: mixed) => ?string, + onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, }; @@ -99,6 +100,7 @@ function prerenderToNodeStreams( undefined, undefined, onFatalError, + options ? options.onPostpone : undefined, ); if (options && options.signal) { const signal = options.signal; diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerImpl.js b/packages/react-dom/src/server/ReactDOMLegacyServerImpl.js index 719879107be2d..0485f88db231a 100644 --- a/packages/react-dom/src/server/ReactDOMLegacyServerImpl.js +++ b/packages/react-dom/src/server/ReactDOMLegacyServerImpl.js @@ -79,6 +79,7 @@ function renderToStringImpl( onShellReady, undefined, undefined, + undefined, ); startWork(request); // If anything suspended and is still pending, we'll abort it before writing. diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerNodeStream.js b/packages/react-dom/src/server/ReactDOMLegacyServerNodeStream.js index 4dc848b2962ab..bf608d4202e11 100644 --- a/packages/react-dom/src/server/ReactDOMLegacyServerNodeStream.js +++ b/packages/react-dom/src/server/ReactDOMLegacyServerNodeStream.js @@ -86,6 +86,7 @@ function renderToNodeStreamImpl( onAllReady, undefined, undefined, + undefined, ); destination.request = request; startWork(request); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index f0690a3d8d43c..3b7f3a6778ec6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -109,6 +109,7 @@ import { enableHostSingletons, enableFormActions, enableAsyncActions, + enablePostpone, } from 'shared/ReactFeatureFlags'; import isArray from 'shared/isArray'; import shallowEqual from 'shared/shallowEqual'; @@ -2859,27 +2860,33 @@ function updateDehydratedSuspenseComponent( // This boundary is in a permanent fallback state. In this case, we'll never // get an update and we'll never be able to hydrate the final content. Let's just try the // client side render instead. - let digest, message, stack; + let digest: ?string; + let message, stack; if (__DEV__) { ({digest, message, stack} = getSuspenseInstanceFallbackErrorDetails(suspenseInstance)); } else { - ({digest} = getSuspenseInstanceFallbackErrorDetails(suspenseInstance)); + digest = + getSuspenseInstanceFallbackErrorDetails(suspenseInstance).digest; } - let error; - if (message) { - // eslint-disable-next-line react-internal/prod-error-codes - error = new Error(message); - } else { - error = new Error( - 'The server could not finish this Suspense boundary, likely ' + - 'due to an error during server rendering. Switched to ' + - 'client rendering.', - ); + let capturedValue = null; + // TODO: Figure out a better signal than encoding a magic digest value. + if (!enablePostpone || digest !== 'POSTPONE') { + let error; + if (message) { + // eslint-disable-next-line react-internal/prod-error-codes + error = new Error(message); + } else { + error = new Error( + 'The server could not finish this Suspense boundary, likely ' + + 'due to an error during server rendering. Switched to ' + + 'client rendering.', + ); + } + (error: any).digest = digest; + capturedValue = createCapturedValue(error, digest, stack); } - (error: any).digest = digest; - const capturedValue = createCapturedValue(error, digest, stack); return retrySuspenseComponentWithoutHydrating( current, workInProgress, diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 68dbe68f88c90..16259a758bf47 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -128,6 +128,7 @@ import { REACT_SERVER_CONTEXT_TYPE, REACT_SCOPE_TYPE, REACT_OFFSCREEN_TYPE, + REACT_POSTPONE_TYPE, } from 'shared/ReactSymbols'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { @@ -137,12 +138,14 @@ import { enableSuspenseAvoidThisFallbackFizz, enableFloat, enableCache, + enablePostpone, } from 'shared/ReactFeatureFlags'; import assign from 'shared/assign'; import getComponentNameFromType from 'shared/getComponentNameFromType'; import isArray from 'shared/isArray'; import {SuspenseException, getSuspendedThenable} from './ReactFizzThenable'; +import type {Postpone} from 'react/src/ReactPostpone'; const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher; const ReactCurrentCache = ReactSharedInternals.ReactCurrentCache; @@ -241,6 +244,9 @@ export opaque type Request = { // emit a different response to the stream instead. onShellError: (error: mixed) => void, onFatalError: (error: mixed) => void, + // onPostpone is called when postpone() is called anywhere in the tree, which will defer + // rendering - e.g. to the client. This is considered intentional and not an error. + onPostpone: (reason: string) => void, }; // This is a default heuristic for how to split up the HTML content into progressive @@ -278,6 +284,7 @@ export function createRequest( onShellReady: void | (() => void), onShellError: void | ((error: mixed) => void), onFatalError: void | ((error: mixed) => void), + onPostpone: void | ((reason: string) => void), ): Request { prepareHostDispatcher(); const pingedTasks: Array = []; @@ -303,6 +310,7 @@ export function createRequest( completedBoundaries: ([]: Array), partialBoundaries: ([]: Array), onError: onError === undefined ? defaultErrorHandler : onError, + onPostpone: onPostpone === undefined ? noop : onPostpone, onAllReady: onAllReady === undefined ? noop : onAllReady, onShellReady: onShellReady === undefined ? noop : onShellReady, onShellError: onShellError === undefined ? noop : onShellError, @@ -508,6 +516,12 @@ function captureBoundaryErrorDetailsDev( } } +function logPostpone(request: Request, reason: string): void { + // 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.onPostpone(reason); +} + function logRecoverableError(request: Request, error: any): ?string { // 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. @@ -622,7 +636,21 @@ function renderSuspenseBoundary( } catch (error) { contentRootSegment.status = ERRORED; newBoundary.forceClientRender = true; - newBoundary.errorDigest = logRecoverableError(request, error); + let errorDigest; + if ( + enablePostpone && + typeof error === 'object' && + error !== null && + error.$$typeof === REACT_POSTPONE_TYPE + ) { + const postponeInstance: Postpone = (error: any); + logPostpone(request, postponeInstance.message); + // TODO: Figure out a better signal than a magic digest value. + errorDigest = 'POSTPONE'; + } else { + errorDigest = logRecoverableError(request, error); + } + newBoundary.errorDigest = errorDigest; if (__DEV__) { captureBoundaryErrorDetailsDev(newBoundary, error); } @@ -1678,7 +1706,20 @@ function erroredTask( error: mixed, ) { // Report the error to a global handler. - const errorDigest = logRecoverableError(request, error); + let errorDigest; + if ( + enablePostpone && + typeof error === 'object' && + error !== null && + error.$$typeof === REACT_POSTPONE_TYPE + ) { + const postponeInstance: Postpone = (error: any); + logPostpone(request, postponeInstance.message); + // TODO: Figure out a better signal than a magic digest value. + errorDigest = 'POSTPONE'; + } else { + errorDigest = logRecoverableError(request, error); + } if (boundary === null) { fatalError(request, error); } else {