From a563112766b8f9d3657a865caf77c428b99e279b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 9 Aug 2023 13:35:31 -0400 Subject: [PATCH 01/10] Add postpone flag and signaling API --- packages/react/index.experimental.js | 1 + packages/react/src/React.js | 2 ++ packages/react/src/ReactPostpone.js | 24 +++++++++++++++++++ .../src/ReactSharedSubset.experimental.js | 1 + packages/shared/ReactFeatureFlags.js | 2 ++ packages/shared/ReactSymbols.js | 2 ++ .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 2 ++ 12 files changed, 39 insertions(+) create mode 100644 packages/react/src/ReactPostpone.js diff --git a/packages/react/index.experimental.js b/packages/react/index.experimental.js index 4bf5f67b74e..081fa274696 100644 --- a/packages/react/index.experimental.js +++ b/packages/react/index.experimental.js @@ -33,6 +33,7 @@ export { unstable_Cache, unstable_DebugTracingMode, unstable_Offscreen, + unstable_postpone, unstable_getCacheSignal, unstable_getCacheForType, unstable_SuspenseList, diff --git a/packages/react/src/React.js b/packages/react/src/React.js index aff0401792f..b45a0cda053 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -36,6 +36,7 @@ import {lazy} from './ReactLazy'; import {forwardRef} from './ReactForwardRef'; import {memo} from './ReactMemo'; import {cache} from './ReactCache'; +import {postpone} from './ReactPostpone'; import { getCacheSignal, getCacheForType, @@ -100,6 +101,7 @@ export { lazy, memo, cache, + postpone as unstable_postpone, useCallback, useContext, useEffect, diff --git a/packages/react/src/ReactPostpone.js b/packages/react/src/ReactPostpone.js new file mode 100644 index 00000000000..6a706dcab14 --- /dev/null +++ b/packages/react/src/ReactPostpone.js @@ -0,0 +1,24 @@ +/** + * Copyright (c) Meta Platforms, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {REACT_POSTPONE_TYPE} from 'shared/ReactSymbols'; + +export type Postpone = { + $$typeof: symbol, + message: string, + stack: string, + ... +}; + +export function postpone(reason: string): void { + // eslint-disable-next-line react-internal/prod-error-codes + const error = new Error(reason); + (error: any).$$typeof = REACT_POSTPONE_TYPE; + throw error; +} diff --git a/packages/react/src/ReactSharedSubset.experimental.js b/packages/react/src/ReactSharedSubset.experimental.js index 460c1fa19a6..ed5340df9de 100644 --- a/packages/react/src/ReactSharedSubset.experimental.js +++ b/packages/react/src/ReactSharedSubset.experimental.js @@ -32,6 +32,7 @@ export { unstable_SuspenseList, unstable_getCacheSignal, unstable_getCacheForType, + unstable_postpone, useId, useCallback, useContext, diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 3813798d09b..44dc2ce07c0 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -86,6 +86,8 @@ export const enableFormActions = __EXPERIMENTAL__; export const enableBinaryFlight = __EXPERIMENTAL__; +export const enablePostpone = __EXPERIMENTAL__; + export const enableTransitionTracing = false; // No known bugs, but needs performance testing diff --git a/packages/shared/ReactSymbols.js b/packages/shared/ReactSymbols.js index 503c6ef0c41..76df61b393a 100644 --- a/packages/shared/ReactSymbols.js +++ b/packages/shared/ReactSymbols.js @@ -49,6 +49,8 @@ export const REACT_MEMO_CACHE_SENTINEL: symbol = Symbol.for( 'react.memo_cache_sentinel', ); +export const REACT_POSTPONE_TYPE: symbol = Symbol.for('react.postpone'); + const MAYBE_ITERATOR_SYMBOL = Symbol.iterator; const FAUX_ITERATOR_SYMBOL = '@@iterator'; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 70909c55a03..ec7d4e6cf58 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -37,6 +37,7 @@ export const enableCacheElement = true; export const enableFetchInstrumentation = false; export const enableFormActions = true; // Doesn't affect Native export const enableBinaryFlight = true; +export const enablePostpone = false; export const enableSchedulerDebugging = false; export const debugRenderPhaseSideEffectsForStrictMode = true; export const disableJavaScriptURLs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 675f937d58d..c70b20b57fd 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -25,6 +25,7 @@ export const enableCacheElement = false; export const enableFetchInstrumentation = false; export const enableFormActions = true; // Doesn't affect Native export const enableBinaryFlight = true; +export const enablePostpone = false; export const disableJavaScriptURLs = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index e83c4dab44b..86f2d6ff6d3 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -25,6 +25,7 @@ export const enableCacheElement = __EXPERIMENTAL__; export const enableFetchInstrumentation = true; export const enableFormActions = true; // Doesn't affect Test Renderer export const enableBinaryFlight = true; +export const enablePostpone = false; export const disableJavaScriptURLs = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 9ddc9004125..50717886640 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -25,6 +25,7 @@ export const enableCacheElement = true; export const enableFetchInstrumentation = false; export const enableFormActions = true; // Doesn't affect Test Renderer export const enableBinaryFlight = true; +export const enablePostpone = false; export const disableJavaScriptURLs = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index bdfc1acd77e..7ac6b230bca 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -25,6 +25,7 @@ export const enableCacheElement = true; export const enableFetchInstrumentation = false; export const enableFormActions = true; // Doesn't affect Test Renderer export const enableBinaryFlight = true; +export const enablePostpone = false; export const enableSchedulerDebugging = false; export const disableJavaScriptURLs = false; export const disableCommentsAsDOMContainers = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index c2908e4335b..466e0a5aa18 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -77,6 +77,8 @@ export const enableFormActions = false; export const enableBinaryFlight = true; +export const enablePostpone = false; + export const disableJavaScriptURLs = true; // TODO: www currently relies on this feature. It's disabled in open source. From 9f92cd3a39f44dad40c62680beb21790e606cd93 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 15 Aug 2023 20:52:26 -0400 Subject: [PATCH 02/10] Add test for Flight and Fiber implementation --- .../__tests__/ReactFlightDOMBrowser-test.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index a1d67eba7d9..96eab515d5f 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -1178,4 +1178,48 @@ describe('ReactFlightDOMBrowser', () => { '

hello world

', ); }); + + // @gate enablePostpone + it('supports postpone in Server Components', async () => { + function Server() { + React.unstable_postpone('testing postpone'); + return 'Not shown'; + } + + let postponed = null; + + const stream = ReactServerDOMServer.renderToReadableStream( + + + , + null, + { + onPostpone(reason) { + postponed = reason; + }, + }, + ); + const response = ReactServerDOMClient.createFromReadableStream(stream); + + function Client() { + return use(response); + } + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render( +
+ Shell: +
, + ); + }); + // We should have reserved the shell already. Which means that the Server + // Component should've been a lazy component. + expect(container.innerHTML).toContain('Shell:'); + expect(container.innerHTML).toContain('Loading...'); + expect(container.innerHTML).not.toContain('Not shown'); + + expect(postponed).toBe('testing postpone'); + }); }); From c6c14282ad9f1840d24a1a59e574a0782c817272 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 15 Aug 2023 22:04:57 -0400 Subject: [PATCH 03/10] Implement in Flight Server Serialize as P. In dev we include reason and stack. Emit onPostpone event if it happens with the reason. --- .../react-server/src/ReactFlightServer.js | 64 ++++++++++++++++++- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 20031655073..0d0e0ee4428 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -9,7 +9,9 @@ import type {Chunk, BinaryChunk, Destination} from './ReactServerStreamConfig'; -import {enableBinaryFlight} from 'shared/ReactFeatureFlags'; +import type {Postpone} from 'react/src/ReactPostpone'; + +import {enableBinaryFlight, enablePostpone} from 'shared/ReactFeatureFlags'; import { scheduleWork, @@ -87,6 +89,7 @@ import { REACT_FRAGMENT_TYPE, REACT_LAZY_TYPE, REACT_MEMO_TYPE, + REACT_POSTPONE_TYPE, REACT_PROVIDER_TYPE, } from 'shared/ReactSymbols'; @@ -189,6 +192,7 @@ export type Request = { identifierPrefix: string, identifierCount: number, onError: (error: mixed) => ?string, + onPostpone: (reason: string) => void, toJSON: (key: string, value: ReactClientValue) => ReactJSONValue, }; @@ -200,6 +204,10 @@ function defaultErrorHandler(error: mixed) { // Don't transform to our wrapper } +function defaultPostponeHandler(reason: string) { + // Noop +} + const OPEN = 0; const CLOSING = 1; const CLOSED = 2; @@ -210,6 +218,7 @@ export function createRequest( onError: void | ((error: mixed) => ?string), context?: Array<[string, ServerContextJSONValue]>, identifierPrefix?: string, + onPostpone: void | ((reason: string) => void), ): Request { if ( ReactCurrentCache.current !== null && @@ -248,6 +257,7 @@ export function createRequest( identifierPrefix: identifierPrefix || '', identifierCount: 1, onError: onError === undefined ? defaultErrorHandler : onError, + onPostpone: onPostpone === undefined ? defaultPostponeHandler : onPostpone, // $FlowFixMe[missing-this-annot] toJSON: function (key: string, value: ReactClientValue): ReactJSONValue { return resolveModelToJSON(request, this, key, value); @@ -297,8 +307,14 @@ function serializeThenable(request: Request, thenable: Thenable): number { } case 'rejected': { const x = thenable.reason; - const digest = logRecoverableError(request, x); - emitErrorChunk(request, newTask.id, digest, x); + if (enablePostpone && (x: any).$$typeof === REACT_POSTPONE_TYPE) { + const postponeInstance: Postpone = (x: any); + logPostpone(request, postponeInstance.message); + emitPostponeChunk(request, newTask.id, postponeInstance); + } else { + const digest = logRecoverableError(request, x); + emitErrorChunk(request, newTask.id, digest, x); + } return newTask.id; } default: { @@ -907,6 +923,15 @@ function resolveModelToJSON( x.then(ping, ping); newTask.thenableState = getThenableStateAfterSuspending(); return serializeLazyID(newTask.id); + } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { + // Something postponed. We'll still send everything we have up until this point. + // We'll replace this element with a lazy reference that postpones on the client. + const postponeInstance: Postpone = (x: any); + request.pendingChunks++; + const postponeId = request.nextChunkId++; + logPostpone(request, postponeInstance.message); + emitPostponeChunk(request, postponeId, postponeInstance); + return serializeLazyID(postponeId); } else { // Something errored. We'll still send everything we have up until this point. // We'll replace this element with a lazy reference that throws on the client @@ -1146,6 +1171,11 @@ function resolveModelToJSON( ); } +function logPostpone(request: Request, reason: string): void { + const onPostpone = request.onPostpone; + onPostpone(reason); +} + function logRecoverableError(request: Request, error: mixed): string { const onError = request.onError; const errorDigest = onError(error); @@ -1169,6 +1199,30 @@ function fatalError(request: Request, error: mixed): void { } } +function emitPostponeChunk( + request: Request, + id: number, + postponeInstance: Postpone, +): void { + let row; + if (__DEV__) { + let reason = ''; + let stack = ''; + try { + // eslint-disable-next-line react-internal/safe-string-coercion + reason = String(postponeInstance.message); + // eslint-disable-next-line react-internal/safe-string-coercion + stack = String(postponeInstance.stack); + } catch (x) {} + row = serializeRowHeader('P', id) + stringify({reason, stack}) + '\n'; + } else { + // No reason included in prod. + row = serializeRowHeader('P', id) + '\n'; + } + const processedChunk = stringToChunk(row); + request.completedErrorChunks.push(processedChunk); +} + function emitErrorChunk( request: Request, id: number, @@ -1328,6 +1382,10 @@ function retryTask(request: Request, task: Task): void { x.then(ping, ping); task.thenableState = getThenableStateAfterSuspending(); return; + } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { + const postponeInstance: Postpone = (x: any); + logPostpone(request, postponeInstance.message); + emitPostponeChunk(request, task.id, postponeInstance); } else { request.abortableTasks.delete(task); task.status = ERRORED; From 7aa0b9f378177291b4c384b3d99fae0a0a61236c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 15 Aug 2023 22:45:34 -0400 Subject: [PATCH 04/10] Expose onPostpone option --- packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js | 2 ++ .../react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js | 2 ++ .../react-server-dom-webpack/src/ReactFlightDOMServerEdge.js | 2 ++ .../react-server-dom-webpack/src/ReactFlightDOMServerNode.js | 2 ++ 4 files changed, 8 insertions(+) diff --git a/packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js b/packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js index 5b9c785a068..def3a58478e 100644 --- a/packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js @@ -49,6 +49,7 @@ function createDrainHandler(destination: Destination, request: Request) { type Options = { onError?: (error: mixed) => void, + onPostpone?: (reason: string) => void, context?: Array<[string, ServerContextJSONValue]>, identifierPrefix?: string, }; @@ -69,6 +70,7 @@ function renderToPipeableStream( options ? options.onError : undefined, options ? options.context : undefined, options ? options.identifierPrefix : undefined, + options ? options.onPostpone : undefined, ); let hasStartedFlowing = false; startWork(request); diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js index 1042ab3f51c..08214a4182a 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js @@ -38,6 +38,7 @@ type Options = { signal?: AbortSignal, context?: Array<[string, ServerContextJSONValue]>, onError?: (error: mixed) => void, + onPostpone?: (reason: string) => void, }; function renderToReadableStream( @@ -51,6 +52,7 @@ function renderToReadableStream( options ? options.onError : undefined, options ? options.context : undefined, options ? options.identifierPrefix : undefined, + options ? options.onPostpone : undefined, ); if (options && options.signal) { const signal = options.signal; diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerEdge.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerEdge.js index 1042ab3f51c..08214a4182a 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerEdge.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerEdge.js @@ -38,6 +38,7 @@ type Options = { signal?: AbortSignal, context?: Array<[string, ServerContextJSONValue]>, onError?: (error: mixed) => void, + onPostpone?: (reason: string) => void, }; function renderToReadableStream( @@ -51,6 +52,7 @@ function renderToReadableStream( options ? options.onError : undefined, options ? options.context : undefined, options ? options.identifierPrefix : undefined, + options ? options.onPostpone : undefined, ); if (options && options.signal) { const signal = options.signal; diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js index 275312cceaf..1e39d000ffe 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js @@ -50,6 +50,7 @@ function createDrainHandler(destination: Destination, request: Request) { type Options = { onError?: (error: mixed) => void, + onPostpone?: (reason: string) => void, context?: Array<[string, ServerContextJSONValue]>, identifierPrefix?: string, }; @@ -70,6 +71,7 @@ function renderToPipeableStream( options ? options.onError : undefined, options ? options.context : undefined, options ? options.identifierPrefix : undefined, + options ? options.onPostpone : undefined, ); let hasStartedFlowing = false; startWork(request); From 464c49d86060232a3f699cb37305dc9c5b26eb05 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 15 Aug 2023 22:16:33 -0400 Subject: [PATCH 05/10] Implement Flight Client Deserialize into the same shape as postpone(). Could actually call it potentially. --- .../react-client/src/ReactFlightClient.js | 80 ++++++++++++++++++- scripts/error-codes/codes.json | 5 +- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 522a11e6d9d..ea9b55cda18 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -21,7 +21,9 @@ import type {HintModel} from 'react-server/src/ReactFlightServerConfig'; import type {CallServerCallback} from './ReactFlightReplyClient'; -import {enableBinaryFlight} from 'shared/ReactFeatureFlags'; +import type {Postpone} from 'react/src/ReactPostpone'; + +import {enableBinaryFlight, enablePostpone} from 'shared/ReactFeatureFlags'; import { resolveClientReference, @@ -39,7 +41,11 @@ import { knownServerReferences, } from './ReactFlightReplyClient'; -import {REACT_LAZY_TYPE, REACT_ELEMENT_TYPE} from 'shared/ReactSymbols'; +import { + REACT_LAZY_TYPE, + REACT_ELEMENT_TYPE, + REACT_POSTPONE_TYPE, +} from 'shared/ReactSymbols'; import {getOrCreateServerContext} from 'shared/ReactServerContextRegistry'; @@ -226,7 +232,7 @@ function createBlockedChunk(response: Response): BlockedChunk { function createErrorChunk( response: Response, - error: ErrorWithDigest, + error: Error | Postpone, ): ErroredChunk { // $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors return new Chunk(ERRORED, null, error, response); @@ -867,6 +873,57 @@ function resolveErrorDev( } } +function resolvePostponeProd(response: Response, id: number): void { + if (__DEV__) { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'resolveErrorProd should never be called in development mode. Use resolveErrorDev instead. This is a bug in React.', + ); + } + const error = new Error( + 'A Server Component was postponed. The reason is omitted in production' + + ' builds to avoid leaking sensitive details.', + ); + const postponeInstance: Postpone = (error: any); + postponeInstance.$$typeof = REACT_POSTPONE_TYPE; + postponeInstance.stack = 'Error: ' + error.message; + const chunks = response._chunks; + const chunk = chunks.get(id); + if (!chunk) { + chunks.set(id, createErrorChunk(response, postponeInstance)); + } else { + triggerErrorOnChunk(chunk, postponeInstance); + } +} + +function resolvePostponeDev( + response: Response, + id: number, + reason: string, + stack: string, +): void { + if (!__DEV__) { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'resolveErrorDev should never be called in production mode. Use resolveErrorProd instead. This is a bug in React.', + ); + } + // eslint-disable-next-line react-internal/prod-error-codes + const error = new Error(reason || ''); + const postponeInstance: Postpone = (error: any); + postponeInstance.$$typeof = REACT_POSTPONE_TYPE; + postponeInstance.stack = stack; + const chunks = response._chunks; + const chunk = chunks.get(id); + if (!chunk) { + chunks.set(id, createErrorChunk(response, postponeInstance)); + } else { + triggerErrorOnChunk(chunk, postponeInstance); + } +} + function resolveHint( response: Response, code: string, @@ -1019,6 +1076,23 @@ function processFullRow( resolveText(response, id, row); return; } + case 80 /* "P" */: { + if (enablePostpone) { + if (__DEV__) { + const postponeInfo = JSON.parse(row); + resolvePostponeDev( + response, + id, + postponeInfo.reason, + postponeInfo.stack, + ); + } else { + resolvePostponeProd(response, id); + } + return; + } + } + // Fallthrough default: /* """ "{" "[" "t" "f" "n" "0" - "9" */ { // We assume anything else is JSON. resolveModel(response, id, row); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 395b61cc701..efa16f3b35a 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -468,5 +468,6 @@ "480": "File/Blob fields are not yet supported in progressive forms. It probably means you are closing over binary data or FormData in a Server Action.", "481": "Tried to encode a Server Action from a different instance than the encoder is from. This is a bug in React.", "482": "async/await is not yet supported in Client Components, only Server Components. This error is often caused by accidentally adding `'use client'` to a module that was originally written for the server.", - "483": "Hooks are not supported inside an async component. This error is often caused by accidentally adding `'use client'` to a module that was originally written for the server." -} + "483": "Hooks are not supported inside an async component. This error is often caused by accidentally adding `'use client'` to a module that was originally written for the server.", + "484": "A Server Component was postponed. The reason is omitted in production builds to avoid leaking sensitive details." +} \ No newline at end of file From e201f4513c206c20913141bd8647857379dc4904 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 15 Aug 2023 22:30:31 -0400 Subject: [PATCH 06/10] Implement in Fiber This just uses a fake infinite promise as a short cut. We could be more clever in the code here and not create any listeners at all. --- .../react-reconciler/src/ReactFiberThrow.js | 351 +++++++++--------- 1 file changed, 178 insertions(+), 173 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index d32cb7d5f3a..a6321042684 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -41,6 +41,7 @@ import { enableDebugTracing, enableLazyContextPropagation, enableUpdaterTracking, + enablePostpone, } from 'shared/ReactFeatureFlags'; import {createCapturedValueAtFiber} from './ReactCapturedValue'; import { @@ -82,6 +83,7 @@ import { } from './ReactFiberHydrationContext'; import {ConcurrentRoot} from './ReactRootTags'; import {noopSuspenseyCommitThenable} from './ReactFiberThenable'; +import {REACT_POSTPONE_TYPE} from 'shared/ReactSymbols'; function createRootErrorUpdate( fiber: Fiber, @@ -332,210 +334,213 @@ function throwException( } } - if ( - value !== null && - typeof value === 'object' && - typeof value.then === 'function' - ) { - // This is a wakeable. The component suspended. - const wakeable: Wakeable = (value: any); - resetSuspendedComponent(sourceFiber, rootRenderLanes); + if (value !== null && typeof value === 'object') { + if (enablePostpone && value.$$typeof === REACT_POSTPONE_TYPE) { + // Act as if this is an infinitely suspending promise. + value = {then: function () {}}; + } + if (typeof value.then === 'function') { + // This is a wakeable. The component suspended. + const wakeable: Wakeable = (value: any); + resetSuspendedComponent(sourceFiber, rootRenderLanes); - if (__DEV__) { - if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { - markDidThrowWhileHydratingDEV(); + if (__DEV__) { + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidThrowWhileHydratingDEV(); + } } - } - if (__DEV__) { - if (enableDebugTracing) { - if (sourceFiber.mode & DebugTracingMode) { - const name = getComponentNameFromFiber(sourceFiber) || 'Unknown'; - logComponentSuspended(name, wakeable); + if (__DEV__) { + if (enableDebugTracing) { + if (sourceFiber.mode & DebugTracingMode) { + const name = getComponentNameFromFiber(sourceFiber) || 'Unknown'; + logComponentSuspended(name, wakeable); + } } } - } - // Mark the nearest Suspense boundary to switch to rendering a fallback. - const suspenseBoundary = getSuspenseHandler(); - if (suspenseBoundary !== null) { - switch (suspenseBoundary.tag) { - case SuspenseComponent: { - // If this suspense boundary is not already showing a fallback, mark - // the in-progress render as suspended. We try to perform this logic - // as soon as soon as possible during the render phase, so the work - // loop can know things like whether it's OK to switch to other tasks, - // or whether it can wait for data to resolve before continuing. - // TODO: Most of these checks are already performed when entering a - // Suspense boundary. We should track the information on the stack so - // we don't have to recompute it on demand. This would also allow us - // to unify with `use` which needs to perform this logic even sooner, - // before `throwException` is called. - if (sourceFiber.mode & ConcurrentMode) { - if (getShellBoundary() === null) { - // Suspended in the "shell" of the app. This is an undesirable - // loading state. We should avoid committing this tree. - renderDidSuspendDelayIfPossible(); - } else { - // If we suspended deeper than the shell, we don't need to delay - // the commmit. However, we still call renderDidSuspend if this is - // a new boundary, to tell the work loop that a new fallback has - // appeared during this render. - // TODO: Theoretically we should be able to delete this branch. - // It's currently used for two things: 1) to throttle the - // appearance of successive loading states, and 2) in - // SuspenseList, to determine whether the children include any - // pending fallbacks. For 1, we should apply throttling to all - // retries, not just ones that render an additional fallback. For - // 2, we should check subtreeFlags instead. Then we can delete - // this branch. - const current = suspenseBoundary.alternate; - if (current === null) { - renderDidSuspend(); + // Mark the nearest Suspense boundary to switch to rendering a fallback. + const suspenseBoundary = getSuspenseHandler(); + if (suspenseBoundary !== null) { + switch (suspenseBoundary.tag) { + case SuspenseComponent: { + // If this suspense boundary is not already showing a fallback, mark + // the in-progress render as suspended. We try to perform this logic + // as soon as soon as possible during the render phase, so the work + // loop can know things like whether it's OK to switch to other tasks, + // or whether it can wait for data to resolve before continuing. + // TODO: Most of these checks are already performed when entering a + // Suspense boundary. We should track the information on the stack so + // we don't have to recompute it on demand. This would also allow us + // to unify with `use` which needs to perform this logic even sooner, + // before `throwException` is called. + if (sourceFiber.mode & ConcurrentMode) { + if (getShellBoundary() === null) { + // Suspended in the "shell" of the app. This is an undesirable + // loading state. We should avoid committing this tree. + renderDidSuspendDelayIfPossible(); + } else { + // If we suspended deeper than the shell, we don't need to delay + // the commmit. However, we still call renderDidSuspend if this is + // a new boundary, to tell the work loop that a new fallback has + // appeared during this render. + // TODO: Theoretically we should be able to delete this branch. + // It's currently used for two things: 1) to throttle the + // appearance of successive loading states, and 2) in + // SuspenseList, to determine whether the children include any + // pending fallbacks. For 1, we should apply throttling to all + // retries, not just ones that render an additional fallback. For + // 2, we should check subtreeFlags instead. Then we can delete + // this branch. + const current = suspenseBoundary.alternate; + if (current === null) { + renderDidSuspend(); + } } } - } - suspenseBoundary.flags &= ~ForceClientRender; - markSuspenseBoundaryShouldCapture( - suspenseBoundary, - returnFiber, - sourceFiber, - root, - rootRenderLanes, - ); - // Retry listener - // - // If the fallback does commit, we need to attach a different type of - // listener. This one schedules an update on the Suspense boundary to - // turn the fallback state off. - // - // Stash the wakeable on the boundary fiber so we can access it in the - // commit phase. - // - // When the wakeable resolves, we'll attempt to render the boundary - // again ("retry"). + suspenseBoundary.flags &= ~ForceClientRender; + markSuspenseBoundaryShouldCapture( + suspenseBoundary, + returnFiber, + sourceFiber, + root, + rootRenderLanes, + ); + // Retry listener + // + // If the fallback does commit, we need to attach a different type of + // listener. This one schedules an update on the Suspense boundary to + // turn the fallback state off. + // + // Stash the wakeable on the boundary fiber so we can access it in the + // commit phase. + // + // When the wakeable resolves, we'll attempt to render the boundary + // again ("retry"). - // Check if this is a Suspensey resource. We do not attach retry - // listeners to these, because we don't actually need them for - // rendering. Only for committing. Instead, if a fallback commits - // and the only thing that suspended was a Suspensey resource, we - // retry immediately. - // TODO: Refactor throwException so that we don't have to do this type - // check. The caller already knows what the cause was. - const isSuspenseyResource = wakeable === noopSuspenseyCommitThenable; - if (isSuspenseyResource) { - suspenseBoundary.flags |= ScheduleRetry; - } else { - const retryQueue: RetryQueue | null = - (suspenseBoundary.updateQueue: any); - if (retryQueue === null) { - suspenseBoundary.updateQueue = new Set([wakeable]); - } else { - retryQueue.add(wakeable); - } - } - break; - } - case OffscreenComponent: { - if (suspenseBoundary.mode & ConcurrentMode) { - suspenseBoundary.flags |= ShouldCapture; + // Check if this is a Suspensey resource. We do not attach retry + // listeners to these, because we don't actually need them for + // rendering. Only for committing. Instead, if a fallback commits + // and the only thing that suspended was a Suspensey resource, we + // retry immediately. + // TODO: Refactor throwException so that we don't have to do this type + // check. The caller already knows what the cause was. const isSuspenseyResource = wakeable === noopSuspenseyCommitThenable; if (isSuspenseyResource) { suspenseBoundary.flags |= ScheduleRetry; } else { - const offscreenQueue: OffscreenQueue | null = + const retryQueue: RetryQueue | null = (suspenseBoundary.updateQueue: any); - if (offscreenQueue === null) { - const newOffscreenQueue: OffscreenQueue = { - transitions: null, - markerInstances: null, - retryQueue: new Set([wakeable]), - }; - suspenseBoundary.updateQueue = newOffscreenQueue; + if (retryQueue === null) { + suspenseBoundary.updateQueue = new Set([wakeable]); + } else { + retryQueue.add(wakeable); + } + } + break; + } + case OffscreenComponent: { + if (suspenseBoundary.mode & ConcurrentMode) { + suspenseBoundary.flags |= ShouldCapture; + const isSuspenseyResource = + wakeable === noopSuspenseyCommitThenable; + if (isSuspenseyResource) { + suspenseBoundary.flags |= ScheduleRetry; } else { - const retryQueue = offscreenQueue.retryQueue; - if (retryQueue === null) { - offscreenQueue.retryQueue = new Set([wakeable]); + const offscreenQueue: OffscreenQueue | null = + (suspenseBoundary.updateQueue: any); + if (offscreenQueue === null) { + const newOffscreenQueue: OffscreenQueue = { + transitions: null, + markerInstances: null, + retryQueue: new Set([wakeable]), + }; + suspenseBoundary.updateQueue = newOffscreenQueue; } else { - retryQueue.add(wakeable); + const retryQueue = offscreenQueue.retryQueue; + if (retryQueue === null) { + offscreenQueue.retryQueue = new Set([wakeable]); + } else { + retryQueue.add(wakeable); + } } } + break; } - break; + // Fall through + } + default: { + throw new Error( + `Unexpected Suspense handler tag (${suspenseBoundary.tag}). This ` + + 'is a bug in React.', + ); } - // Fall through } - default: { - throw new Error( - `Unexpected Suspense handler tag (${suspenseBoundary.tag}). This ` + - 'is a bug in React.', - ); + // We only attach ping listeners in concurrent mode. Legacy Suspense always + // commits fallbacks synchronously, so there are no pings. + if (suspenseBoundary.mode & ConcurrentMode) { + attachPingListener(root, wakeable, rootRenderLanes); } - } - // We only attach ping listeners in concurrent mode. Legacy Suspense always - // commits fallbacks synchronously, so there are no pings. - if (suspenseBoundary.mode & ConcurrentMode) { - attachPingListener(root, wakeable, rootRenderLanes); - } - return; - } else { - // No boundary was found. Unless this is a sync update, this is OK. - // We can suspend and wait for more data to arrive. - - if (root.tag === ConcurrentRoot) { - // In a concurrent root, suspending without a Suspense boundary is - // allowed. It will suspend indefinitely without committing. - // - // TODO: Should we have different behavior for discrete updates? What - // about flushSync? Maybe it should put the tree into an inert state, - // and potentially log a warning. Revisit this for a future release. - attachPingListener(root, wakeable, rootRenderLanes); - renderDidSuspendDelayIfPossible(); return; } else { - // In a legacy root, suspending without a boundary is always an error. - const uncaughtSuspenseError = new Error( - 'A component suspended while responding to synchronous input. This ' + - 'will cause the UI to be replaced with a loading indicator. To ' + - 'fix, updates that suspend should be wrapped ' + - 'with startTransition.', - ); - value = uncaughtSuspenseError; + // No boundary was found. Unless this is a sync update, this is OK. + // We can suspend and wait for more data to arrive. + + if (root.tag === ConcurrentRoot) { + // In a concurrent root, suspending without a Suspense boundary is + // allowed. It will suspend indefinitely without committing. + // + // TODO: Should we have different behavior for discrete updates? What + // about flushSync? Maybe it should put the tree into an inert state, + // and potentially log a warning. Revisit this for a future release. + attachPingListener(root, wakeable, rootRenderLanes); + renderDidSuspendDelayIfPossible(); + return; + } else { + // In a legacy root, suspending without a boundary is always an error. + const uncaughtSuspenseError = new Error( + 'A component suspended while responding to synchronous input. This ' + + 'will cause the UI to be replaced with a loading indicator. To ' + + 'fix, updates that suspend should be wrapped ' + + 'with startTransition.', + ); + value = uncaughtSuspenseError; + } } } - } else { - // This is a regular error, not a Suspense wakeable. - if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { - markDidThrowWhileHydratingDEV(); - const suspenseBoundary = getSuspenseHandler(); - // If the error was thrown during hydration, we may be able to recover by - // discarding the dehydrated content and switching to a client render. - // Instead of surfacing the error, find the nearest Suspense boundary - // and render it again without hydration. - if (suspenseBoundary !== null) { - if ((suspenseBoundary.flags & ShouldCapture) === NoFlags) { - // Set a flag to indicate that we should try rendering the normal - // children again, not the fallback. - suspenseBoundary.flags |= ForceClientRender; - } - markSuspenseBoundaryShouldCapture( - suspenseBoundary, - returnFiber, - sourceFiber, - root, - rootRenderLanes, - ); + } - // Even though the user may not be affected by this error, we should - // still log it so it can be fixed. - queueHydrationError(createCapturedValueAtFiber(value, sourceFiber)); - return; + // This is a regular error, not a Suspense wakeable. + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidThrowWhileHydratingDEV(); + const suspenseBoundary = getSuspenseHandler(); + // If the error was thrown during hydration, we may be able to recover by + // discarding the dehydrated content and switching to a client render. + // Instead of surfacing the error, find the nearest Suspense boundary + // and render it again without hydration. + if (suspenseBoundary !== null) { + if ((suspenseBoundary.flags & ShouldCapture) === NoFlags) { + // Set a flag to indicate that we should try rendering the normal + // children again, not the fallback. + suspenseBoundary.flags |= ForceClientRender; } - } else { - // Otherwise, fall through to the error path. + markSuspenseBoundaryShouldCapture( + suspenseBoundary, + returnFiber, + sourceFiber, + root, + rootRenderLanes, + ); + + // Even though the user may not be affected by this error, we should + // still log it so it can be fixed. + queueHydrationError(createCapturedValueAtFiber(value, sourceFiber)); + return; } + } else { + // Otherwise, fall through to the error path. } value = createCapturedValueAtFiber(value, sourceFiber); From eec516ebd2ec1003a520892e3ee6c89cb00ee5f6 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 16 Aug 2023 15:26:18 -0400 Subject: [PATCH 07/10] Conceptually this extends Error We just don't use native classes for this and brand check with $$typeof. --- packages/react/src/ReactPostpone.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/react/src/ReactPostpone.js b/packages/react/src/ReactPostpone.js index 6a706dcab14..0496d2985b3 100644 --- a/packages/react/src/ReactPostpone.js +++ b/packages/react/src/ReactPostpone.js @@ -9,16 +9,15 @@ import {REACT_POSTPONE_TYPE} from 'shared/ReactSymbols'; -export type Postpone = { - $$typeof: symbol, - message: string, - stack: string, - ... -}; +declare class Postpone extends Error { + $$typeof: symbol; +} + +export type {Postpone}; export function postpone(reason: string): void { // eslint-disable-next-line react-internal/prod-error-codes - const error = new Error(reason); - (error: any).$$typeof = REACT_POSTPONE_TYPE; - throw error; + const postponeInstance: Postpone = (new Error(reason): any); + postponeInstance.$$typeof = REACT_POSTPONE_TYPE; + throw postponeInstance; } From 2380013ff93ab1238da38fd7c09b1b3215f7354e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 16 Aug 2023 15:53:52 -0400 Subject: [PATCH 08/10] 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 | 32 ++++--- packages/react-server/src/ReactFizzServer.js | 45 ++++++++- 13 files changed, 208 insertions(+), 15 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 188674ac6bf..e16b2189b37 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 9a4b798e8c6..1af024566a4 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 6bda14046fa..97a1041d175 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 73dc8cfc3b0..a43451eab25 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 6bda14046fa..97a1041d175 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 e1512dea071..ecd4ca7f6fb 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 5c9b6376495..f3990d2a6be 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 5c9b6376495..f3990d2a6be 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 156b947d8d7..9ea4e2bd7df 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 719879107be..0485f88db23 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 4dc848b2962..bf608d4202e 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 f0690a3d8d4..57a86e6984d 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,7 +2860,8 @@ 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)); @@ -2867,19 +2869,23 @@ function updateDehydratedSuspenseComponent( ({digest} = getSuspenseInstanceFallbackErrorDetails(suspenseInstance)); } - 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 68dbe68f88c..16259a758bf 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 { From da3919db87c8fc5b878b88e475cbc6cdd346a11a Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 17 Aug 2023 13:01:56 -0400 Subject: [PATCH 09/10] Null checks and set status code --- .../react-server/src/ReactFlightServer.js | 108 ++++++++++-------- 1 file changed, 59 insertions(+), 49 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 0d0e0ee4428..cb62cd0cb1f 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -307,7 +307,12 @@ function serializeThenable(request: Request, thenable: Thenable): number { } case 'rejected': { const x = thenable.reason; - if (enablePostpone && (x: any).$$typeof === REACT_POSTPONE_TYPE) { + if ( + enablePostpone && + typeof x === 'object' && + x !== null && + (x: any).$$typeof === REACT_POSTPONE_TYPE + ) { const postponeInstance: Postpone = (x: any); logPostpone(request, postponeInstance.message); emitPostponeChunk(request, newTask.id, postponeInstance); @@ -909,39 +914,40 @@ function resolveModelToJSON( // later, once we deprecate the old API in favor of `use`. getSuspendedThenable() : thrownValue; - // $FlowFixMe[method-unbinding] - if (typeof x === 'object' && x !== null && typeof x.then === 'function') { - // Something suspended, we'll need to create a new task and resolve it later. - request.pendingChunks++; - const newTask = createTask( - request, - value, - getActiveContext(), - request.abortableTasks, - ); - const ping = newTask.ping; - x.then(ping, ping); - newTask.thenableState = getThenableStateAfterSuspending(); - return serializeLazyID(newTask.id); - } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { - // Something postponed. We'll still send everything we have up until this point. - // We'll replace this element with a lazy reference that postpones on the client. - const postponeInstance: Postpone = (x: any); - request.pendingChunks++; - const postponeId = request.nextChunkId++; - logPostpone(request, postponeInstance.message); - emitPostponeChunk(request, postponeId, postponeInstance); - return serializeLazyID(postponeId); - } else { - // Something errored. We'll still send everything we have up until this point. - // We'll replace this element with a lazy reference that throws on the client - // once it gets rendered. - request.pendingChunks++; - const errorId = request.nextChunkId++; - const digest = logRecoverableError(request, x); - emitErrorChunk(request, errorId, digest, x); - return serializeLazyID(errorId); + if (typeof x === 'object' && x !== null) { + // $FlowFixMe[method-unbinding] + if (typeof x.then === 'function') { + // Something suspended, we'll need to create a new task and resolve it later. + request.pendingChunks++; + const newTask = createTask( + request, + value, + getActiveContext(), + request.abortableTasks, + ); + const ping = newTask.ping; + x.then(ping, ping); + newTask.thenableState = getThenableStateAfterSuspending(); + return serializeLazyID(newTask.id); + } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { + // Something postponed. We'll still send everything we have up until this point. + // We'll replace this element with a lazy reference that postpones on the client. + const postponeInstance: Postpone = (x: any); + request.pendingChunks++; + const postponeId = request.nextChunkId++; + logPostpone(request, postponeInstance.message); + emitPostponeChunk(request, postponeId, postponeInstance); + return serializeLazyID(postponeId); + } } + // Something errored. We'll still send everything we have up until this point. + // We'll replace this element with a lazy reference that throws on the client + // once it gets rendered. + request.pendingChunks++; + const errorId = request.nextChunkId++; + const digest = logRecoverableError(request, x); + emitErrorChunk(request, errorId, digest, x); + return serializeLazyID(errorId); } } @@ -1375,23 +1381,27 @@ function retryTask(request: Request, task: Task): void { // later, once we deprecate the old API in favor of `use`. getSuspendedThenable() : thrownValue; - // $FlowFixMe[method-unbinding] - if (typeof x === 'object' && x !== null && typeof x.then === 'function') { - // Something suspended again, let's pick it back up later. - const ping = task.ping; - x.then(ping, ping); - task.thenableState = getThenableStateAfterSuspending(); - return; - } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { - const postponeInstance: Postpone = (x: any); - logPostpone(request, postponeInstance.message); - emitPostponeChunk(request, task.id, postponeInstance); - } else { - request.abortableTasks.delete(task); - task.status = ERRORED; - const digest = logRecoverableError(request, x); - emitErrorChunk(request, task.id, digest, x); + if (typeof x === 'object' && x !== null) { + // $FlowFixMe[method-unbinding] + if (typeof x.then === 'function') { + // Something suspended again, let's pick it back up later. + const ping = task.ping; + x.then(ping, ping); + task.thenableState = getThenableStateAfterSuspending(); + return; + } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { + request.abortableTasks.delete(task); + task.status = ERRORED; + const postponeInstance: Postpone = (x: any); + logPostpone(request, postponeInstance.message); + emitPostponeChunk(request, task.id, postponeInstance); + return; + } } + request.abortableTasks.delete(task); + task.status = ERRORED; + const digest = logRecoverableError(request, x); + emitErrorChunk(request, task.id, digest, x); } } From bdd97039fb81c10db2aa87bb3d23831e5867a5c4 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 17 Aug 2023 13:04:19 -0400 Subject: [PATCH 10/10] Rename method names --- packages/react-client/src/ReactFlightClient.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index ea9b55cda18..549055acd69 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -878,7 +878,7 @@ function resolvePostponeProd(response: Response, id: number): void { // These errors should never make it into a build so we don't need to encode them in codes.json // eslint-disable-next-line react-internal/prod-error-codes throw new Error( - 'resolveErrorProd should never be called in development mode. Use resolveErrorDev instead. This is a bug in React.', + 'resolvePostponeProd should never be called in development mode. Use resolvePostponeDev instead. This is a bug in React.', ); } const error = new Error( @@ -907,7 +907,7 @@ function resolvePostponeDev( // These errors should never make it into a build so we don't need to encode them in codes.json // eslint-disable-next-line react-internal/prod-error-codes throw new Error( - 'resolveErrorDev should never be called in production mode. Use resolveErrorProd instead. This is a bug in React.', + 'resolvePostponeDev should never be called in production mode. Use resolvePostponeProd instead. This is a bug in React.', ); } // eslint-disable-next-line react-internal/prod-error-codes