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-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-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"'); + }); + }); }); 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 = {