diff --git a/.eslintrc.js b/.eslintrc.js index 1bb4e868d0694..8ffc5a732c877 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -611,6 +611,7 @@ module.exports = { TimeoutID: 'readonly', WheelEventHandler: 'readonly', FinalizationRegistry: 'readonly', + Exclude: 'readonly', Omit: 'readonly', Keyframe: 'readonly', PropertyIndexedKeyframes: 'readonly', diff --git a/fixtures/fizz/package.json b/fixtures/fizz/package.json index 096a392f4b524..7156aa9e8fa0c 100644 --- a/fixtures/fizz/package.json +++ b/fixtures/fizz/package.json @@ -28,8 +28,8 @@ "prettier": "1.19.1" }, "scripts": { - "predev": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/", + "predev": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", "dev": "concurrently \"npm run dev:server\" \"npm run dev:bundler\"", "start": "concurrently \"npm run start:server\" \"npm run start:bundler\"", "dev:server": "cross-env NODE_ENV=development nodemon -- --inspect server/server.js", diff --git a/fixtures/flight-esm/package.json b/fixtures/flight-esm/package.json index cb4ca1ea30b82..de711927588ca 100644 --- a/fixtures/flight-esm/package.json +++ b/fixtures/flight-esm/package.json @@ -17,8 +17,8 @@ "webpack-sources": "^3.2.0" }, "scripts": { - "predev": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/", + "predev": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", "dev": "concurrently \"npm run dev:region\" \"npm run dev:global\"", "dev:global": "NODE_ENV=development BUILD_PATH=dist node server/global", "dev:region": "NODE_ENV=development BUILD_PATH=dist nodemon --watch src --watch dist -- --enable-source-maps --experimental-loader ./loader/region.js --conditions=react-server server/region", diff --git a/fixtures/flight/package.json b/fixtures/flight/package.json index c63500727d02a..6d48c311cce3e 100644 --- a/fixtures/flight/package.json +++ b/fixtures/flight/package.json @@ -69,8 +69,8 @@ "@playwright/test": "^1.51.1" }, "scripts": { - "predev": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prebuild": "cp -r ../../build/oss-experimental/* ./node_modules/", + "predev": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prebuild": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", "dev": "concurrently \"npm run dev:region\" \"npm run dev:global\"", "dev:global": "NODE_ENV=development BUILD_PATH=dist node --experimental-loader ./loader/global.js --inspect=127.0.0.1:9230 server/global", "dev:region": "NODE_ENV=development BUILD_PATH=dist nodemon --watch src --watch dist -- --enable-source-maps --experimental-loader ./loader/region.js --conditions=react-server --inspect=127.0.0.1:9229 server/region", diff --git a/fixtures/owner-stacks/package.json b/fixtures/owner-stacks/package.json index cd288c8baeac7..2798aab568ecf 100644 --- a/fixtures/owner-stacks/package.json +++ b/fixtures/owner-stacks/package.json @@ -9,7 +9,7 @@ "web-vitals": "^2.1.0" }, "scripts": { - "prestart": "cp -a ../../build/oss-experimental/. node_modules", + "prestart": "cp -a ../../build/oss-experimental/. node_modules && rm -rf node_modules/.cache;", "start": "react-scripts start", "build": "react-scripts build", "test": "react-scripts test", diff --git a/fixtures/ssr/src/components/LargeContent.js b/fixtures/ssr/src/components/LargeContent.js index 7c4a6cf2258ef..f5c8adb03e233 100644 --- a/fixtures/ssr/src/components/LargeContent.js +++ b/fixtures/ssr/src/components/LargeContent.js @@ -6,7 +6,7 @@ import React, { export default function LargeContent() { return ( - +

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris diff --git a/fixtures/view-transition/package.json b/fixtures/view-transition/package.json index 7b9af04096027..8d222b29d3c07 100644 --- a/fixtures/view-transition/package.json +++ b/fixtures/view-transition/package.json @@ -22,9 +22,9 @@ ] }, "scripts": { - "predev": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prebuild": "cp -r ../../build/oss-experimental/* ./node_modules/", + "predev": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prebuild": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", "dev": "concurrently \"npm run dev:server\" \"npm run dev:client\"", "dev:client": "BROWSER=none PORT=3001 react-scripts start", "dev:server": "NODE_ENV=development node server", diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 2afa52f4c7f64..41421edb2a32a 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -78,6 +78,8 @@ import { logComponentRender, logDedupedComponentRender, logComponentErrored, + logIOInfo, + logComponentAwait, } from './ReactFlightPerformanceTrack'; import { @@ -675,11 +677,11 @@ function nullRefGetter() { } function getIOInfoTaskName(ioInfo: ReactIOInfo): string { - return ''; // TODO + return ioInfo.name || 'unknown'; } function getAsyncInfoTaskName(asyncInfo: ReactAsyncInfo): string { - return 'await'; // We could be smarter about this and give it a name like `then` or `Promise.all`. + return 'await ' + getIOInfoTaskName(asyncInfo.awaited); } function getServerComponentTaskName(componentInfo: ReactComponentInfo): string { @@ -811,7 +813,13 @@ function createElement( console, getTaskName(type), ); - const callStack = buildFakeCallStack(response, stack, env, createTaskFn); + const callStack = buildFakeCallStack( + response, + stack, + env, + false, + createTaskFn, + ); // This owner should ideally have already been initialized to avoid getting // user stack frames on the stack. const ownerTask = @@ -2132,6 +2140,7 @@ function resolveErrorDev( response, stack, env, + false, // $FlowFixMe[incompatible-use] Error.bind( null, @@ -2194,6 +2203,7 @@ function resolvePostponeDev( response, stack, env, + false, // $FlowFixMe[incompatible-use] Error.bind(null, reason || ''), ); @@ -2402,12 +2412,17 @@ function buildFakeCallStack( response: Response, stack: ReactStackTrace, environmentName: string, + useEnclosingLine: boolean, innerCall: () => T, ): () => T { let callStack = innerCall; for (let i = 0; i < stack.length; i++) { const frame = stack[i]; - const frameKey = frame.join('-') + '-' + environmentName; + const frameKey = + frame.join('-') + + '-' + + environmentName + + (useEnclosingLine ? '-e' : '-n'); let fn = fakeFunctionCache.get(frameKey); if (fn === undefined) { const [name, filename, line, col, enclosingLine, enclosingCol] = frame; @@ -2421,8 +2436,8 @@ function buildFakeCallStack( sourceMap, line, col, - enclosingLine, - enclosingCol, + useEnclosingLine ? line : enclosingLine, + useEnclosingLine ? col : enclosingCol, environmentName, ); // TODO: This cache should technically live on the response since the _debugFindSourceMapURL @@ -2468,6 +2483,15 @@ function initializeFakeTask( // If it's null, we can't initialize a task. return null; } + + // Workaround for a bug where Chrome Performance tracking uses the enclosing line/column + // instead of the callsite. For ReactAsyncInfo/ReactIOInfo, the only thing we're going + // to use the fake task for is the Performance tracking so we encode the enclosing line/ + // column at the callsite to get a better line number. We could do this for Components too + // but we're going to use those for other things too like console logs and it's not worth + // duplicating. If this bug is every fixed in Chrome, this should be set to false. + const useEnclosingLine = debugInfo.key === undefined; + const stack = debugInfo.stack; const env: string = debugInfo.env == null ? response._rootEnvironmentName : debugInfo.env; @@ -2484,6 +2508,7 @@ function initializeFakeTask( stack, '"use ' + childEnvironmentName.toLowerCase() + '"', env, + useEnclosingLine, ); } else { const cachedEntry = debugInfo.debugTask; @@ -2508,6 +2533,7 @@ function initializeFakeTask( stack, taskName, env, + useEnclosingLine, )); } } @@ -2518,9 +2544,16 @@ function buildFakeTask( stack: ReactStackTrace, taskName: string, env: string, + useEnclosingLine: boolean, ): ConsoleTask { const createTaskFn = (console: any).createTask.bind(console, taskName); - const callStack = buildFakeCallStack(response, stack, env, createTaskFn); + const callStack = buildFakeCallStack( + response, + stack, + env, + useEnclosingLine, + createTaskFn, + ); if (ownerTask === null) { const rootTask = getRootTask(response, env); if (rootTask != null) { @@ -2543,6 +2576,7 @@ const createFakeJSXCallStack = { response, stack, environmentName, + false, fakeJSXCallSite, ); return callStackForError(); @@ -2615,14 +2649,15 @@ function resolveDebugInfo( initializeFakeTask(response, componentInfoOrAsyncInfo, env); } if (debugInfo.owner === null && response._debugRootOwner != null) { - // $FlowFixMe[prop-missing] By narrowing `owner` to `null`, we narrowed `debugInfo` to `ReactComponentInfo` - const componentInfo: ReactComponentInfo = debugInfo; + const componentInfoOrAsyncInfo: ReactComponentInfo | ReactAsyncInfo = + // $FlowFixMe: By narrowing `owner` to `null`, we narrowed `debugInfo` to `ReactComponentInfo` + debugInfo; // $FlowFixMe[cannot-write] - componentInfo.owner = response._debugRootOwner; + componentInfoOrAsyncInfo.owner = response._debugRootOwner; // We override the stack if we override the owner since the stack where the root JSX // was created on the server isn't very useful but where the request was made is. // $FlowFixMe[cannot-write] - componentInfo.debugStack = response._debugRootStack; + componentInfoOrAsyncInfo.debugStack = response._debugRootStack; } else if (debugInfo.stack !== undefined) { const componentInfoOrAsyncInfo: ReactComponentInfo | ReactAsyncInfo = // $FlowFixMe[incompatible-type] @@ -2678,6 +2713,7 @@ const replayConsoleWithCallStack = { response, stackTrace, env, + false, bindToConsole(methodName, args, env), ); if (owner != null) { @@ -2756,19 +2792,18 @@ function resolveConsoleEntry( function initializeIOInfo(response: Response, ioInfo: ReactIOInfo): void { const env = - // TODO: Pass env through I/O info. - // ioInfo.env !== undefined ? ioInfo.env : - response._rootEnvironmentName; + ioInfo.env === undefined ? response._rootEnvironmentName : ioInfo.env; if (ioInfo.stack !== undefined) { initializeFakeTask(response, ioInfo, env); initializeFakeStack(response, ioInfo); } - // TODO: Initialize owner. // Adjust the time to the current environment's time space. // $FlowFixMe[cannot-write] ioInfo.start += response._timeOrigin; // $FlowFixMe[cannot-write] ioInfo.end += response._timeOrigin; + + logIOInfo(ioInfo, response._rootEnvironmentName); } function resolveIOInfo( @@ -2890,6 +2925,7 @@ function flushComponentPerformance( trackIdx, parentEndTime, previousEndTime, + response._rootEnvironmentName, ); } // Since we didn't bump the track this time, we just return the same track. @@ -2969,9 +3005,12 @@ function flushComponentPerformance( for (let i = debugInfo.length - 1; i >= 0; i--) { const info = debugInfo[i]; if (typeof info.time === 'number') { - endTime = info.time; - if (endTime > childrenEndTime) { - childrenEndTime = endTime; + if (info.time > childrenEndTime) { + childrenEndTime = info.time; + } + if (endTime === 0) { + // Last timestamp is the end of the last component. + endTime = info.time; } } if (typeof info.name === 'string' && i > 0) { @@ -3009,8 +3048,29 @@ function flushComponentPerformance( } // Track the root most component of the result for deduping logging. result.component = componentInfo; + // Set the end time of the previous component to the start of the previous. + endTime = startTime; } isLastComponent = false; + } else if (info.awaited && i > 0 && i < debugInfo.length - 2) { + // $FlowFixMe: Refined. + const asyncInfo: ReactAsyncInfo = info; + const startTimeInfo = debugInfo[i - 1]; + const endTimeInfo = debugInfo[i + 1]; + if ( + typeof startTimeInfo.time === 'number' && + typeof endTimeInfo.time === 'number' + ) { + const awaitStartTime = startTimeInfo.time; + const awaitEndTime = endTimeInfo.time; + logComponentAwait( + asyncInfo, + trackIdx, + awaitStartTime, + awaitEndTime, + response._rootEnvironmentName, + ); + } } } } diff --git a/packages/react-client/src/ReactFlightPerformanceTrack.js b/packages/react-client/src/ReactFlightPerformanceTrack.js index ed0f67a4f313e..faa5cf9650d9c 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -9,7 +9,11 @@ /* eslint-disable react-internal/no-production-logging */ -import type {ReactComponentInfo} from 'shared/ReactTypes'; +import type { + ReactComponentInfo, + ReactIOInfo, + ReactAsyncInfo, +} from 'shared/ReactTypes'; import {enableProfilerTimer} from 'shared/ReactFeatureFlags'; @@ -18,6 +22,7 @@ const supportsUserTiming = typeof console !== 'undefined' && typeof console.timeStamp === 'function'; +const IO_TRACK = 'Server Requests ⚛'; const COMPONENTS_TRACK = 'Server Components ⚛'; export function markAllTracksInOrder() { @@ -25,6 +30,14 @@ export function markAllTracksInOrder() { // Ensure we create the Server Component track groups earlier than the Client Scheduler // and Client Components. We can always add the 0 time slot even if it's in the past. // That's still considered for ordering. + console.timeStamp( + 'Server Requests Track', + 0.001, + 0.001, + IO_TRACK, + undefined, + 'primary-light', + ); console.timeStamp( 'Server Components Track', 0.001, @@ -166,9 +179,13 @@ export function logDedupedComponentRender( trackIdx: number, startTime: number, endTime: number, + rootEnv: string, ): void { if (supportsUserTiming && endTime >= 0 && trackIdx < 10) { + const env = componentInfo.env; const name = componentInfo.name; + const isPrimaryEnv = env === rootEnv; + const color = isPrimaryEnv ? 'primary-light' : 'secondary-light'; const entryName = name + ' [deduped]'; const debugTask = componentInfo.debugTask; if (__DEV__ && debugTask) { @@ -181,7 +198,7 @@ export function logDedupedComponentRender( endTime, trackNames[trackIdx], COMPONENTS_TRACK, - 'tertiary-light', + color, ), ); } else { @@ -191,7 +208,100 @@ export function logDedupedComponentRender( endTime, trackNames[trackIdx], COMPONENTS_TRACK, - 'tertiary-light', + color, + ); + } + } +} + +function getIOColor( + functionName: string, +): 'tertiary-light' | 'tertiary' | 'tertiary-dark' { + // Add some color variation to be able to distinguish various sources. + switch (functionName.charCodeAt(0) % 3) { + case 0: + return 'tertiary-light'; + case 1: + return 'tertiary'; + default: + return 'tertiary-dark'; + } +} + +export function logComponentAwait( + asyncInfo: ReactAsyncInfo, + trackIdx: number, + startTime: number, + endTime: number, + rootEnv: string, +): void { + if (supportsUserTiming && endTime > 0) { + const env = asyncInfo.env; + const name = asyncInfo.awaited.name; + const isPrimaryEnv = env === rootEnv; + const color = getIOColor(name); + const entryName = + 'await ' + + (isPrimaryEnv || env === undefined ? name : name + ' [' + env + ']'); + const debugTask = asyncInfo.debugTask; + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + entryName, + startTime < 0 ? 0 : startTime, + endTime, + trackNames[trackIdx], + COMPONENTS_TRACK, + color, + ), + ); + } else { + console.timeStamp( + entryName, + startTime < 0 ? 0 : startTime, + endTime, + trackNames[trackIdx], + COMPONENTS_TRACK, + color, + ); + } + } +} + +export function logIOInfo(ioInfo: ReactIOInfo, rootEnv: string): void { + const startTime = ioInfo.start; + const endTime = ioInfo.end; + if (supportsUserTiming && endTime >= 0) { + const name = ioInfo.name; + const env = ioInfo.env; + const isPrimaryEnv = env === rootEnv; + const entryName = + isPrimaryEnv || env === undefined ? name : name + ' [' + env + ']'; + const debugTask = ioInfo.debugTask; + const color = getIOColor(name); + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + entryName, + startTime < 0 ? 0 : startTime, + endTime, + IO_TRACK, + undefined, + color, + ), + ); + } else { + console.timeStamp( + entryName, + startTime < 0 ? 0 : startTime, + endTime, + IO_TRACK, + undefined, + color, ); } } diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index d5f635f964934..57124ec6e0c0e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1289,7 +1289,7 @@ describe('ReactDOMFizzServer', () => { function App({showMore}) { return (

- + {a} {b} {showMore ? ( diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 478bed90a3a0d..c306b1369b174 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -2254,7 +2254,7 @@ describe('ReactDOMFizzStaticBrowser', () => { function App() { return (
- + diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js index 977d2dbf154f4..74db51d242960 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js @@ -197,6 +197,70 @@ describe('ReactDOMFizzSuspenseList', () => { ); }); + // @gate enableSuspenseList + it('independently with revealOrder="independent"', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + + ); + } + + await A.resolve(); + + await serverAct(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + pipe(writable); + }); + + assertLog(['A', 'Suspend! [B]', 'Suspend! [C]', 'Loading B', 'Loading C']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + Loading B + Loading C +
, + ); + + await serverAct(() => C.resolve()); + assertLog(['C']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + Loading B + C +
, + ); + + await serverAct(() => B.resolve()); + assertLog(['B']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + B + C +
, + ); + }); + // @gate enableSuspenseList it('displays all "together"', async () => { const A = createAsyncText('A'); @@ -452,7 +516,7 @@ describe('ReactDOMFizzSuspenseList', () => { }); // @gate enableSuspenseList - it('displays all "together" in nested SuspenseLists where the inner is default', async () => { + it('displays all "together" in nested SuspenseLists where the inner is "independent"', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -464,7 +528,7 @@ describe('ReactDOMFizzSuspenseList', () => { }> - + }> @@ -523,7 +587,7 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- + }> @@ -586,7 +650,7 @@ describe('ReactDOMFizzSuspenseList', () => { }); // @gate enableSuspenseList - it('displays each items in "backwards" order', async () => { + it('displays each items in "backwards" order in legacy mode', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -594,7 +658,7 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- + }> @@ -665,8 +729,10 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- - + + }> @@ -736,7 +802,7 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- + }> @@ -791,7 +857,7 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- + }> diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 6436f1898b798..b8a4e5b86ae91 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -5755,7 +5755,7 @@ body { - + diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 8cace332cd552..5b719ddf4acc5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -2362,7 +2362,7 @@ describe('ReactDOMServerPartialHydration', () => { function App({showMore}) { return ( - + {a} {b} {showMore ? ( diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js index 8a959a294987c..6e6f9bb09261e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js @@ -123,7 +123,7 @@ describe('ReactDOMServerSuspense', () => { // @gate enableSuspenseList it('server renders a SuspenseList component and its children', async () => { const example = ( - +
A
 diff --git a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js index e221f01b92d65..0c73410f2edf0 100644 --- a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js +++ b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js @@ -172,7 +172,7 @@ test('regression (#20932): return pointer is correct before entering deleted tre function App() { return ( - + }> diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index fcb2406552899..f574162b41b0c 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -2097,7 +2097,9 @@ export function validateSuspenseListChildren( ) { if (__DEV__) { if ( - (revealOrder === 'forwards' || revealOrder === 'backwards') && + (revealOrder === 'forwards' || + revealOrder === 'backwards' || + revealOrder === 'unstable_legacy-backwards') && children !== undefined && children !== null && children !== false diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 7b86962f778fe..a9316163156f5 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -337,7 +337,7 @@ if (__DEV__) { didWarnAboutContextTypes = ({}: {[string]: boolean}); didWarnAboutGetDerivedStateOnFunctionComponent = ({}: {[string]: boolean}); didWarnAboutReassigningProps = false; - didWarnAboutRevealOrder = ({}: {[empty]: boolean}); + didWarnAboutRevealOrder = ({}: {[string]: boolean}); didWarnAboutTailOptions = ({}: {[string]: boolean}); didWarnAboutDefaultPropsOnFunctionComponent = ({}: {[string]: boolean}); didWarnAboutClassNameOnViewTransition = ({}: {[string]: boolean}); @@ -3225,19 +3225,32 @@ function findLastContentRow(firstChild: null | Fiber): null | Fiber { function validateRevealOrder(revealOrder: SuspenseListRevealOrder) { if (__DEV__) { + const cacheKey = revealOrder == null ? 'null' : revealOrder; if ( - revealOrder !== undefined && revealOrder !== 'forwards' && - revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' && revealOrder !== 'together' && - !didWarnAboutRevealOrder[revealOrder] + revealOrder !== 'independent' && + !didWarnAboutRevealOrder[cacheKey] ) { - didWarnAboutRevealOrder[revealOrder] = true; - if (typeof revealOrder === 'string') { + didWarnAboutRevealOrder[cacheKey] = true; + if (revealOrder == null) { + console.error( + 'The default for the prop is changing. ' + + 'To be future compatible you must explictly specify either ' + + '"independent" (the current default), "together", "forwards" or "legacy_unstable-backwards".', + ); + } else if (revealOrder === 'backwards') { + console.error( + 'The rendering order of is changing. ' + + 'To be future compatible you must specify revealOrder="legacy_unstable-backwards" instead.', + ); + } else if (typeof revealOrder === 'string') { switch (revealOrder.toLowerCase()) { case 'together': case 'forwards': - case 'backwards': { + case 'backwards': + case 'independent': { console.error( '"%s" is not a valid value for revealOrder on . ' + 'Use lowercase "%s" instead.', @@ -3259,7 +3272,7 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) { default: console.error( '"%s" is not a supported revealOrder on . ' + - 'Did you mean "together", "forwards" or "backwards"?', + 'Did you mean "independent", "together", "forwards" or "backwards"?', revealOrder, ); break; @@ -3267,7 +3280,7 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) { } else { console.error( '%s is not a supported value for revealOrder on . ' + - 'Did you mean "together", "forwards" or "backwards"?', + 'Did you mean "independent", "together", "forwards" or "backwards"?', revealOrder, ); } @@ -3280,16 +3293,38 @@ function validateTailOptions( revealOrder: SuspenseListRevealOrder, ) { if (__DEV__) { - if (tailMode !== undefined && !didWarnAboutTailOptions[tailMode]) { - if (tailMode !== 'collapsed' && tailMode !== 'hidden') { - didWarnAboutTailOptions[tailMode] = true; + const cacheKey = tailMode == null ? 'null' : tailMode; + if (!didWarnAboutTailOptions[cacheKey]) { + if (tailMode == null) { + if ( + revealOrder === 'forwards' || + revealOrder === 'backwards' || + revealOrder === 'unstable_legacy-backwards' + ) { + didWarnAboutTailOptions[cacheKey] = true; + console.error( + 'The default for the prop is changing. ' + + 'To be future compatible you must explictly specify either ' + + '"visible" (the current default), "collapsed" or "hidden".', + ); + } + } else if ( + tailMode !== 'visible' && + tailMode !== 'collapsed' && + tailMode !== 'hidden' + ) { + didWarnAboutTailOptions[cacheKey] = true; console.error( '"%s" is not a supported value for tail on . ' + - 'Did you mean "collapsed" or "hidden"?', + 'Did you mean "visible", "collapsed" or "hidden"?', tailMode, ); - } else if (revealOrder !== 'forwards' && revealOrder !== 'backwards') { - didWarnAboutTailOptions[tailMode] = true; + } else if ( + revealOrder !== 'forwards' && + revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' + ) { + didWarnAboutTailOptions[cacheKey] = true; console.error( ' is only valid if revealOrder is ' + '"forwards" or "backwards". ' + @@ -3414,7 +3449,8 @@ function updateSuspenseListComponent( ); break; } - case 'backwards': { + case 'backwards': + case 'unstable_legacy-backwards': { // We're going to find the first row that has existing content. // At the same time we're going to reverse the list of everything // we pass in the meantime. That's going to be our tail in reverse diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js index f7ae78ed6d313..2542d660c4c47 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js @@ -75,9 +75,12 @@ export function findFirstSuspended(row: Fiber): null | Fiber { } } else if ( node.tag === SuspenseListComponent && - // revealOrder undefined can't be trusted because it don't + // Independent revealOrder can't be trusted because it doesn't // keep track of whether it suspended or not. - node.memoizedProps.revealOrder !== undefined + (node.memoizedProps.revealOrder === 'forwards' || + node.memoizedProps.revealOrder === 'backwards' || + node.memoizedProps.revealOrder === 'unstable_legacy-backwards' || + node.memoizedProps.revealOrder === 'together') ) { const didSuspend = (node.flags & DidCapture) !== NoFlags; if (didSuspend) { diff --git a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js index 6ed19ba6d505b..0a338c8aa8710 100644 --- a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js +++ b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js @@ -677,7 +677,7 @@ describe('ReactLazyContextPropagation', () => { setContext = setValue; const children = React.useMemo( () => ( - + diff --git a/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js b/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js index 1b8beba58b07e..9e4e9694a6a88 100644 --- a/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js +++ b/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js @@ -255,7 +255,7 @@ describe('ReactFragment', () => { onCaughtError, }).render( - + , diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index f9efb330cf891..bfbf165dbaba6 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -79,7 +79,7 @@ describe('ReactSuspenseList', () => { }); assertConsoleErrorDev([ '"something" is not a supported revealOrder on ' + - '. Did you mean "together", "forwards" or "backwards"?' + + '. Did you mean "independent", "together", "forwards" or "backwards"?' + '\n in SuspenseList (at **)' + '\n in Foo (at **)', ]); @@ -131,7 +131,11 @@ describe('ReactSuspenseList', () => { // @gate enableSuspenseList it('warns if a single element is passed to a "forwards" list', async () => { function Foo({children}) { - return {children}; + return ( + + {children} + + ); } ReactNoop.render(); @@ -166,7 +170,7 @@ describe('ReactSuspenseList', () => { it('warns if a single fragment is passed to a "backwards" list', async () => { function Foo() { return ( - + <>{[]} ); @@ -176,7 +180,7 @@ describe('ReactSuspenseList', () => { ReactNoop.render(); }); assertConsoleErrorDev([ - 'A single row was passed to a . ' + + 'A single row was passed to a . ' + 'This is not useful since it needs multiple rows. ' + 'Did you mean to pass multiple children or an array?' + '\n in SuspenseList (at **)' + @@ -188,7 +192,7 @@ describe('ReactSuspenseList', () => { it('warns if a nested array is passed to a "forwards" list', async () => { function Foo({items}) { return ( - + {items.map(name => ( {name} @@ -214,7 +218,7 @@ describe('ReactSuspenseList', () => { }); // @gate enableSuspenseList - it('shows content independently by default', async () => { + it('warns if no revealOrder is specified', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -250,6 +254,86 @@ describe('ReactSuspenseList', () => { 'Suspend! [C]', ]); + assertConsoleErrorDev([ + 'The default for the prop is changing. ' + + 'To be future compatible you must explictly specify either ' + + '"independent" (the current default), "together", "forwards" or "legacy_unstable-backwards".' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading B + Loading C + , + ); + + await act(() => C.resolve()); + assertLog( + gate('alwaysThrottleRetries') + ? ['Suspend! [B]', 'C', 'Suspend! [B]'] + : ['C'], + ); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading B + C + , + ); + + await act(() => B.resolve()); + assertLog(['B']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); + }); + + // @gate enableSuspenseList + it('shows content independently with revealOrder="independent"', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + + }> + + + }> + + + }> + + + + ); + } + + await A.resolve(); + + ReactNoop.render(); + + await waitForAll([ + 'A', + 'Suspend! [B]', + 'Loading B', + 'Suspend! [C]', + 'Loading C', + // pre-warming + 'Suspend! [B]', + 'Suspend! [C]', + ]); + expect(ReactNoop).toMatchRenderedOutput( <> A @@ -564,7 +648,7 @@ describe('ReactSuspenseList', () => { }); // @gate enableSuspenseList - it('displays all "together" in nested SuspenseLists where the inner is default', async () => { + it('displays all "together" in nested SuspenseLists where the inner is "independent"', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -575,7 +659,7 @@ describe('ReactSuspenseList', () => { }> - + }> @@ -897,7 +981,7 @@ describe('ReactSuspenseList', () => { function Foo() { return ( - + }> @@ -955,6 +1039,85 @@ describe('ReactSuspenseList', () => { ); }); + // @gate enableSuspenseList + it('warns if revealOrder="backwards" is specified', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + + }> + + + }> + + + }> + + + + ); + } + + await A.resolve(); + + ReactNoop.render(); + + await waitForAll([ + 'Suspend! [C]', + 'Loading C', + 'Loading B', + 'Loading A', + // pre-warming + 'Suspend! [C]', + ]); + + assertConsoleErrorDev([ + 'The rendering order of is changing. ' + + 'To be future compatible you must specify ' + + 'revealOrder="legacy_unstable-backwards" instead.' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + Loading A + Loading B + Loading C + , + ); + + await act(() => C.resolve()); + assertLog([ + 'C', + 'Suspend! [B]', + // pre-warming + 'Suspend! [B]', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + Loading A + Loading B + C + , + ); + + await act(() => B.resolve()); + assertLog(['B', 'A']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); + }); + // @gate enableSuspenseList it('displays each items in "backwards" order', async () => { const A = createAsyncText('A'); @@ -963,7 +1126,7 @@ describe('ReactSuspenseList', () => { function Foo() { return ( - + }> @@ -1037,7 +1200,7 @@ describe('ReactSuspenseList', () => { function Foo({items}) { return ( - + {items.map(([key, Component]) => ( }> @@ -1222,7 +1385,7 @@ describe('ReactSuspenseList', () => { function Foo({items}) { return ( - + {items.map(([key, Component]) => ( }> @@ -1400,7 +1563,7 @@ describe('ReactSuspenseList', () => { it('switches to rendering fallbacks if the tail takes long CPU time', async () => { function Foo() { return ( - + }> @@ -1535,6 +1698,29 @@ describe('ReactSuspenseList', () => { ); }); + // @gate enableSuspenseList + it('warns if no tail option is specified', async () => { + function Foo() { + return ( + + A + B + + ); + } + + await act(() => { + ReactNoop.render(); + }); + assertConsoleErrorDev([ + 'The default for the prop is changing. ' + + 'To be future compatible you must explictly specify either ' + + '"visible" (the current default), "collapsed" or "hidden".' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ]); + }); + // @gate enableSuspenseList it('warns if an unsupported tail option is used', async () => { function Foo() { @@ -1551,7 +1737,7 @@ describe('ReactSuspenseList', () => { }); assertConsoleErrorDev([ '"collapse" is not a supported value for tail on ' + - '. Did you mean "collapsed" or "hidden"?' + + '. Did you mean "visible", "collapsed" or "hidden"?' + '\n in SuspenseList (at **)' + '\n in Foo (at **)', ]); @@ -1796,7 +1982,7 @@ describe('ReactSuspenseList', () => { function Foo({items}) { return ( - + {items.map(([key, Component]) => ( }> @@ -2154,7 +2340,7 @@ describe('ReactSuspenseList', () => { function Foo() { return ( - + }> @@ -2255,7 +2441,7 @@ describe('ReactSuspenseList', () => { function Foo({showB}) { return ( - + }> @@ -2321,7 +2507,7 @@ describe('ReactSuspenseList', () => { function Foo() { return (
- + @@ -2673,7 +2859,7 @@ describe('ReactSuspenseList', () => { function App() { Scheduler.log('App'); return ( - + }> @@ -2760,7 +2946,7 @@ describe('ReactSuspenseList', () => { Scheduler.log('App'); return ( - + }> @@ -2936,7 +3122,7 @@ describe('ReactSuspenseList', () => { // Several layers of Bailout wrappers help verify we're // marking updates all the way to the propagation root. return ( - + @@ -3029,7 +3215,7 @@ describe('ReactSuspenseList', () => { function Repro({update}) { return ( - + {update && ( }> @@ -3128,7 +3314,7 @@ describe('ReactSuspenseList', () => { } function Foo() { return ( - + ); @@ -3184,7 +3370,11 @@ describe('ReactSuspenseList', () => { }; function Foo() { - return {iterable}; + return ( + + {iterable} + + ); } await act(() => { @@ -3270,7 +3460,7 @@ describe('ReactSuspenseList', () => { it('warns if a nested async iterable is passed to a "forwards" list', async () => { function Foo({items}) { return ( - + {items}
Tail
 diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js index cad1a011817f6..2e252acbf3be7 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js @@ -345,7 +345,7 @@ describe('ReactSuspenseyCommitPhase', () => { it('demonstrate current behavior when used with SuspenseList (not ideal)', async () => { function App() { return ( - + }> diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index a18d8d16748f9..579edf25c9102 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1799,7 +1799,7 @@ function renderSuspenseListRows( task: Task, keyPath: KeyNode, rows: Array, - revealOrder: 'forwards' | 'backwards', + revealOrder: 'forwards' | 'backwards' | 'unstable_legacy-backwards', ): void { // This is a fork of renderChildrenArray that's aware of tracking rows. const prevKeyPath = task.keyPath; @@ -1827,7 +1827,11 @@ function renderSuspenseListRows( // Since we are going to resume into a slot whose order was already // determined by the prerender, we can safely resume it even in reverse // render order. - const i = revealOrder !== 'backwards' ? n : totalChildren - 1 - n; + const i = + revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' + ? n + : totalChildren - 1 - n; const node = rows[i]; task.row = previousSuspenseListRow = createSuspenseListRow( previousSuspenseListRow, @@ -1852,7 +1856,11 @@ function renderSuspenseListRows( // Since we are going to resume into a slot whose order was already // determined by the prerender, we can safely resume it even in reverse // render order. - const i = revealOrder !== 'backwards' ? n : totalChildren - 1 - n; + const i = + revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' + ? n + : totalChildren - 1 - n; const node = rows[i]; if (__DEV__) { warnForMissingKey(request, task, node); @@ -1869,7 +1877,10 @@ function renderSuspenseListRows( } } else { task = ((task: any): RenderTask); // Refined - if (revealOrder !== 'backwards') { + if ( + revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' + ) { // Forwards direction for (let i = 0; i < totalChildren; i++) { const node = rows[i]; @@ -1973,7 +1984,11 @@ function renderSuspenseList( const revealOrder: SuspenseListRevealOrder = props.revealOrder; // TODO: Support tail hidden/collapsed modes. // const tailMode: SuspenseListTailMode = props.tail; - if (revealOrder === 'forwards' || revealOrder === 'backwards') { + if ( + revealOrder === 'forwards' || + revealOrder === 'backwards' || + revealOrder === 'unstable_legacy-backwards' + ) { // For ordered reveal, we need to produce rows from the children. if (isArray(children)) { renderSuspenseListRows(request, task, keyPath, children, revealOrder); diff --git a/packages/react-server/src/ReactFlightAsyncSequence.js b/packages/react-server/src/ReactFlightAsyncSequence.js index 533990d827a53..b36fb551091ee 100644 --- a/packages/react-server/src/ReactFlightAsyncSequence.js +++ b/packages/react-server/src/ReactFlightAsyncSequence.js @@ -7,12 +7,15 @@ * @flow */ +import type {ReactComponentInfo} from 'shared/ReactTypes'; + export const IO_NODE = 0; export const PROMISE_NODE = 1; export const AWAIT_NODE = 2; export type IONode = { tag: 0, + owner: null | ReactComponentInfo, stack: Error, // callsite that spawned the I/O start: number, // start time when the first part of the I/O sequence started end: number, // we typically don't use this. only when there's no promise intermediate. @@ -22,6 +25,7 @@ export type IONode = { export type PromiseNode = { tag: 1, + owner: null | ReactComponentInfo, stack: Error, // callsite that created the Promise start: number, // start time when the Promise was created end: number, // end time when the Promise was resolved. @@ -31,9 +35,10 @@ export type PromiseNode = { export type AwaitNode = { tag: 2, + owner: null | ReactComponentInfo, stack: Error, // callsite that awaited (using await, .then(), Promise.all(), ...) - start: -1.1, // not used. We use the timing of the awaited promise. - end: -1.1, // not used. + start: number, // when we started blocking. This might be later than the I/O started. + end: number, // when we unblocked. This might be later than the I/O resolved if there's CPU time. awaited: null | AsyncSequence, // the promise we were waiting on previous: null | AsyncSequence, // the sequence that was blocking us from awaiting in the first place }; diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 6a27329560882..e5c6043652c85 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -64,6 +64,7 @@ import type { ReactAsyncInfo, ReactTimeInfo, ReactStackTrace, + ReactCallSite, ReactFunctionLocation, ReactErrorInfo, ReactErrorInfoDev, @@ -164,55 +165,73 @@ function defaultFilterStackFrame( ); } -// DEV-only cache of parsed and filtered stack frames. -const stackTraceCache: WeakMap = __DEV__ - ? new WeakMap() - : (null: any); +function devirtualizeURL(url: string): string { + if (url.startsWith('rsc://React/')) { + // This callsite is a virtual fake callsite that came from another Flight client. + // We need to reverse it back into the original location by stripping its prefix + // and suffix. We don't need the environment name because it's available on the + // parent object that will contain the stack. + const envIdx = url.indexOf('/', 12); + const suffixIdx = url.lastIndexOf('?'); + if (envIdx > -1 && suffixIdx > -1) { + return url.slice(envIdx + 1, suffixIdx); + } + } + return url; +} -function filterStackTrace( +function findCalledFunctionNameFromStackTrace( request: Request, - error: Error, - skipFrames: number, -): ReactStackTrace { - const existing = stackTraceCache.get(error); - if (existing !== undefined) { - // Return a clone because the Flight protocol isn't yet resilient to deduping - // objects in the debug info. TODO: Support deduping stacks. - const clone = existing.slice(0); - for (let i = 0; i < clone.length; i++) { - // $FlowFixMe[invalid-tuple-arity] - clone[i] = clone[i].slice(0); + stack: ReactStackTrace, +): string { + // Gets the name of the first function called from first party code. + let bestMatch = ''; + const filterStackFrame = request.filterStackFrame; + for (let i = 0; i < stack.length; i++) { + const callsite = stack[i]; + const functionName = callsite[0]; + const url = devirtualizeURL(callsite[1]); + if (filterStackFrame(url, functionName)) { + if (bestMatch === '') { + // If we had no good stack frames for internal calls, just use the last + // first party function name. + return functionName; + } + return bestMatch; + } else if (functionName === 'new Promise') { + // Ignore Promise constructors. + } else if (url === 'node:internal/async_hooks') { + // Ignore the stack frames from the async hooks themselves. + } else { + bestMatch = functionName; } - return clone; } + return ''; +} + +function filterStackTrace( + request: Request, + stack: ReactStackTrace, +): ReactStackTrace { // Since stacks can be quite large and we pass a lot of them, we filter them out eagerly // to save bandwidth even in DEV. We'll also replay these stacks on the client so by // stripping them early we avoid that overhead. Otherwise we'd normally just rely on // the DevTools or framework's ignore lists to filter them out. const filterStackFrame = request.filterStackFrame; - const stack = parseStackTrace(error, skipFrames); + const filteredStack: ReactStackTrace = []; for (let i = 0; i < stack.length; i++) { const callsite = stack[i]; const functionName = callsite[0]; - let url = callsite[1]; - if (url.startsWith('rsc://React/')) { - // This callsite is a virtual fake callsite that came from another Flight client. - // We need to reverse it back into the original location by stripping its prefix - // and suffix. We don't need the environment name because it's available on the - // parent object that will contain the stack. - const envIdx = url.indexOf('/', 12); - const suffixIdx = url.lastIndexOf('?'); - if (envIdx > -1 && suffixIdx > -1) { - url = callsite[1] = url.slice(envIdx + 1, suffixIdx); - } - } - if (!filterStackFrame(url, functionName)) { - stack.splice(i, 1); - i--; + const url = devirtualizeURL(callsite[1]); + if (filterStackFrame(url, functionName)) { + // Use a clone because the Flight protocol isn't yet resilient to deduping + // objects in the debug info. TODO: Support deduping stacks. + const clone: ReactCallSite = (callsite.slice(0): any); + clone[1] = url; + filteredStack.push(clone); } } - stackTraceCache.set(error, stack); - return stack; + return filteredStack; } initAsyncDebugInfo(); @@ -240,8 +259,7 @@ function patchConsole(consoleInst: typeof console, methodName: string) { // one stack frame but keeping it simple for now and include all frames. const stack = filterStackTrace( request, - new Error('react-stack-top-frame'), - 1, + parseStackTrace(new Error('react-stack-top-frame'), 1), ); request.pendingChunks++; const owner: null | ReactComponentInfo = resolveOwner(); @@ -1078,7 +1096,7 @@ function callWithDebugContextInDEV( componentDebugInfo.stack = task.debugStack === null ? null - : filterStackTrace(request, task.debugStack, 1); + : filterStackTrace(request, parseStackTrace(task.debugStack, 1)); // $FlowFixMe[cannot-write] componentDebugInfo.debugStack = task.debugStack; // $FlowFixMe[cannot-write] @@ -1279,7 +1297,7 @@ function renderFunctionComponent( componentDebugInfo.stack = task.debugStack === null ? null - : filterStackTrace(request, task.debugStack, 1); + : filterStackTrace(request, parseStackTrace(task.debugStack, 1)); // $FlowFixMe[cannot-write] componentDebugInfo.props = props; // $FlowFixMe[cannot-write] @@ -1615,7 +1633,7 @@ function renderClientElement( task.debugOwner, task.debugStack === null ? null - : filterStackTrace(request, task.debugStack, 1), + : filterStackTrace(request, parseStackTrace(task.debugStack, 1)), validated, ] : [REACT_ELEMENT_TYPE, type, key, props]; @@ -1748,7 +1766,7 @@ function renderElement( stack: task.debugStack === null ? null - : filterStackTrace(request, task.debugStack, 1), + : filterStackTrace(request, parseStackTrace(task.debugStack, 1)), props: props, debugStack: task.debugStack, debugTask: task.debugTask, @@ -1877,7 +1895,10 @@ function visitAsyncNode( // We don't log it yet though. We return it to be logged by the point where it's awaited. // The ioNode might be another PromiseNode in the case where none of the AwaitNode had // unfiltered stacks. - if (filterStackTrace(request, node.stack, 1).length === 0) { + if ( + filterStackTrace(request, parseStackTrace(node.stack, 1)).length === + 0 + ) { // Typically we assume that the outer most Promise that was awaited in user space has the // most actionable stack trace for the start of the operation. However, if this Promise // was created inside only third party code, then try to use the inner node instead. @@ -1898,7 +1919,28 @@ function visitAsyncNode( if (awaited !== null) { const ioNode = visitAsyncNode(request, task, awaited, cutOff, visited); if (ioNode !== null) { - const stack = filterStackTrace(request, node.stack, 1); + if (node.end < 0) { + // If we haven't defined an end time, use the resolve of the inner Promise. + // This can happen because the ping gets invoked before the await gets resolved. + if (ioNode.end < node.start) { + // If we're awaiting a resolved Promise it could have finished before we started. + node.end = node.start; + } else { + node.end = ioNode.end; + } + } + if (node.end < cutOff) { + // This was already resolved when we started this sequence. It must have been + // part of a different component. + // TODO: Think of some other way to exclude irrelevant data since if we awaited + // a cached promise, we should still log this component as being dependent on that data. + return null; + } + + const stack = filterStackTrace( + request, + parseStackTrace(node.stack, 1), + ); if (stack.length === 0) { // If this await was fully filtered out, then it was inside third party code // such as in an external library. We return the I/O node and try another await. @@ -1906,12 +1948,27 @@ function visitAsyncNode( } // Outline the IO node. serializeIONode(request, ioNode); + // We log the environment at the time when the last promise pigned ping which may + // be later than what the environment was when we actually started awaiting. + const env = (0, request.environmentName)(); + if (node.start <= cutOff) { + // If this was an await that started before this sequence but finished after, + // then we clamp it to the start of this sequence. We don't need to emit a time + // TODO: Typically we'll already have a previous time stamp with the cutOff time + // so we shouldn't need to emit another one. But not always. + emitTimingChunk(request, task.id, cutOff); + } else { + emitTimingChunk(request, task.id, node.start); + } // Then emit a reference to us awaiting it in the current task. request.pendingChunks++; emitDebugChunk(request, task.id, { awaited: ((ioNode: any): ReactIOInfo), // This is deduped by this reference. + env: env, + owner: node.owner, stack: stack, }); + emitTimingChunk(request, task.id, node.end); } } // If we had awaited anything we would have written it now. @@ -1944,9 +2001,17 @@ function emitAsyncSequence( } serializeIONode(request, awaitedNode); request.pendingChunks++; + // We log the environment at the time when we ping which may be later than what the + // environment was when we actually started awaiting. + const env = (0, request.environmentName)(); + // If we don't have any thing awaited, the time we started awaiting was internal + // when we yielded after rendering. The cutOff time is basically that. + emitTimingChunk(request, task.id, cutOff); emitDebugChunk(request, task.id, { awaited: ((awaitedNode: any): ReactIOInfo), // This is deduped by this reference. + env: env, }); + emitTimingChunk(request, task.id, awaitedNode.end); } } @@ -3272,7 +3337,7 @@ function emitPostponeChunk( try { // eslint-disable-next-line react-internal/safe-string-coercion reason = String(postponeInstance.message); - stack = filterStackTrace(request, postponeInstance, 0); + stack = filterStackTrace(request, parseStackTrace(postponeInstance, 0)); } catch (x) { stack = []; } @@ -3295,7 +3360,7 @@ function serializeErrorValue(request: Request, error: Error): string { name = error.name; // eslint-disable-next-line react-internal/safe-string-coercion message = String(error.message); - stack = filterStackTrace(request, error, 0); + stack = filterStackTrace(request, parseStackTrace(error, 0)); const errorEnv = (error: any).environmentName; if (typeof errorEnv === 'string') { // This probably came from another FlightClient as a pass through. @@ -3334,7 +3399,7 @@ function emitErrorChunk( name = error.name; // eslint-disable-next-line react-internal/safe-string-coercion message = String(error.message); - stack = filterStackTrace(request, error, 0); + stack = filterStackTrace(request, parseStackTrace(error, 0)); const errorEnv = (error: any).environmentName; if (typeof errorEnv === 'string') { // This probably came from another FlightClient as a pass through. @@ -3496,8 +3561,11 @@ function outlineComponentInfo( function emitIOInfoChunk( request: Request, id: number, + name: string, start: number, end: number, + env: ?string, + owner: ?ReactComponentInfo, stack: ?ReactStackTrace, ): void { if (!__DEV__) { @@ -3532,10 +3600,22 @@ function emitIOInfoChunk( const relativeStartTimestamp = start - request.timeOrigin; const relativeEndTimestamp = end - request.timeOrigin; const debugIOInfo: Omit = { + name: name, start: relativeStartTimestamp, end: relativeEndTimestamp, - stack: stack, }; + if (env != null) { + // $FlowFixMe[cannot-write] + debugIOInfo.env = env; + } + if (stack != null) { + // $FlowFixMe[cannot-write] + debugIOInfo.stack = stack; + } + if (owner != null) { + // $FlowFixMe[cannot-write] + debugIOInfo.owner = owner; + } // $FlowFixMe[incompatible-type] stringify can return null const json: string = stringify(debugIOInfo, replacer); const row = id.toString(16) + ':J' + json + '\n'; @@ -3551,7 +3631,21 @@ function outlineIOInfo(request: Request, ioInfo: ReactIOInfo): void { // We can't serialize the ConsoleTask/Error objects so we need to omit them before serializing. request.pendingChunks++; const id = request.nextChunkId++; - emitIOInfoChunk(request, id, ioInfo.start, ioInfo.end, ioInfo.stack); + const owner = ioInfo.owner; + // Ensure the owner is already outlined. + if (owner != null) { + outlineComponentInfo(request, owner); + } + emitIOInfoChunk( + request, + id, + ioInfo.name, + ioInfo.start, + ioInfo.end, + ioInfo.env, + owner, + ioInfo.stack, + ); request.writtenObjects.set(ioInfo, serializeByValueID(id)); } @@ -3566,12 +3660,41 @@ function serializeIONode( } let stack = null; + let name = ''; if (ioNode.stack !== null) { - stack = filterStackTrace(request, ioNode.stack, 1); + const fullStack = parseStackTrace(ioNode.stack, 1); + stack = filterStackTrace(request, fullStack); + name = findCalledFunctionNameFromStackTrace(request, fullStack); + // The name can include the object that this was called on but sometimes that's + // just unnecessary context. + if (name.startsWith('Window.')) { + name = name.slice(7); + } else if (name.startsWith('.')) { + name = name.slice(7); + } + } + const owner = ioNode.owner; + // Ensure the owner is already outlined. + if (owner != null) { + outlineComponentInfo(request, owner); } + + // We log the environment at the time when we serialize the I/O node. + // The environment name may have changed from when the I/O was actually started. + const env = (0, request.environmentName)(); + request.pendingChunks++; const id = request.nextChunkId++; - emitIOInfoChunk(request, id, ioNode.start, ioNode.end, stack); + emitIOInfoChunk( + request, + id, + name, + ioNode.start, + ioNode.end, + env, + owner, + stack, + ); const ref = serializeByValueID(id); request.writtenObjects.set(ioNode, ref); return ref; @@ -3712,7 +3835,10 @@ function renderConsoleValue( let debugStack: null | ReactStackTrace = null; if (element._debugStack != null) { // Outline the debug stack so that it doesn't get cut off. - debugStack = filterStackTrace(request, element._debugStack, 1); + debugStack = filterStackTrace( + request, + parseStackTrace(element._debugStack, 1), + ); doNotLimit.add(debugStack); for (let i = 0; i < debugStack.length; i++) { doNotLimit.add(debugStack[i]); @@ -4094,6 +4220,8 @@ function forwardDebugInfo( const debugAsyncInfo: Omit = { awaited: ioInfo, + env: debugInfo[i].env, + owner: debugInfo[i].owner, stack: debugInfo[i].stack, }; emitDebugChunk(request, id, debugAsyncInfo); diff --git a/packages/react-server/src/ReactFlightServerConfigDebugNode.js b/packages/react-server/src/ReactFlightServerConfigDebugNode.js index b1e2dd4f27e9c..7a86231ed9a5c 100644 --- a/packages/react-server/src/ReactFlightServerConfigDebugNode.js +++ b/packages/react-server/src/ReactFlightServerConfigDebugNode.js @@ -15,6 +15,7 @@ import type { } from './ReactFlightAsyncSequence'; import {IO_NODE, PROMISE_NODE, AWAIT_NODE} from './ReactFlightAsyncSequence'; +import {resolveOwner} from './flight/ReactFlightCurrentOwner'; import {createHook, executionAsyncId} from 'async_hooks'; import {enableAsyncDebugInfo} from 'shared/ReactFeatureFlags'; @@ -46,15 +47,17 @@ export function initAsyncDebugInfo(): void { // so that we can later pick the best stack trace in user space. node = ({ tag: AWAIT_NODE, + owner: resolveOwner(), stack: new Error(), - start: -1.1, - end: -1.1, + start: performance.now(), + end: -1.1, // set when resolved. awaited: trigger, // The thing we're awaiting on. Might get overrriden when we resolve. previous: current === undefined ? null : current, // The path that led us here. }: AwaitNode); } else { node = ({ tag: PROMISE_NODE, + owner: resolveOwner(), stack: new Error(), start: performance.now(), end: -1.1, // Set when we resolve. @@ -74,6 +77,7 @@ export function initAsyncDebugInfo(): void { // We have begun a new I/O sequence. node = ({ tag: IO_NODE, + owner: resolveOwner(), stack: new Error(), // This is only used if no native promises are used. start: performance.now(), end: -1.1, // Only set when pinged. @@ -84,6 +88,7 @@ export function initAsyncDebugInfo(): void { // We have begun a new I/O sequence after the await. node = ({ tag: IO_NODE, + owner: resolveOwner(), stack: new Error(), start: performance.now(), end: -1.1, // Only set when pinged. @@ -113,10 +118,8 @@ export function initAsyncDebugInfo(): void { 'A Promise should never be an IO_NODE. This is a bug in React.', ); } - if (resolvedNode.tag === PROMISE_NODE) { - // Log the end time when we resolved the promise. - resolvedNode.end = performance.now(); - } + // Log the end time when we resolved the promise. + resolvedNode.end = performance.now(); const currentAsyncId = executionAsyncId(); if (asyncId !== currentAsyncId) { // If the promise was not resolved by itself, then that means that diff --git a/packages/react-server/src/ReactFlightStackConfigV8.js b/packages/react-server/src/ReactFlightStackConfigV8.js index 25bc2aba9de8b..4905dfd63281e 100644 --- a/packages/react-server/src/ReactFlightStackConfigV8.js +++ b/packages/react-server/src/ReactFlightStackConfigV8.js @@ -126,10 +126,22 @@ function collectStackTrace( const frameRegExp = /^ {3} at (?:(.+) \((?:(.+):(\d+):(\d+)|\)\)|(?:async )?(.+):(\d+):(\d+)|\)$/; +// DEV-only cache of parsed and filtered stack frames. +const stackTraceCache: WeakMap = __DEV__ + ? new WeakMap() + : (null: any); + export function parseStackTrace( error: Error, skipFrames: number, ): ReactStackTrace { + // We can only get structured data out of error objects once. So we cache the information + // so we can get it again each time. It also helps performance when the same error is + // referenced more than once. + const existing = stackTraceCache.get(error); + if (existing !== undefined) { + return existing; + } // We override Error.prepareStackTrace with our own version that collects // the structured data. We need more information than the raw stack gives us // and we need to ensure that we don't get the source mapped version. @@ -148,6 +160,7 @@ export function parseStackTrace( if (collectedStackTrace !== null) { const result = collectedStackTrace; collectedStackTrace = null; + stackTraceCache.set(error, result); return result; } @@ -191,5 +204,6 @@ export function parseStackTrace( const col = +(parsed[4] || parsed[7]); parsedFrames.push([name, filename, line, col, 0, 0]); } + stackTraceCache.set(error, parsedFrames); return parsedFrames; } diff --git a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js index 32eafab78626a..da2fc898b93b5 100644 --- a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js +++ b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js @@ -39,6 +39,9 @@ function normalizeIOInfo(ioInfo) { if (ioInfo.stack) { copy.stack = normalizeStack(ioInfo.stack); } + if (ioInfo.owner) { + copy.owner = normalizeDebugInfo(ioInfo.owner); + } if (typeof ioInfo.start === 'number') { copy.start = 0; } @@ -160,109 +163,192 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 130, + 133, 109, - 117, + 120, 50, ], ], }, + { + "time": 0, + }, { "awaited": { "end": 0, + "env": "Server", + "name": "delay", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 133, + 109, + 120, + 50, + ], + ], + }, "stack": [ [ "delay", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 112, + 115, 12, - 111, + 114, 3, ], [ "getData", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 119, + 122, 13, - 118, + 121, 5, ], [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 126, + 129, 26, - 125, + 128, 5, ], ], "start": 0, }, + "env": "Server", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 133, + 109, + 120, + 50, + ], + ], + }, "stack": [ [ "getData", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 119, + 122, 13, - 118, + 121, 5, ], [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 126, + 129, 26, - 125, + 128, 5, ], ], }, + { + "time": 0, + }, + { + "time": 0, + }, { "awaited": { "end": 0, + "env": "Server", + "name": "delay", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 133, + 109, + 120, + 50, + ], + ], + }, "stack": [ [ "delay", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 112, + 115, 12, - 111, + 114, 3, ], [ "getData", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 120, + 123, 21, - 118, + 121, 5, ], [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 126, + 129, 20, - 125, + 128, 5, ], ], "start": 0, }, + "env": "Server", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 133, + 109, + 120, + 50, + ], + ], + }, "stack": [ [ "getData", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 121, + 124, 21, - 118, + 121, 5, ], [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 126, + 129, 20, - 125, + 128, 5, ], ], @@ -270,6 +356,9 @@ describe('ReactFlightAsyncDebugInfo', () => { { "time": 0, }, + { + "time": 0, + }, ] `); } @@ -321,28 +410,54 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 291, + 380, 109, - 278, + 367, 67, ], ], }, + { + "time": 0, + }, { "awaited": { "end": 0, + "env": "Server", + "name": "setTimeout", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 380, + 109, + 367, + 67, + ], + ], + }, "stack": [ [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 281, + 370, 7, - 279, + 368, 5, ], ], "start": 0, }, + "env": "Server", + }, + { + "time": 0, }, { "time": 0, diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 793d5dc6e25b0..ea9d3d1d0789a 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -231,8 +231,11 @@ export type ReactErrorInfo = ReactErrorInfoProd | ReactErrorInfoDev; // The point where the Async Info started which might not be the same place it was awaited. export type ReactIOInfo = { + +name: string, // the name of the async function being called (e.g. "fetch") +start: number, // the start time +end: number, // the end time (this might be different from the time the await was unblocked) + +env?: string, // the environment where this I/O was spawned. + +owner?: null | ReactComponentInfo, +stack?: null | ReactStackTrace, // Stashed Data for the Specific Execution Environment. Not part of the transport protocol +debugStack?: null | Error, @@ -241,6 +244,8 @@ export type ReactIOInfo = { export type ReactAsyncInfo = { +awaited: ReactIOInfo, + +env?: string, // the environment where this was awaited. This might not be the same as where it was spawned. + +owner?: null | ReactComponentInfo, +stack?: null | ReactStackTrace, // Stashed Data for the Specific Execution Environment. Not part of the transport protocol +debugStack?: null | Error, @@ -303,20 +308,32 @@ export type SuspenseProps = { export type SuspenseListRevealOrder = | 'forwards' | 'backwards' + | 'unstable_legacy-backwards' | 'together' + | 'independent' | void; -export type SuspenseListTailMode = 'collapsed' | 'hidden' | void; +export type SuspenseListTailMode = 'visible' | 'collapsed' | 'hidden' | void; + +// A SuspenseList row cannot include a nested Array since it's an easy mistake to not realize it +// is treated as a single row. A Fragment can be used to intentionally have multiple children as +// a single row. +type SuspenseListRow = Exclude< + ReactNodeList, + Iterable | AsyncIterable, +>; type DirectionalSuspenseListProps = { - children?: ReactNodeList, - revealOrder: 'forwards' | 'backwards', + // Directional SuspenseList are defined by an array of children or multiple slots to JSX + // It does not allow a single element child. + children?: Iterable | AsyncIterable, // Note: AsyncIterable is experimental. + revealOrder: 'forwards' | 'backwards' | 'unstable_legacy-backwards', tail?: SuspenseListTailMode, }; type NonDirectionalSuspenseListProps = { children?: ReactNodeList, - revealOrder?: 'together' | void, + revealOrder?: 'independent' | 'together' | void, tail?: void, };