From b94603b95504130aec72f61e02d7b66d48f33653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 10:17:53 -0400 Subject: [PATCH 1/2] [Fizz] Gate rel="expect" behind enableFizzBlockingRender (#33183) Enabled in experimental channel. We know this is critical semantics to enforce at the HTML level since if you don't then you can't add explicit boundaries after the fact. However, this might have to go in a major release to allow for upgrading. --- .../src/server/ReactFizzConfigDOM.js | 38 +++++++++------- .../src/__tests__/ReactDOMFizzServer-test.js | 23 ++++++++-- .../ReactDOMFizzServerBrowser-test.js | 22 ++++++++-- .../__tests__/ReactDOMFizzServerEdge-test.js | 12 +++-- .../__tests__/ReactDOMFizzServerNode-test.js | 12 +++-- .../ReactDOMFizzStaticBrowser-test.js | 23 +++++++--- .../__tests__/ReactDOMFizzStaticNode-test.js | 12 +++-- .../src/__tests__/ReactDOMFloat-test.js | 10 ++++- .../src/__tests__/ReactDOMLegacyFloat-test.js | 11 ++++- .../src/__tests__/ReactRenderDocument-test.js | 44 +++++++++++++++---- .../src/__tests__/ReactFlightDOM-test.js | 22 ++++++++-- .../__tests__/ReactFlightDOMBrowser-test.js | 12 ++++- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 19 files changed, 194 insertions(+), 55 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index a54a623bb39..85ec4ae7364 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -34,6 +34,7 @@ import {Children} from 'react'; import { enableFizzExternalRuntime, enableSrcObject, + enableFizzBlockingRender, } from 'shared/ReactFeatureFlags'; import type { @@ -4146,16 +4147,21 @@ export function writeCompletedRoot( // we need to track the paint time of the shell so we know how much to throttle the reveal. writeShellTimeInstruction(destination, resumableState, renderState); } - const preamble = renderState.preamble; - if (preamble.htmlChunks || preamble.headChunks) { - // If we rendered the whole document, then we emitted a rel="expect" that needs a - // matching target. Normally we use one of the bootstrap scripts for this but if - // there are none, then we need to emit a tag to complete the shell. - if ((resumableState.instructions & SentCompletedShellId) === NothingSent) { - writeChunk(destination, startChunkForTag('template')); - writeCompletedShellIdAttribute(destination, resumableState); - writeChunk(destination, endOfStartTag); - writeChunk(destination, endChunkForTag('template')); + if (enableFizzBlockingRender) { + const preamble = renderState.preamble; + if (preamble.htmlChunks || preamble.headChunks) { + // If we rendered the whole document, then we emitted a rel="expect" that needs a + // matching target. Normally we use one of the bootstrap scripts for this but if + // there are none, then we need to emit a tag to complete the shell. + if ( + (resumableState.instructions & SentCompletedShellId) === + NothingSent + ) { + writeChunk(destination, startChunkForTag('template')); + writeCompletedShellIdAttribute(destination, resumableState); + writeChunk(destination, endOfStartTag); + writeChunk(destination, endChunkForTag('template')); + } } } return writeBootstrap(destination, renderState); @@ -5040,11 +5046,13 @@ function writeBlockingRenderInstruction( resumableState: ResumableState, renderState: RenderState, ): void { - const idPrefix = resumableState.idPrefix; - const shellId = '\u00AB' + idPrefix + 'R\u00BB'; - writeChunk(destination, blockingRenderChunkStart); - writeChunk(destination, stringToChunk(escapeTextForBrowser(shellId))); - writeChunk(destination, blockingRenderChunkEnd); + if (enableFizzBlockingRender) { + const idPrefix = resumableState.idPrefix; + const shellId = '\u00AB' + idPrefix + 'R\u00BB'; + writeChunk(destination, blockingRenderChunkStart); + writeChunk(destination, stringToChunk(escapeTextForBrowser(shellId))); + writeChunk(destination, blockingRenderChunkEnd); + } } const completedShellIdAttributeStart = stringToPrecomputedChunk(' id="'); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 1be040bfe36..8bb3e2f4b74 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3590,7 +3590,9 @@ describe('ReactDOMFizzServer', () => { (gate(flags => flags.shouldUseFizzExternalRuntime) ? '' : '') + - '', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); }); @@ -4523,7 +4525,15 @@ describe('ReactDOMFizzServer', () => { // the html should be as-is expect(document.documentElement.innerHTML).toEqual( - '

hello world!

', + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '

hello world!

' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); @@ -6512,7 +6522,14 @@ describe('ReactDOMFizzServer', () => { (gate(flags => flags.shouldUseFizzExternalRuntime) ? '' : '') + - '', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index f5b01d24624..7c0aa14c260 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -84,9 +84,15 @@ describe('ReactDOMFizzServerBrowser', () => { ), ); const result = await readResult(stream); - expect(result).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(result).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(result).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); it('should emit bootstrap script src at the end', async () => { @@ -529,7 +535,15 @@ describe('ReactDOMFizzServerBrowser', () => { const result = await readResult(stream); expect(result).toEqual( - 'foobar', + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + 'foobar' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js index 1eefe1a4082..801baa9678a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js @@ -71,8 +71,14 @@ describe('ReactDOMFizzServerEdge', () => { setTimeout(resolve, 1); }); - expect(result).toMatchInlineSnapshot( - `"
hello
"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(result).toMatchInlineSnapshot( + `"
hello
"`, + ); + } else { + expect(result).toMatchInlineSnapshot( + `"
hello
"`, + ); + } }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 2704c243eba..d4f3486a1db 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -78,9 +78,15 @@ describe('ReactDOMFizzServerNode', () => { pipe(writable); }); // with Float, we emit empty heads if they are elided when rendering - expect(output.result).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(output.result).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(output.result).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); it('should emit bootstrap script src at the end', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 578b2bf916f..55a89bc525a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -195,9 +195,15 @@ describe('ReactDOMFizzStaticBrowser', () => { ), ); const prelude = await readContent(result.prelude); - expect(prelude).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); it('should emit bootstrap script src at the end', async () => { @@ -1438,8 +1444,15 @@ describe('ReactDOMFizzStaticBrowser', () => { expect(await readContent(content)).toBe( '' + '' + - '' + - 'Hello', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + 'Hello' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js index d9bd7c70db0..3aa7a70cb80 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js @@ -63,9 +63,15 @@ describe('ReactDOMFizzStaticNode', () => { , ); const prelude = await readContent(result.prelude); - expect(prelude).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); // @gate experimental diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index b7f42bcf8db..2d3f1f0b8b6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -704,8 +704,14 @@ describe('ReactDOMFloat', () => { (gate(flags => flags.shouldUseFizzExternalRuntime) ? '' : '') + - 'foo' + - 'bar', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + 'foo' + + 'bar' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), '', ]); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js index f2cabafc9f5..f6d868c8413 100644 --- a/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js @@ -34,8 +34,15 @@ describe('ReactDOMFloat', () => { ); expect(result).toEqual( - '' + - 'title', + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + 'title' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); }); diff --git a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js index 2b54bc90090..3501f1bd92a 100644 --- a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js @@ -78,14 +78,20 @@ describe('rendering React components at document', () => { root = ReactDOMClient.hydrateRoot(testDocument, ); }); expect(testDocument.body.innerHTML).toBe( - 'Hello world' + '', + 'Hello world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); await act(() => { root.render(); }); expect(testDocument.body.innerHTML).toBe( - 'Hello moon' + '', + 'Hello moon' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); expect(body === testDocument.body).toBe(true); @@ -112,7 +118,10 @@ describe('rendering React components at document', () => { root = ReactDOMClient.hydrateRoot(testDocument, ); }); expect(testDocument.body.innerHTML).toBe( - 'Hello world' + '', + 'Hello world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); const originalDocEl = testDocument.documentElement; @@ -124,9 +133,15 @@ describe('rendering React components at document', () => { expect(testDocument.firstChild).toBe(originalDocEl); expect(testDocument.head).toBe(originalHead); expect(testDocument.body).toBe(originalBody); - expect(originalBody.innerHTML).toBe(''); + expect(originalBody.innerHTML).toBe( + gate(flags => flags.enableFizzBlockingRender) + ? '' + : '', + ); expect(originalHead.innerHTML).toBe( - '', + gate(flags => flags.enableFizzBlockingRender) + ? '' + : '', ); }); @@ -166,7 +181,10 @@ describe('rendering React components at document', () => { }); expect(testDocument.body.innerHTML).toBe( - 'Hello world' + '', + 'Hello world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); await act(() => { @@ -174,7 +192,9 @@ describe('rendering React components at document', () => { }); expect(testDocument.body.innerHTML).toBe( - '' + 'Goodbye world', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + 'Goodbye world', ); }); @@ -205,7 +225,10 @@ describe('rendering React components at document', () => { }); expect(testDocument.body.innerHTML).toBe( - 'Hello world' + '', + 'Hello world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); }); @@ -341,7 +364,10 @@ describe('rendering React components at document', () => { expect(testDocument.body.innerHTML).toBe( favorSafetyOverHydrationPerf ? 'Hello world' - : 'Goodbye world', + : 'Goodbye world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 80562624eb1..05a6a227c2b 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -1921,14 +1921,28 @@ describe('ReactFlightDOM', () => { expect(content1).toEqual( '' + '' + - '' + - '

hello world

', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + '

hello world

' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); expect(content2).toEqual( '' + '' + - '' + - '

hello world

', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + '

hello world

' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); 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 4313c379b70..9d668ea3c3e 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -1899,8 +1899,16 @@ describe('ReactFlightDOMBrowser', () => { } expect(content).toEqual( - '' + - '

hello world

', + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + '

hello world

' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 13c1121c95e..216b6d668a9 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -98,6 +98,8 @@ export const enableScrollEndPolyfill = __EXPERIMENTAL__; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = __EXPERIMENTAL__; // rel="expect" + export const enableSrcObject = __EXPERIMENTAL__; export const enableHydrationChangeEvent = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index d885f9f7164..4298a267a36 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -83,6 +83,7 @@ export const enableViewTransition = false; export const enableGestureTransition = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = true; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 301552857f8..8a7a59ebb26 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -73,6 +73,7 @@ export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 9d8a1808d99..6de2578838f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -73,6 +73,7 @@ export const enableFastAddPropertiesInDiffing = true; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 4a9fb9737ac..643626d6f97 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -70,6 +70,7 @@ export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 28587994948..d22d5cadd74 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -84,6 +84,7 @@ export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 4869d77741d..839248e2e7b 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -114,6 +114,7 @@ export const enableLazyPublicInstanceInFabric = false; export const enableGestureTransition = false; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; From 997c7bc930304142b3af37bcb21599181124aeb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 12:39:10 -0400 Subject: [PATCH 2/2] [DevTools] Get source location from structured callsites in prepareStackTrace (#33143) When we get the source location for "View source for this element" we should be using the enclosing function of the callsite of the child. So that we don't just point to some random line within the component. This is similar to the technique in #33136. This technique is now really better than the fake throw technique, when available. So I now favor the owner technique. The only problem it's only available in DEV and only if it has a child that's owned (and not filtered). We could implement this same technique for the error that's thrown in the fake throwing solution. However, we really shouldn't need that at all because for client components we should be able to call `inspect(fn)` at least in Chrome which is even better. --- .../src/backend/fiber/renderer.js | 26 +++---- .../src/backend/shared/DevToolsOwnerStack.js | 6 +- .../src/backend/utils/index.js | 73 +++++++++++++++++++ 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 711d76c92d0..4fc59a24d92 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -50,6 +50,7 @@ import { gt, gte, parseSourceFromComponentStack, + parseSourceFromOwnerStack, serializeToString, } from 'react-devtools-shared/src/backend/utils'; import { @@ -5805,15 +5806,13 @@ export function attach( function getSourceForFiberInstance( fiberInstance: FiberInstance, ): Source | null { - const unresolvedSource = fiberInstance.source; - if ( - unresolvedSource !== null && - typeof unresolvedSource === 'object' && - !isError(unresolvedSource) - ) { - // $FlowFixMe: isError should have refined it. - return unresolvedSource; + // Favor the owner source if we have one. + const ownerSource = getSourceForInstance(fiberInstance); + if (ownerSource !== null) { + return ownerSource; } + + // Otherwise fallback to the throwing trick. const dispatcherRef = getDispatcherRef(renderer); const stackFrame = dispatcherRef == null @@ -5824,10 +5823,7 @@ export function attach( dispatcherRef, ); if (stackFrame === null) { - // If we don't find a source location by throwing, try to get one - // from an owned child if possible. This is the same branch as - // for virtual instances. - return getSourceForInstance(fiberInstance); + return null; } const source = parseSourceFromComponentStack(stackFrame); fiberInstance.source = source; @@ -5835,7 +5831,7 @@ export function attach( } function getSourceForInstance(instance: DevToolsInstance): Source | null { - let unresolvedSource = instance.source; + const unresolvedSource = instance.source; if (unresolvedSource === null) { // We don't have any source yet. We can try again later in case an owned child mounts later. // TODO: We won't have any information here if the child is filtered. @@ -5848,7 +5844,9 @@ export function attach( // any intermediate utility functions. This won't point to the top of the component function // but it's at least somewhere within it. if (isError(unresolvedSource)) { - unresolvedSource = formatOwnerStack((unresolvedSource: any)); + return (instance.source = parseSourceFromOwnerStack( + (unresolvedSource: any), + )); } if (typeof unresolvedSource === 'string') { const idx = unresolvedSource.lastIndexOf('\n'); diff --git a/packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js b/packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js index e03d948a45d..7d4cfa65ce0 100644 --- a/packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js +++ b/packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js @@ -13,8 +13,12 @@ export function formatOwnerStack(error: Error): string { const prevPrepareStackTrace = Error.prepareStackTrace; // $FlowFixMe[incompatible-type] It does accept undefined. Error.prepareStackTrace = undefined; - let stack = error.stack; + const stack = error.stack; Error.prepareStackTrace = prevPrepareStackTrace; + return formatOwnerStackString(stack); +} + +export function formatOwnerStackString(stack: string): string { if (stack.startsWith('Error: react-stack-top-frame\n')) { // V8's default formatting prefixes with the error message which we // don't want/need. diff --git a/packages/react-devtools-shared/src/backend/utils/index.js b/packages/react-devtools-shared/src/backend/utils/index.js index 977683ef9a2..950161a020b 100644 --- a/packages/react-devtools-shared/src/backend/utils/index.js +++ b/packages/react-devtools-shared/src/backend/utils/index.js @@ -18,6 +18,8 @@ import type {DehydratedData} from 'react-devtools-shared/src/frontend/types'; export {default as formatWithStyles} from './formatWithStyles'; export {default as formatConsoleArguments} from './formatConsoleArguments'; +import {formatOwnerStackString} from '../shared/DevToolsOwnerStack'; + // TODO: update this to the first React version that has a corresponding DevTools backend const FIRST_DEVTOOLS_BACKEND_LOCKSTEP_VER = '999.9.9'; export function hasAssignedBackend(version?: string): boolean { @@ -345,6 +347,77 @@ export function parseSourceFromComponentStack( return parseSourceFromFirefoxStack(componentStack); } +let collectedLocation: Source | null = null; + +function collectStackTrace( + error: Error, + structuredStackTrace: CallSite[], +): string { + let result: null | Source = null; + // Collect structured stack traces from the callsites. + // We mirror how V8 serializes stack frames and how we later parse them. + for (let i = 0; i < structuredStackTrace.length; i++) { + const callSite = structuredStackTrace[i]; + if (callSite.getFunctionName() === 'react-stack-bottom-frame') { + // We pick the last frame that matches before the bottom frame since + // that will be immediately inside the component as opposed to some helper. + // If we don't find a bottom frame then we bail to string parsing. + collectedLocation = result; + // Skip everything after the bottom frame since it'll be internals. + break; + } else { + const sourceURL = callSite.getScriptNameOrSourceURL(); + const line = + // $FlowFixMe[prop-missing] + typeof callSite.getEnclosingLineNumber === 'function' + ? (callSite: any).getEnclosingLineNumber() + : callSite.getLineNumber(); + const col = + // $FlowFixMe[prop-missing] + typeof callSite.getEnclosingColumnNumber === 'function' + ? (callSite: any).getEnclosingColumnNumber() + : callSite.getLineNumber(); + if (!sourceURL || !line || !col) { + // Skip eval etc. without source url. They don't have location. + continue; + } + result = { + sourceURL, + line: line, + column: col, + }; + } + } + // At the same time we generate a string stack trace just in case someone + // else reads it. + const name = error.name || 'Error'; + const message = error.message || ''; + let stack = name + ': ' + message; + for (let i = 0; i < structuredStackTrace.length; i++) { + stack += '\n at ' + structuredStackTrace[i].toString(); + } + return stack; +} + +export function parseSourceFromOwnerStack(error: Error): Source | null { + // First attempt to collected the structured data using prepareStackTrace. + collectedLocation = null; + const previousPrepare = Error.prepareStackTrace; + Error.prepareStackTrace = collectStackTrace; + let stack; + try { + stack = error.stack; + } finally { + Error.prepareStackTrace = previousPrepare; + } + if (collectedLocation !== null) { + return collectedLocation; + } + // Fallback to parsing the string form. + const componentStack = formatOwnerStackString(stack); + return parseSourceFromComponentStack(componentStack); +} + // 0.123456789 => 0.123 // Expects high-resolution timestamp in milliseconds, like from performance.now() // Mainly used for optimizing the size of serialized profiling payload