From e0ec273b93029450dd5e12e663d164f46465b00b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 6 May 2025 21:20:46 -0400 Subject: [PATCH 1/3] Encode enclosing line/column numbers in ReactCallSite If we don't have the ability to get enclosing line/column we encode it as zero. This will point to the beginning of the file which is likely to cause a miss in source mapping to an identifier which is better since it falls back to getting the name of the function in that case instead. --- .../react-client/src/ReactFlightReplyClient.js | 11 ++++++----- .../react-server/src/ReactFlightStackConfigV8.js | 16 +++++++++++++--- packages/shared/ReactTypes.js | 2 ++ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index e3c4ec20ba1ab..28efab6799ec1 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -1279,7 +1279,7 @@ export function createBoundServerReference, T>( const location = metaData.location; if (location) { const functionName = metaData.name || ''; - const [, filename, line, col] = location; + const [, filename, , , enclosingLine, enclosingCol] = location; const env = metaData.env || 'Server'; const sourceMap = findSourceMapURL == null ? null : findSourceMapURL(filename, env); @@ -1287,8 +1287,8 @@ export function createBoundServerReference, T>( functionName, filename, sourceMap, - line, - col, + enclosingLine, + enclosingCol, env, action, ); @@ -1350,10 +1350,11 @@ function parseStackLocation(error: Error): null | ReactCallSite { if (filename === '') { filename = ''; } + // This is really the enclosingLine/Column. const line = +(parsed[3] || parsed[6]); const col = +(parsed[4] || parsed[7]); - return [name, filename, line, col]; + return [name, filename, line, col, line, col]; } export function createServerReference, T>( @@ -1374,7 +1375,7 @@ export function createServerReference, T>( // multiple passes of compilation as long as we can find the final source map. const location = parseStackLocation(new Error('react-stack-top-frame')); if (location !== null) { - const [, filename, line, col] = location; + const [, filename, , , line, col] = location; // While the environment that the Server Reference points to can be // in any environment, what matters here is where the compiled source // is from and that's in the currently executing environment. We hard diff --git a/packages/react-server/src/ReactFlightStackConfigV8.js b/packages/react-server/src/ReactFlightStackConfigV8.js index d2f4633cec67a..25bc2aba9de8b 100644 --- a/packages/react-server/src/ReactFlightStackConfigV8.js +++ b/packages/react-server/src/ReactFlightStackConfigV8.js @@ -63,7 +63,7 @@ function collectStackTrace( // Skip everything after the bottom frame since it'll be internals. break; } else if (callSite.isNative()) { - result.push([name, '', 0, 0]); + result.push([name, '', 0, 0, 0, 0]); } else { // We encode complex function calls as if they're part of the function // name since we cannot simulate the complex ones and they look the same @@ -88,7 +88,17 @@ function collectStackTrace( } const line = callSite.getLineNumber() || 0; const col = callSite.getColumnNumber() || 0; - result.push([name, filename, line, col]); + const enclosingLine: number = + // $FlowFixMe[prop-missing] + typeof callSite.getEnclosingLineNumber === 'function' + ? (callSite: any).getEnclosingLineNumber() || 0 + : 0; + const enclosingCol: number = + // $FlowFixMe[prop-missing] + typeof callSite.getEnclosingColumnNumber === 'function' + ? (callSite: any).getEnclosingColumnNumber() || 0 + : 0; + result.push([name, filename, line, col, enclosingLine, enclosingCol]); } } // At the same time we generate a string stack trace just in case someone @@ -179,7 +189,7 @@ export function parseStackTrace( } const line = +(parsed[3] || parsed[6]); const col = +(parsed[4] || parsed[7]); - parsedFrames.push([name, filename, line, col]); + parsedFrames.push([name, filename, line, col, 0, 0]); } return parsedFrames; } diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index e199fa9e7b0b7..2172c92bfc90a 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -186,6 +186,8 @@ export type ReactCallSite = [ string, // file name TODO: model nested eval locations as nested arrays number, // line number number, // column number + number, // enclosing line number + number, // enclosing column number ]; export type ReactStackTrace = Array; From 9260310e352abcdadaac389c803ed0c69880b4c0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 6 May 2025 22:46:13 -0400 Subject: [PATCH 2/3] Encode the start of the fake function at the enclosing line/col --- .../react-client/src/ReactFlightClient.js | 104 ++++++++++++++++-- 1 file changed, 93 insertions(+), 11 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 3234814952fad..fbf195ba7b65a 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -2226,6 +2226,8 @@ function createFakeFunction( sourceMap: null | string, line: number, col: number, + enclosingLine: number, + enclosingCol: number, environmentName: string, ): FakeFunction { // This creates a fake copy of a Server Module. It represents a module that has already @@ -2243,26 +2245,104 @@ function createFakeFunction( // This allows us to use the original source map as the source map of this fake file to // point to the original source. let code; - if (line <= 1) { - const minSize = encodedName.length + 7; + // Normalize line/col to zero based. + if (enclosingLine < 1) { + enclosingLine = 0; + } else { + enclosingLine--; + } + if (enclosingCol < 1) { + enclosingCol = 0; + } else { + enclosingCol--; + } + if (line < 1) { + line = 0; + } else { + line--; + } + if (col < 1) { + col = 0; + } else { + col--; + } + if (line < enclosingLine || (line === enclosingLine && col < enclosingCol)) { + // Protection against invalid enclosing information. Should not happen. + enclosingLine = 0; + enclosingCol = 0; + } + if (line < 1) { + // Fit everything on the first line. + const minCol = encodedName.length + 3; + let enclosingColDistance = enclosingCol - minCol; + if (enclosingColDistance < 0) { + enclosingColDistance = 0; + } + let colDistance = col - enclosingColDistance - minCol - 3; + if (colDistance < 0) { + colDistance = 0; + } code = '({' + encodedName + - ':_=>' + - ' '.repeat(col < minSize ? 0 : col - minSize) + - '_()})\n' + - comment; + ':' + + ' '.repeat(enclosingColDistance) + + '_=>' + + ' '.repeat(colDistance) + + '_()})'; + } else if (enclosingLine < 1) { + // Fit just the enclosing function on the first line. + const minCol = encodedName.length + 3; + let enclosingColDistance = enclosingCol - minCol; + if (enclosingColDistance < 0) { + enclosingColDistance = 0; + } + code = + '({' + + encodedName + + ':' + + ' '.repeat(enclosingColDistance) + + '_=>' + + '\n'.repeat(line - enclosingLine) + + ' '.repeat(col) + + '_()})'; + } else if (enclosingLine === line) { + // Fit the enclosing function and callsite on same line. + let colDistance = col - enclosingCol - 3; + if (colDistance < 0) { + colDistance = 0; + } + code = + '\n'.repeat(enclosingLine - 1) + + '({' + + encodedName + + ':\n' + + ' '.repeat(enclosingCol) + + '_=>' + + ' '.repeat(colDistance) + + '_()})'; } else { + // This is the ideal because we can always encode any position. code = - comment + - '\n'.repeat(line - 2) + + '\n'.repeat(enclosingLine - 1) + '({' + encodedName + - ':_=>\n' + - ' '.repeat(col < 1 ? 0 : col - 1) + + ':\n' + + ' '.repeat(enclosingCol) + + '_=>' + + '\n'.repeat(line - enclosingLine) + + ' '.repeat(col) + '_()})'; } + if (enclosingLine < 1) { + // If the function starts at the first line, we append the comment after. + code = code + '\n' + comment; + } else { + // Otherwise we prepend the comment on the first line. + code = comment + code; + } + if (filename.startsWith('/')) { // If the filename starts with `/` we assume that it is a file system file // rather than relative to the current host. Since on the server fully qualified @@ -2320,7 +2400,7 @@ function buildFakeCallStack( const frameKey = frame.join('-') + '-' + environmentName; let fn = fakeFunctionCache.get(frameKey); if (fn === undefined) { - const [name, filename, line, col] = frame; + const [name, filename, line, col, enclosingLine, enclosingCol] = frame; const findSourceMapURL = response._debugFindSourceMapURL; const sourceMap = findSourceMapURL ? findSourceMapURL(filename, environmentName) @@ -2331,6 +2411,8 @@ function buildFakeCallStack( sourceMap, line, col, + enclosingLine, + enclosingCol, environmentName, ); // TODO: This cache should technically live on the response since the _debugFindSourceMapURL From bcb36b890d7021fc3eba0e1f9f7ad9f392483855 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 7 May 2025 11:54:50 -0400 Subject: [PATCH 3/3] Use line/col of the createServerReference callsite --- packages/react-client/src/ReactFlightReplyClient.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index 28efab6799ec1..cf21d6e9adf13 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -1279,7 +1279,7 @@ export function createBoundServerReference, T>( const location = metaData.location; if (location) { const functionName = metaData.name || ''; - const [, filename, , , enclosingLine, enclosingCol] = location; + const [, filename, line, col] = location; const env = metaData.env || 'Server'; const sourceMap = findSourceMapURL == null ? null : findSourceMapURL(filename, env); @@ -1287,8 +1287,8 @@ export function createBoundServerReference, T>( functionName, filename, sourceMap, - enclosingLine, - enclosingCol, + line, + col, env, action, ); @@ -1375,7 +1375,7 @@ export function createServerReference, T>( // multiple passes of compilation as long as we can find the final source map. const location = parseStackLocation(new Error('react-stack-top-frame')); if (location !== null) { - const [, filename, , , line, col] = location; + const [, filename, line, col] = location; // While the environment that the Server Reference points to can be // in any environment, what matters here is where the compiled source // is from and that's in the currently executing environment. We hard