From 8366f3389d3717d718b447ebff0e6a785c7e2d4d Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Mon, 10 Aug 2026 16:18:46 +0200 Subject: [PATCH 1/2] [Flight] Transfer key validation of lazy nodes when they are unwrapped (#37258) --- .../react-client/src/ReactFlightClient.js | 54 +++- .../__tests__/ReactFlightDOMBrowser-test.js | 288 ++++++++++++++++++ 2 files changed, 326 insertions(+), 16 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index b49f1f15023..12ef52669ba 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -1280,10 +1280,11 @@ function getTaskName(type: mixed): string { type !== null && type.$$typeof === REACT_LAZY_TYPE ) { - if (type._init === readChunk) { - // This is a lazy node created by Flight. It is probably a client reference. - // We use the "use client" string to indicate that this is the boundary into - // the client. There will only be one for any given owner chain. + if (type._payload instanceof ReactPromise) { + // This is a lazy node created by Flight, i.e. it wraps a chunk. It is + // probably a client reference. We use the "use client" string to indicate + // that this is the boundary into the client. There will only be one for + // any given owner chain. return '"use client"'; } // We don't want to eagerly initialize the initializer in DEV mode so we can't @@ -1374,16 +1375,6 @@ function initializeElement( } if (lazyNode !== null) { - // In case the JSX runtime has validated the lazy type as a static child, we - // need to transfer this information to the element. - if ( - lazyNode._store && - lazyNode._store.validated && - !element._store.validated - ) { - element._store.validated = lazyNode._store.validated; - } - // If the lazy node is initialized, we move its debug info to the inner // value. if (lazyNode._payload.status === INITIALIZED && lazyNode._debugInfo) { @@ -1535,6 +1526,29 @@ function createElement( return element; } +function transferValidation(store: {validated: 0 | 1 | 2}, value: mixed): void { + if (store.validated && typeof value === 'object' && value !== null) { + // Only elements and lazy nodes carry key validation. Any other value, e.g. + // an array of children, needs to have its own items validated instead. + const $$typeof = (value as any).$$typeof; + if ($$typeof === REACT_ELEMENT_TYPE || $$typeof === REACT_LAZY_TYPE) { + const valueStore = (value as any)._store; + if (valueStore && !valueStore.validated) { + valueStore.validated = store.validated; + } + } + } +} + +function readChunkAndTransferValidation( + store: {validated: 0 | 1 | 2}, + payload: SomeChunk, +): T { + const value: T = readChunk(payload); + transferValidation(store, value); + return value; +} + function createLazyChunkWrapper( chunk: SomeChunk, validated: 0 | 1 | 2, // DEV-only @@ -1547,8 +1561,16 @@ function createLazyChunkWrapper( if (__DEV__) { // Forward the live array lazyType._debugInfo = chunk._debugInfo; - // Initialize a store for key validation by the JSX runtime. - lazyType._store = {validated: validated}; + // Initialize a store for key validation by the JSX runtime. It can only + // validate the lazy node itself, because the value it refers to might not + // exist yet at that point, e.g. if it's an outlined row that hasn't been + // initialized. So the validation is transferred to the value when the lazy + // node is unwrapped. If the value is another lazy node, unwrapping that one + // forwards the validation further. + const store = {validated: validated}; + lazyType._store = store; + // $FlowFixMe[incompatible-type] `bind` loses the type argument. + lazyType._init = readChunkAndTransferValidation.bind(null, store); } return lazyType; } 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 d7ec51780a1..9a8b2736f2d 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -3137,4 +3137,292 @@ describe('ReactFlightDOMBrowser', () => { expect(container.innerHTML).toBe('
'); }); + + // Long enough to exceed MAX_ROW_SIZE in ReactFlightServer, which makes the + // element prop that follows it be outlined into its own row. + const longText = 'a'.repeat(4000); + + it('should not have missing key warnings when a static child is outlined', async () => { + const ClientComponent = clientExports(function ClientComponent({ + text, + element, + }) { + return ( +
+ {text.length} + {element} +
+ ); + }); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + Hello} />, + webpackMap, + ), + ); + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream(stream); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + + expect(container.innerHTML).toBe( + '
4000Hello
', + ); + }); + + it('should not have missing key warnings when an outlined static child is blocked on debug info', async () => { + const ClientComponent = clientExports(function ClientComponent({ + text, + element, + }) { + return ( +
+ {text.length} + {element} +
+ ); + }); + + let debugReadableStreamController; + + const debugReadableStream = new ReadableStream({ + start(controller) { + debugReadableStreamController = controller; + }, + }); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + Hello} />, + webpackMap, + { + debugChannel: { + writable: new WritableStream({ + write(chunk) { + debugReadableStreamController.enqueue(chunk); + }, + close() { + debugReadableStreamController.close(); + }, + }), + }, + }, + ), + ); + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream(stream, { + debugChannel: {readable: createDelayedStream(debugReadableStream)}, + }); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + + // Wait for the debug info to be processed. + await act(() => {}); + + expect(container.innerHTML).toBe( + '
4000Hello
', + ); + }); + + it('should have missing key warnings when an outlined element is used in an array', async () => { + const ClientComponent = clientExports(function ClientComponent({ + text, + element, + }) { + return ( +
+ {text.length} + {[element]} +
+ ); + }); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + Hello} />, + webpackMap, + ), + ); + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream(stream); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + + assertConsoleErrorDev([ + 'Each child in a list should have a unique "key" prop.\n\n' + + 'Check the render method of `div`. ' + + 'See https://react.dev/link/warning-keys for more information.\n' + + ' in span (at **)', + ]); + + expect(container.innerHTML).toBe( + '
4000Hello
', + ); + }); + + it('should have missing key warnings when an outlined element that is blocked on debug info is used in an array', async () => { + const ClientComponent = clientExports(function ClientComponent({ + text, + element, + }) { + return ( +
+ {text.length} + {[element]} +
+ ); + }); + + let debugReadableStreamController; + + const debugReadableStream = new ReadableStream({ + start(controller) { + debugReadableStreamController = controller; + }, + }); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + Hello} />, + webpackMap, + { + debugChannel: { + writable: new WritableStream({ + write(chunk) { + debugReadableStreamController.enqueue(chunk); + }, + close() { + debugReadableStreamController.close(); + }, + }), + }, + }, + ), + ); + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream(stream, { + debugChannel: {readable: createDelayedStream(debugReadableStream)}, + }); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + + // The element can only be rendered, and therefore validated, after it's + // unblocked by the debug info. + await act(() => {}); + + assertConsoleErrorDev([ + 'Each child in a list should have a unique "key" prop.\n\n' + + 'Check the render method of `div`. ' + + 'See https://react.dev/link/warning-keys for more information.\n' + + ' in span (at **)', + ]); + + expect(container.innerHTML).toBe( + '
4000Hello
', + ); + }); + + describe('with console.createTask', () => { + // Stands in for what a browser console does with fake tasks: whatever runs + // inside a task is shown under that task's name in the async stack. This is + // the same setup that `ReactServer-test` uses to assert on task names. + let currentTask; + + beforeEach(() => { + const {AsyncLocalStorage} = require('node:async_hooks'); + currentTask = new AsyncLocalStorage(); + (console: any).createTask = taskName => ({ + run: taskFn => { + const parentTask = currentTask.getStore() || ''; + return currentTask.run(parentTask + '\n' + taskName, taskFn); + }, + }); + + // `supportsCreateTask` is captured when ReactFlightClient is required, so + // the client modules need to be required again with this in place. + jest.resetModules(); + patchMessageChannel(); + ({act} = require('internal-test-utils')); + React = require('react'); + use = React.use; + ReactDOMClient = require('react-dom/client'); + ReactServerDOMClient = require('react-server-dom-webpack/client'); + }); + + afterEach(() => { + delete (console: any).createTask; + }); + + // @gate __DEV__ + it('renders a client component inside a "use client" task', async () => { + let taskWhileRendering; + + const ClientComponent = clientExports(function ClientComponent() { + taskWhileRendering = currentTask.getStore(); + return Hello; + }); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), + ); + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream(stream); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + + expect(container.innerHTML).toBe('Hello'); + // The element's type is a lazy node wrapping the client reference, so the + // task that the component renders in marks the boundary into the client. + expect(taskWhileRendering).toBe('\n"use client"'); + }); + }); }); From 807d21fdfdcf0da588e4ce3bdb9e8e539a34b5a1 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Mon, 10 Aug 2026 11:42:47 -0400 Subject: [PATCH 2/2] Add lazy reasons to browser() (#37241) Changes `ReactDOM.browser()` to return a cheap branded recoverable token instead of eagerly constructing an `Error`. It accepts an optional reason string or initializer that runs only when a server renderer consumes the token and may return any value; the client renderer ignores the reason without invoking the initializer, so browser-only rendering does not pay for an unused stack. When Fizz consumes the token through `use()` or `abort()`, it creates a consistent browser-bailout error at the consumption point so its stack identifies the relevant operation. The initialized reason is preserved unchanged as the optional `cause`, allowing strings, errors, and structured framework metadata without runtime validation. If an initializer throws, Fizz substitutes a stable diagnostic fallback so reason generation cannot change rendering control flow. Successful recoveries report the error through `onBrowserBailout`. When no Suspense boundary can recover the render, Fizz clones the branded recoverable error into an unbranded fatal diagnostic while preserving its cause and consumption frames. During an abort, the request retains the original branded error so every remaining task observes the same reason; fatal clones are created only when reporting a fatal root or closing the stream. Centralized recoverable logging uses the brand to route successful bailouts through `onBrowserBailout` and fatal clones through `onError`. The empty recoverable digest and client hydration suppression behavior remain unchanged. Tests cover omitted and direct reasons, lazy string, error, structured, and primitive reasons, repeated use sites, throwing initializers, lazy client behavior, consumption stacks, flattened fatal errors, recoverable and fatal use and abort paths, nested aborts, direct throws, debug tools, and development and production rendering. --- .../react-debug-tools/src/ReactDebugHooks.js | 8 +- .../src/__tests__/ReactDOMBrowser-test.js | 6 +- .../src/__tests__/ReactDOMFizzServer-test.js | 455 +++++++++++++++++- .../ReactDOMServerSuspense-test.internal.js | 47 ++ .../react-dom/src/shared/ReactDOMBrowser.js | 27 +- packages/react-server/src/ReactFizzHooks.js | 103 ++-- packages/react-server/src/ReactFizzServer.js | 125 +++-- packages/shared/ReactTypes.js | 7 +- 8 files changed, 633 insertions(+), 145 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index ee964c815fc..f63b9b22818 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -111,10 +111,10 @@ function getPrimitiveStackCache(): Map> { $$typeof: REACT_CONTEXT_TYPE, _currentValue: null, } as any); - const recoverable = new Error(); - Object.defineProperty(recoverable as any, '$$typeof', { - value: REACT_RECOVERABLE_TYPE, - }); + const recoverable = { + $$typeof: REACT_RECOVERABLE_TYPE, + _reason: undefined, + }; Dispatcher.use(recoverable as any); Dispatcher.use({ then() {}, diff --git a/packages/react-dom/src/__tests__/ReactDOMBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMBrowser-test.js index 0bfef8c753f..bd1806dc630 100644 --- a/packages/react-dom/src/__tests__/ReactDOMBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMBrowser-test.js @@ -18,7 +18,10 @@ describe('ReactDOM.browser', () => { it('can create browser-only content before the browser renderer is initialized', async () => { const React = require('react'); const ReactDOM = require('react-dom'); - const browserOnly = ReactDOM.browser(); + const initializeReason = jest.fn( + () => new Error('Only render this content in a browser'), + ); + const browserOnly = ReactDOM.browser(initializeReason); const ReactDOMClient = require('react-dom/client'); const {act} = require('internal-test-utils'); @@ -37,5 +40,6 @@ describe('ReactDOM.browser', () => { ); }); expect(container.innerHTML).toBe('Browser'); + expect(initializeReason).not.toHaveBeenCalled(); }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index b0e47939a2c..f3959b544d6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -412,7 +412,14 @@ describe('ReactDOMFizzServer', () => { const browserText = new Promise(resolve => { resolveBrowserText = resolve; }); - const browserOnly = ReactDOM.browser(); + let browserReason; + const initializeReason = jest.fn(() => { + browserReason = Object.freeze( + new Error('Only render this content in a browser'), + ); + return browserReason; + }); + const browserOnly = ReactDOM.browser(initializeReason); function BrowserOnly() { use(browserOnly); @@ -446,8 +453,14 @@ describe('ReactDOMFizzServer', () => { }); expect(serverErrors).toEqual([]); + expect(initializeReason).toHaveBeenCalledTimes(1); expect(browserBailouts).toHaveLength(1); - expect(browserBailouts[0].error).toBe(browserOnly); + expect(browserBailouts[0].error).toBeInstanceOf(Error); + expect(browserBailouts[0].error.message).toBe( + 'Browser-only rendering was requested by `browser()`.', + ); + expect(browserBailouts[0].error.stack).toContain('BrowserOnly'); + expect(browserBailouts[0].error.cause).toBe(browserReason); expect( normalizeCodeLocInfo(browserBailouts[0].errorInfo.componentStack), ).toBe(componentStack(['BrowserOnly', 'Suspense', 'div', 'App'])); @@ -476,6 +489,7 @@ describe('ReactDOMFizzServer', () => { assertLog(['Browser']); expect(recoverableErrors).toEqual([]); + expect(initializeReason).toHaveBeenCalledTimes(1); expect(getVisibleChildren(container)).toEqual(
Browser @@ -489,10 +503,13 @@ describe('ReactDOMFizzServer', () => { const serverReady = new Promise(resolve => { resolveServerReady = resolve; }); + const initializeReason = jest.fn( + () => 'Only render this content in a browser', + ); function BrowserOnly() { use(serverReady); - use(ReactDOM.browser()); + use(ReactDOM.browser(initializeReason)); return Browser; } @@ -507,11 +524,15 @@ describe('ReactDOMFizzServer', () => { } const serverErrors = []; + const browserBailouts = []; await act(() => { const {pipe} = renderToPipeableStream(, { onError(error) { serverErrors.push(error); }, + onBrowserBailout(error) { + browserBailouts.push(error); + }, }); pipe(writable); }); @@ -527,6 +548,15 @@ describe('ReactDOMFizzServer', () => { }); expect(serverErrors).toEqual([]); + expect(initializeReason).toHaveBeenCalledTimes(1); + expect(browserBailouts).toHaveLength(1); + expect(browserBailouts[0].message).toBe( + 'Browser-only rendering was requested by `browser()`.', + ); + expect(browserBailouts[0].stack).toContain('BrowserOnly'); + expect(browserBailouts[0].cause).toBe( + 'Only render this content in a browser', + ); const recoverableErrors = []; ReactDOMClient.hydrateRoot(container, , { @@ -537,6 +567,7 @@ describe('ReactDOMFizzServer', () => { await waitForAll([]); expect(recoverableErrors).toEqual([]); + expect(initializeReason).toHaveBeenCalledTimes(1); expect(getVisibleChildren(container)).toEqual(
Browser @@ -545,11 +576,207 @@ describe('ReactDOMFizzServer', () => { }); // @gate enableBrowserAPI - it('errors if browser-only content is rendered outside Suspense', async () => { - function createBrowserValue() { - return ReactDOM.browser(); + it('supports omitted and direct string browser reasons', async () => { + const directReason = 'Only render this content in a browser'; + const withoutReason = ReactDOM.browser(); + const withDirectReason = ReactDOM.browser(directReason); + + function WithoutReason() { + use(withoutReason); + return Browser; } - const browserValue = createBrowserValue(); + + function WithDirectReason() { + use(withDirectReason); + return Browser; + } + + const serverErrors = []; + const browserBailouts = []; + await act(() => { + const {pipe} = renderToPipeableStream( + <> + Fallback A}> + + + Fallback B}> + + + , + { + onError(error) { + serverErrors.push(error); + }, + onBrowserBailout(error) { + browserBailouts.push(error); + }, + }, + ); + pipe(writable); + }); + + expect(serverErrors).toEqual([]); + expect(browserBailouts).toHaveLength(2); + expect(browserBailouts[0].message).toBe( + 'Browser-only rendering was requested by `browser()`.', + ); + expect(browserBailouts[0].stack).toContain('WithoutReason'); + expect( + Object.prototype.hasOwnProperty.call(browserBailouts[0], 'cause'), + ).toBe(false); + expect(browserBailouts[1].message).toBe( + 'Browser-only rendering was requested by `browser()`.', + ); + expect(browserBailouts[1].stack).toContain('WithDirectReason'); + expect(browserBailouts[1].cause).toBe(directReason); + }); + + // @gate enableBrowserAPI + it('supports any value returned by a browser reason initializer', async () => { + const reasonValues = [undefined, null, 42, Symbol('browser reason')]; + const initializeReasons = reasonValues.map(reason => jest.fn(() => reason)); + const browserValues = initializeReasons.map(initializeReason => + ReactDOM.browser(initializeReason), + ); + + function BrowserOnly({browserValue}) { + use(browserValue); + return Browser; + } + + const serverErrors = []; + const browserBailouts = []; + await act(() => { + const {pipe} = renderToPipeableStream( + <> + {browserValues.map((browserValue, index) => ( + Fallback}> + + + ))} + , + { + onError(error) { + serverErrors.push(error); + }, + onBrowserBailout(error) { + browserBailouts.push(error); + }, + }, + ); + pipe(writable); + }); + + expect(serverErrors).toEqual([]); + expect(browserBailouts).toHaveLength(reasonValues.length); + initializeReasons.forEach(initializeReason => { + expect(initializeReason).toHaveBeenCalledTimes(1); + }); + browserBailouts.forEach((error, index) => { + expect(error).toBeInstanceOf(Error); + expect(error.message).toBe( + 'Browser-only rendering was requested by `browser()`.', + ); + expect(Object.prototype.hasOwnProperty.call(error, 'cause')).toBe(true); + expect(error.cause).toBe(reasonValues[index]); + }); + }); + + // @gate enableBrowserAPI + it('initializes a shared browser reason at each use site', async () => { + const browserReasons = []; + const initializeReason = jest.fn(() => { + const browserReason = {index: browserReasons.length}; + browserReasons.push(browserReason); + return browserReason; + }); + const browserValue = ReactDOM.browser(initializeReason); + + function BrowserOnlyA() { + use(browserValue); + return Browser A; + } + + function BrowserOnlyB() { + use(browserValue); + return Browser B; + } + + const browserBailouts = []; + await act(() => { + const {pipe} = renderToPipeableStream( + <> + Fallback A}> + + + Fallback B}> + + + , + { + onBrowserBailout(error) { + browserBailouts.push(error); + }, + }, + ); + pipe(writable); + }); + + expect(initializeReason).toHaveBeenCalledTimes(2); + expect(browserBailouts).toHaveLength(2); + expect(browserBailouts[0]).not.toBe(browserBailouts[1]); + expect(browserBailouts[0].cause).toBe(browserReasons[0]); + expect(browserBailouts[0].stack).toContain('BrowserOnlyA'); + expect(browserBailouts[1].cause).toBe(browserReasons[1]); + expect(browserBailouts[1].stack).toContain('BrowserOnlyB'); + }); + + // @gate enableBrowserAPI + it('uses a fallback if a browser reason initializer throws', async () => { + const reasonError = new Error('Failed to initialize browser reason'); + const initializeReason = jest.fn(() => { + throw reasonError; + }); + const browserValue = ReactDOM.browser(initializeReason); + + function BrowserOnly() { + use(browserValue); + return Browser; + } + + const serverErrors = []; + const browserBailouts = []; + await act(() => { + const {pipe} = renderToPipeableStream( + Fallback}> + + , + { + onError(error) { + serverErrors.push(error); + }, + onBrowserBailout(error) { + browserBailouts.push(error); + }, + }, + ); + pipe(writable); + }); + + expect(initializeReason).toHaveBeenCalledTimes(1); + expect(serverErrors).toEqual([]); + expect(browserBailouts).toHaveLength(1); + expect(browserBailouts[0].cause).toBe( + 'The reason for browser-only rendering could not be determined because ' + + 'its initializer threw.', + ); + expect(getVisibleChildren(container)).toEqual(Fallback); + }); + + // @gate enableBrowserAPI + it('errors if browser-only content is rendered outside Suspense', async () => { + const browserReason = 'Only render this content in a browser'; + const browserValue = ReactDOM.browser(browserReason); function BrowserOnly() { use(browserValue); @@ -583,9 +810,11 @@ describe('ReactDOMFizzServer', () => { "requested outside a Suspense boundary. See this error's cause for " + 'additional details.', ); + expect(shellError.cause).toBe(browserReason); expect(shellError.stack).toContain('BrowserOnly'); - expect(shellError.cause).toBe(browserValue); - expect(shellError.cause.stack).toContain('createBrowserValue'); + expect(shellError.stack.split('\n')[0]).toBe( + 'Error: ' + shellError.message, + ); expect(shellReady).toBe(false); expect(reportedErrors).toEqual([shellError]); expect(browserBailouts).toEqual([]); @@ -619,7 +848,9 @@ describe('ReactDOMFizzServer', () => { const serverErrors = []; const browserBailouts = []; - const browserValue = ReactDOM.browser(); + const browserReason = {code: 'render-pending-content-in-browser'}; + const initializeReason = jest.fn(() => browserReason); + const browserValue = ReactDOM.browser(initializeReason); let abort; await act(() => { const controls = renderToPipeableStream(, { @@ -643,11 +874,22 @@ describe('ReactDOMFizzServer', () => { ); await act(() => { - abort(browserValue); + function abortToBrowser() { + abort(browserValue); + } + abortToBrowser(); }); expect(serverErrors).toEqual([]); - expect(browserBailouts).toEqual([browserValue, browserValue]); + expect(initializeReason).toHaveBeenCalledTimes(1); + expect(browserBailouts).toHaveLength(2); + expect(browserBailouts[0]).toBeInstanceOf(Error); + expect(browserBailouts[0].message).toBe( + 'Browser-only rendering was requested by `browser()`.', + ); + expect(browserBailouts[0].stack).toContain('abortToBrowser'); + expect(browserBailouts[0].cause).toBe(browserReason); + expect(browserBailouts[1]).toBe(browserBailouts[0]); isClient = true; const recoverableErrors = []; @@ -671,7 +913,12 @@ describe('ReactDOMFizzServer', () => { // @gate enableBrowserAPI it('errors if aborted with browser() before the shell completes', async () => { const never = new Promise(() => {}); - const browserValue = ReactDOM.browser(); + let browserReason; + const initializeReason = jest.fn(() => { + browserReason = new Error('Only abort this render on the server'); + return browserReason; + }); + const browserValue = ReactDOM.browser(initializeReason); function PendingRoot() { use(never); @@ -702,24 +949,145 @@ describe('ReactDOMFizzServer', () => { }); await act(() => { - abort(browserValue); + function abortToBrowser() { + abort(browserValue); + } + abortToBrowser(); }); expect(shellError).toBeInstanceOf(Error); + expect(initializeReason).toHaveBeenCalledTimes(1); expect(shellError.message).toBe( 'The server render could not complete because client rendering was ' + "requested outside a Suspense boundary. See this error's cause for " + 'additional details.', ); - expect(shellError.cause).toBe(browserValue); + expect(shellError.cause).toBe(browserReason); + expect(shellError.stack).toContain('abortToBrowser'); expect(shellReady).toBe(false); expect(reportedErrors).toEqual([shellError]); expect(browserBailouts).toEqual([]); }); + // @gate enableBrowserAPI + it('reports nested browser bailouts if aborting fatals the shell', async () => { + const never = new Promise(() => {}); + const browserReason = 'Abort pending work into browser rendering'; + const browserValue = ReactDOM.browser(browserReason); + + function Pending() { + use(never); + return Pending; + } + + const reportedErrors = []; + const browserBailouts = []; + let shellError; + let abort; + await act(() => { + const controls = renderToPipeableStream( + <> + Fallback}> + + + + Fallback}> + + + + , + { + onError(error) { + reportedErrors.push(error); + }, + onBrowserBailout(error) { + browserBailouts.push(error); + }, + onShellError(error) { + shellError = error; + }, + }, + ); + abort = controls.abort; + }); + + await act(() => { + abort(browserValue); + }); + + expect(shellError).toBeInstanceOf(Error); + expect(shellError.message).toBe( + 'The server render could not complete because client rendering was ' + + "requested outside a Suspense boundary. See this error's cause for " + + 'additional details.', + ); + expect(shellError.cause).toBe(browserReason); + expect(reportedErrors).toHaveLength(2); + expect(reportedErrors[0]).toBe(shellError); + expect(reportedErrors[1].message).toBe(shellError.message); + expect(reportedErrors[1].cause).toBe(browserReason); + expect(browserBailouts).toHaveLength(2); + expect(browserBailouts[0]).toBe(browserBailouts[1]); + expect(browserBailouts[0]).not.toBe(shellError); + expect(browserBailouts[0].message).toBe( + 'Browser-only rendering was requested by `browser()`.', + ); + expect(browserBailouts[0].cause).toBe(browserReason); + }); + + // @gate enableBrowserAPI + it('uses a fallback if a browser reason initializer throws during abort', async () => { + const never = new Promise(() => {}); + const reasonError = new Error('Failed to initialize browser reason'); + const initializeReason = jest.fn(() => { + throw reasonError; + }); + const browserValue = ReactDOM.browser(initializeReason); + + function PendingRoot() { + use(never); + return Root; + } + + const reportedErrors = []; + const browserBailouts = []; + let shellError; + let abort; + await act(() => { + const controls = renderToPipeableStream(, { + onError(error) { + reportedErrors.push(error); + }, + onBrowserBailout(error) { + browserBailouts.push(error); + }, + onShellError(error) { + shellError = error; + }, + }); + abort = controls.abort; + }); + + await act(() => { + abort(browserValue); + }); + + expect(initializeReason).toHaveBeenCalledTimes(1); + expect(shellError).toBeInstanceOf(Error); + expect(shellError.cause).toBe( + 'The reason for browser-only rendering could not be determined because ' + + 'its initializer threw.', + ); + expect(reportedErrors).toEqual([shellError]); + expect(browserBailouts).toEqual([]); + }); + // @gate enableBrowserAPI it('reports the browser value if it is thrown instead of passed to use', async () => { - const browserValue = ReactDOM.browser(); + const initializeReason = jest.fn( + () => new Error('Only render this content in a browser'), + ); + const browserValue = ReactDOM.browser(initializeReason); function BrowserOnly() { throw browserValue; @@ -746,6 +1114,7 @@ describe('ReactDOMFizzServer', () => { expect(reportedErrors).toEqual([browserValue]); expect(browserBailouts).toEqual([]); + expect(initializeReason).not.toHaveBeenCalled(); expect(getVisibleChildren(container)).toEqual(Fallback); }); @@ -7605,6 +7974,60 @@ describe('ReactDOMFizzServer', () => { expect(errors).toEqual(['abort reason', 'abort reason']); }); + // @gate enableBrowserAPI + it('reports an in-flight browser bailout after another root task fatals while aborting', async () => { + const promise = new Promise(() => {}); + function SuspendedRoot() { + use(promise); + return null; + } + + function Child() { + return 'child'; + } + + const browserValue = ReactDOM.browser('abort reason'); + const abortRef = {current: null}; + function ComponentThatAborts() { + abortRef.current(browserValue); + return ; + } + + const errors = []; + const browserBailouts = []; + let shellError; + await act(() => { + const {abort} = renderToPipeableStream( + <> + + + + + , + { + onError(error) { + errors.push(error); + }, + onBrowserBailout(error) { + browserBailouts.push(error); + }, + onShellError(error) { + shellError = error; + }, + }, + ); + abortRef.current = abort; + }); + + expect(errors).toEqual([shellError]); + expect(browserBailouts).toHaveLength(1); + expect(browserBailouts[0]).not.toBe(shellError); + expect(browserBailouts[0].message).toBe( + 'Browser-only rendering was requested by `browser()`.', + ); + expect(browserBailouts[0].cause).toBe('abort reason'); + }); + it('reports a root task before rendering a suspended child returned after aborting', async () => { const promise = new Promise(() => {}); function SuspendedRoot() { diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js index 6e6f9bb0926..ca3cda88fb7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js @@ -10,6 +10,7 @@ 'use strict'; let React; +let ReactDOM; let ReactDOMClient; let ReactDOMServer; let act; @@ -21,6 +22,7 @@ describe('ReactDOMServerSuspense', () => { jest.resetModules(); React = require('react'); + ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); act = require('internal-test-utils').act; @@ -98,6 +100,51 @@ describe('ReactDOMServerSuspense', () => { expect(getVisibleChildren(container)).toEqual(
Fallback
); }); + // @gate enableBrowserAPI + it('hydrates browser-only content rendered with renderToString', async () => { + function BrowserOnly() { + React.use(ReactDOM.browser('Only render this content in the browser')); + return ; + } + + const app = ( + }> + + + ); + const container = document.createElement('div'); + container.innerHTML = ReactDOMServer.renderToString(app); + expect(getVisibleChildren(container)).toEqual(
Fallback
); + + const recoverableErrors = []; + await act(() => { + ReactDOMClient.hydrateRoot(container, app, { + onRecoverableError(error) { + recoverableErrors.push(error); + }, + }); + }); + + expect(recoverableErrors).toEqual([]); + expect(getVisibleChildren(container)).toEqual(
Children
); + }); + + // @gate enableBrowserAPI + it('renders only the browser-only fallback with renderToStaticMarkup', () => { + function BrowserOnly() { + React.use(ReactDOM.browser('Only render this content in the browser')); + return ; + } + + const html = ReactDOMServer.renderToStaticMarkup( + }> + + , + ); + + expect(html).toBe('
Fallback
'); + }); + it('should work with nested suspense components', async () => { const container = document.createElement('div'); const html = ReactDOMServer.renderToString( diff --git a/packages/react-dom/src/shared/ReactDOMBrowser.js b/packages/react-dom/src/shared/ReactDOMBrowser.js index eaf1c05d83d..2aad4bac85c 100644 --- a/packages/react-dom/src/shared/ReactDOMBrowser.js +++ b/packages/react-dom/src/shared/ReactDOMBrowser.js @@ -7,23 +7,22 @@ * @flow */ -import type {ReactRecoverable} from 'shared/ReactTypes'; +import type {ReactRecoverable, ReactRecoverableReason} from 'shared/ReactTypes'; import {enableBrowserAPI} from 'shared/ReactFeatureFlags'; import {REACT_RECOVERABLE_TYPE} from 'shared/ReactSymbols'; -const browserImpl = function browser(): ReactRecoverable { - // Recoverables are Errors so that a renderer can preserve the browser() call - // site as the cause if no downstream renderer can recover the subtree. - const recoverable = new Error( - 'Browser-only rendering was requested by `browser()`.', - ); - Object.defineProperty(recoverable as any, '$$typeof', { - value: REACT_RECOVERABLE_TYPE, - }); - return recoverable as any; +const browserImpl = function browser( + reason?: ReactRecoverableReason, +): ReactRecoverable { + // This also runs in the browser, where the reason is never observed. Keep the + // value cheap and let an SSR renderer initialize the error if it defers work. + return { + $$typeof: REACT_RECOVERABLE_TYPE, + _reason: reason, + }; }; -export const browser: (() => ReactRecoverable) | void = enableBrowserAPI - ? browserImpl - : undefined; +export const browser: + | ((reason?: ReactRecoverableReason) => ReactRecoverable) + | void = enableBrowserAPI ? browserImpl : undefined; diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index e42ebcbee61..4a867bb46df 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -40,6 +40,7 @@ import { import {createFastHash} from './ReactServerStreamConfig'; import is from 'shared/objectIs'; +import hasOwnProperty from 'shared/hasOwnProperty'; import { REACT_CONTEXT_TYPE, REACT_RECOVERABLE_TYPE, @@ -91,31 +92,68 @@ let actionStateMatchingIndex: number = -1; // Counts the number of use(thenable) calls in this component let thenableIndexCounter: number = 0; let thenableState: ThenableState | null = null; -// An opaque exception that lets the Fizz work loop distinguish a recoverable -// from an Error thrown by application code. The actual errors are stored -// separately so this implementation detail cannot be mistaken for either -// diagnostic if it is caught by userspace. -export const RecoverableException: mixed = new Error( - "Recoverable Exception: This is not a real error! It's an implementation " + - 'detail of `use` to interrupt the current render so a downstream ' + - 'renderer can recover it. You must either rethrow it immediately, or move ' + - 'the `use` call outside of the `try/catch` block. Capturing without ' + - 'rethrowing will lead to unexpected behavior.', -); -let suspendedRecoverableError: Error | null = null; - -export function createFatalRecoverableError( - recoverable: ReactRecoverable, -): Error { - // This is created eagerly when use() encounters the recoverable so its stack - // points to the component call site. It only becomes fatal if no Suspense - // boundary can recover the render. - return new Error( + +const browserReasonInitializationFallback = + 'The reason for browser-only rendering could not be determined because its ' + + 'initializer threw.'; + +export function createRecoverableError(recoverable: ReactRecoverable): Error { + const reason = recoverable._reason; + let initializedReason; + if (typeof reason === 'function') { + try { + initializedReason = reason(); + } catch { + // A reason is only diagnostic metadata. Its initializer must not affect + // whether the renderer can defer this subtree to the browser. + initializedReason = browserReasonInitializationFallback; + } + } else { + initializedReason = reason; + } + // Always create the recoverable at the consumption point so its stack + // identifies the relevant use() or abort() call. A lazy reason is diagnostic + // metadata and can be any value supported by Error.cause. + const error = new Error( + 'Browser-only rendering was requested by `browser()`.', + reason === undefined ? undefined : {cause: initializedReason}, + ); + Object.defineProperty(error, REACT_RECOVERABLE_TYPE, {value: true}); + return error; +} + +export function isRecoverableError(error: mixed): boolean { + if (typeof error !== 'object' || error === null) { + return false; + } + return (error as any)[REACT_RECOVERABLE_TYPE] === true; +} + +export function cloneRecoverableErrorAsFatal(recoverableError: Error): Error { + // Create a separate diagnostic for fatal reporting without changing the + // branded recoverable error that other tasks may still need to observe. + const fatalRecoverableError = new Error( 'The server render could not complete because client rendering was ' + "requested outside a Suspense boundary. See this error's cause for " + 'additional details.', - {cause: recoverable}, + hasOwnProperty.call(recoverableError, 'cause') + ? {cause: (recoverableError as any).cause} + : undefined, ); + // Keep the frames captured where the recoverable was consumed, but replace + // the first line with the fatal error's message. + const stack = recoverableError.stack; + if (stack !== undefined) { + const frameStart = stack.indexOf('\n'); + fatalRecoverableError.stack = + fatalRecoverableError.name + + ': ' + + fatalRecoverableError.message + + (frameStart === -1 ? '' : stack.slice(frameStart)); + } else { + (fatalRecoverableError as any).stack = undefined; + } + return fatalRecoverableError; } // Lazily created map of render-phase updates @@ -305,18 +343,6 @@ export function getThenableStateAfterSuspending(): null | ThenableState { return state; } -export function getSuspendedRecoverableError(): Error { - if (suspendedRecoverableError === null) { - throw new Error( - 'Expected a suspended recoverable. This is a bug in React. Please file ' + - 'an issue.', - ); - } - const error = suspendedRecoverableError; - suspendedRecoverableError = null; - return error; -} - export function checkDidRenderIdHook(): boolean { // This should be called immediately after every finishHooks call. // Conceptually, it's part of the return value of finishHooks; it's only a @@ -798,14 +824,11 @@ function use(usable: Usable): T { const thenable: Thenable = usable as any; return unwrapThenable(thenable); } else if (usable.$$typeof === REACT_RECOVERABLE_TYPE) { - // Fizz can defer this subtree to a downstream renderer. Like a suspended - // thenable, keep the actual value out of userspace and throw an opaque - // sentinel to unwind the stack. Capture the use() call site eagerly so - // that if there is no Suspense boundary, the fatal error points here and - // its cause points to where the recoverable was created. + // Create the recoverable error here so its stack captures the component + // that passed this value to use(). The internal brand lets the renderer + // distinguish it from an Error thrown by application code. const recoverable: ReactRecoverable = usable as any; - suspendedRecoverableError = createFatalRecoverableError(recoverable); - throw RecoverableException; + throw createRecoverableError(recoverable); } else if (usable.$$typeof === REACT_CONTEXT_TYPE) { const context: ReactContext = usable as any; return readContext(context); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 6641ced6898..8ea7013d17f 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -28,7 +28,6 @@ import type { SuspenseListProps, SuspenseListRevealOrder, ReactKey, - ReactRecoverable, } from 'shared/ReactTypes'; import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; import type { @@ -135,9 +134,9 @@ import { readPreviousThenableFromState, getActionStateCount, getActionStateMatchingIndex, - RecoverableException, - createFatalRecoverableError, - getSuspendedRecoverableError, + createRecoverableError, + isRecoverableError, + cloneRecoverableErrorAsFatal, } from './ReactFizzHooks'; import {DefaultAsyncDispatcher} from './ReactFizzAsyncDispatcher'; import { @@ -1337,7 +1336,7 @@ function encodeErrorForBoundary( ) { boundary.errorDigest = digest; if (__DEV__) { - if (error === RecoverableException) { + if (isRecoverableError(error)) { boundary.errorMessage = wasAborted ? 'Switched to client rendering because the server render was aborted ' + 'with a request to render on the client.' @@ -1375,17 +1374,8 @@ function logRecoverableError( errorInfo: ThrownInfo, debugTask: null | ConsoleTask, ): ?string { - if (error === RecoverableException) { - // The fatal wrapper was created eagerly to capture the use() call site, but - // this path recovered at a Suspense boundary. Report its original cause and - // discard the wrapper. - const fatalRecoverableError = getSuspendedRecoverableError(); - logBrowserBailout( - request, - fatalRecoverableError.cause, - errorInfo, - debugTask, - ); + if (isRecoverableError(error)) { + logBrowserBailout(request, error, errorInfo, debugTask); return REACT_RECOVERABLE_DIGEST; } @@ -1460,7 +1450,11 @@ function fatalError( closeWithError(request.destination, error); } else { request.status = CLOSING; - request.fatalError = error; + // abort() already stored the reason that every remaining task must + // observe. This error may only be a fatal diagnostic derived from it. + if (!request.aborted) { + request.fatalError = error; + } } } @@ -4593,10 +4587,12 @@ function erroredTask( // shell and defer its content to a downstream renderer. At the root there // is no shell to stream, so this is a fatal error and must be reported like // any other root error. - if (error === RecoverableException) { - const useError = getSuspendedRecoverableError(); - logRecoverableError(request, useError, errorInfo, debugTask); - fatalError(request, useError, errorInfo, debugTask); + if (isRecoverableError(error)) { + // This recoverable reached the root without a Suspense boundary, so + // report it using the fatal diagnostic while leaving the original intact. + const fatalRecoverableError = cloneRecoverableErrorAsFatal(error as any); + logRecoverableError(request, fatalRecoverableError, errorInfo, debugTask); + fatalError(request, fatalRecoverableError, errorInfo, debugTask); } else { logRecoverableError(request, error, errorInfo, debugTask); fatalError(request, error, errorInfo, debugTask); @@ -4845,14 +4841,9 @@ function finishAbortedTask(task: Task, request: Request, error: mixed): void { } const errorInfo = getThrownInfo(task.componentStack); - // Only abort reasons get this interpretation. Throwing a recoverable - // directly is still an application error; it must be passed to use() or - // abort() for a renderer to recover it. - const isRecoverableAbort = - typeof error === 'object' && - error !== null && - // $FlowFixMe[prop-missing] - error.$$typeof === REACT_RECOVERABLE_TYPE; + // Only errors materialized by use() or abort() carry this internal brand. + // Throwing the browser() token directly is still an application error. + const isRecoverableReason = isRecoverableError(error); if (boundary === null) { const replay: null | ReplaySet = task.replay; @@ -4860,7 +4851,7 @@ function finishAbortedTask(task: Task, request: Request, error: mixed): void { // We didn't complete the root so we have nothing to show. We can close // the request; if ( - !isRecoverableAbort && + !isRecoverableReason && request.trackedPostpones !== null && segment !== null ) { @@ -4870,9 +4861,12 @@ function finishAbortedTask(task: Task, request: Request, error: mixed): void { logRecoverableError(request, error, errorInfo, task.debugTask); trackPostpone(request, trackedPostpones, task, segment); finishedTask(request, null, task.row, segment); - } else if (isRecoverableAbort) { - const recoverable: ReactRecoverable = error as any; - const fatalRecoverableError = createFatalRecoverableError(recoverable); + } else if (isRecoverableReason) { + // This root task cannot recover from the abort. Report a fatal clone, + // but keep the original branded reason on the request for other tasks. + const fatalRecoverableError = cloneRecoverableErrorAsFatal( + error as any, + ); logRecoverableError( request, fatalRecoverableError, @@ -4896,22 +4890,18 @@ function finishAbortedTask(task: Task, request: Request, error: mixed): void { // the ReplaySet. replay.pendingTasks--; if (replay.pendingTasks === 0 && replay.nodes.length > 0) { - let errorDigest; - let errorForBoundary; - if (isRecoverableAbort) { - logBrowserBailout(request, error, errorInfo, null); - errorDigest = REACT_RECOVERABLE_DIGEST; - errorForBoundary = RecoverableException; - } else { - errorDigest = logRecoverableError(request, error, errorInfo, null); - errorForBoundary = error; - } + const errorDigest = logRecoverableError( + request, + error, + errorInfo, + null, + ); abortRemainingReplayNodes( request, null, replay.nodes, replay.slots, - errorForBoundary, + error, errorDigest, errorInfo, true, @@ -4928,7 +4918,7 @@ function finishAbortedTask(task: Task, request: Request, error: mixed): void { const trackedPostpones = request.trackedPostpones; if (boundary.status !== CLIENT_RENDERED) { if ( - !isRecoverableAbort && + !isRecoverableReason && trackedPostpones !== null && segment !== null ) { @@ -4947,28 +4937,13 @@ function finishAbortedTask(task: Task, request: Request, error: mixed): void { boundary.status = CLIENT_RENDERED; // We are aborting a render or resume which should put boundaries // into an explicitly client rendered state - let errorDigest; - let errorForBoundary; - if (isRecoverableAbort) { - logBrowserBailout(request, error, errorInfo, task.debugTask); - errorDigest = REACT_RECOVERABLE_DIGEST; - errorForBoundary = RecoverableException; - } else { - errorDigest = logRecoverableError( - request, - error, - errorInfo, - task.debugTask, - ); - errorForBoundary = error; - } - encodeErrorForBoundary( - boundary, - errorDigest, - errorForBoundary, + const errorDigest = logRecoverableError( + request, + error, errorInfo, - true, + task.debugTask, ); + encodeErrorForBoundary(boundary, errorDigest, error, errorInfo, true); untrackBoundary(request, boundary); @@ -6475,7 +6450,13 @@ export function prepareForStartFlowingIfBeforeAllReady(request: Request) { export function startFlowing(request: Request, destination: Destination): void { if (request.status === CLOSING) { request.status = CLOSED; - closeWithError(destination, request.fatalError); + let error = request.fatalError; + if (isRecoverableError(error)) { + // An aborted request keeps its original branded reason while tasks + // unwind. Convert it only now that the stream must receive a fatal. + error = cloneRecoverableErrorAsFatal(error as any); + } + closeWithError(destination, error); return; } if (request.status === CLOSED) { @@ -6533,9 +6514,17 @@ export function abort(request: Request, reason: mixed): void { // can be aborted. in practice this makes abort callable at most once per render. return; } + const isRecoverableReason = + typeof reason === 'object' && + reason !== null && + // $FlowFixMe[prop-missing] + reason.$$typeof === REACT_RECOVERABLE_TYPE; + // Mark the request before initializing a recoverable reason so an initializer + // cannot reenter abort(). request.aborted = true; - const error = - reason === undefined + const error = isRecoverableReason + ? createRecoverableError(reason as any) + : reason === undefined ? new Error('The render was aborted by the server without a reason.') : typeof reason === 'object' && reason !== null && diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 0de151ce970..43fd2b81c5b 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -150,12 +150,15 @@ export type Thenable = | FulfilledThenable | RejectedThenable; +export type ReactRecoverableReason = string | (() => mixed); + // A recoverable lets an intermediate renderer defer a subtree to a downstream // renderer. It does not produce a value: a renderer either continues through // it or interrupts the current render so that a later renderer can recover the -// subtree. -export type ReactRecoverable = Error & { +// subtree. The reason is initialized only by a renderer that defers the work. +export type ReactRecoverable = { $$typeof: symbol, + _reason: ReactRecoverableReason | void, }; export type StartTransitionOptions = {