From eaeee64ef1885f8fb23add4990652878c1c36d51 Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Tue, 18 Feb 2025 12:47:48 +0100 Subject: [PATCH] [dev-overlay] Fix off-by-one column sourcemapping in Webpack --- .../parseNotFoundError.ts | 3 +- .../server/middleware-turbopack.ts | 6 +- .../server/middleware-webpack.ts | 17 +- .../capture-console-error-owner-stack.test.ts | 96 +++--------- .../error-ignored-frames.test.ts | 44 +++--- .../owner-stack-invalid-element-type.test.ts | 147 ++++++------------ ...owner-stack-react-missing-key-prop.test.ts | 24 +-- .../app-dir/owner-stack/owner-stack.test.ts | 27 ++-- .../server-navigation-error.test.ts | 18 +-- ...use-cache-standalone-search-params.test.ts | 4 +- 10 files changed, 138 insertions(+), 248 deletions(-) diff --git a/packages/next/src/build/webpack/plugins/wellknown-errors-plugin/parseNotFoundError.ts b/packages/next/src/build/webpack/plugins/wellknown-errors-plugin/parseNotFoundError.ts index 508b370673b57..89ad209ce3e43 100644 --- a/packages/next/src/build/webpack/plugins/wellknown-errors-plugin/parseNotFoundError.ts +++ b/packages/next/src/build/webpack/plugins/wellknown-errors-plugin/parseNotFoundError.ts @@ -76,7 +76,8 @@ async function getSourceFrame( file: fileName, methodName: '', lineNumber: loc.start.line, - column: loc.start.column, + // loc is 0-based but columns in stack frames are 1-based. + column: (loc.start.column ?? 0) + 1, }, }) diff --git a/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts b/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts index 1b22faec75fa7..788c39b5e7c88 100644 --- a/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts +++ b/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts @@ -232,6 +232,9 @@ function findApplicableSourceMapPayload( } } +/** + * @returns 1-based lines and 0-based columns + */ async function nativeTraceSource( frame: TurbopackStackFrame ): Promise<{ frame: IgnorableStackFrame; source: string | null } | undefined> { @@ -264,7 +267,8 @@ async function nativeTraceSource( try { const originalPosition = consumer.originalPositionFor({ line: frame.line ?? 1, - column: frame.column ?? 1, + // 0-based columns out requires 0-based columns in. + column: (frame.column ?? 1) - 1, }) if (originalPosition.source === null) { diff --git a/packages/next/src/client/components/react-dev-overlay/server/middleware-webpack.ts b/packages/next/src/client/components/react-dev-overlay/server/middleware-webpack.ts index 08639644a64f4..2b055a065fcd9 100644 --- a/packages/next/src/client/components/react-dev-overlay/server/middleware-webpack.ts +++ b/packages/next/src/client/components/react-dev-overlay/server/middleware-webpack.ts @@ -90,9 +90,12 @@ function getSourcePath(source: string) { return source.replace(/^(webpack:\/\/\/|webpack:\/\/|webpack:\/\/_N_E\/)/, '') } +/** + * @returns 1-based lines and 0-based columns + */ async function findOriginalSourcePositionAndContent( sourceMap: RawSourceMap, - position: { line: number; column: number | null } + position: { lineNumber: number | null; column: number | null } ): Promise { let consumer: BasicSourceMapConsumer try { @@ -106,8 +109,9 @@ async function findOriginalSourcePositionAndContent( try { const sourcePosition = consumer.originalPositionFor({ - line: position.line, - column: position.column ?? 0, + line: position.lineNumber ?? 1, + // 0-based columns out requires 0-based columns in. + column: (position.column ?? 1) - 1, }) if (!sourcePosition.source) { @@ -191,7 +195,6 @@ export async function createOriginalStackFrame({ frame: StackFrame errorMessage?: string }): Promise { - const { lineNumber, column } = frame const moduleNotFound = findModuleNotFoundFromError(errorMessage) const result = await (() => { if (moduleNotFound) { @@ -205,11 +208,7 @@ export async function createOriginalStackFrame({ source.compilation ) } - // This returns 1-based lines and 0-based columns - return findOriginalSourcePositionAndContent(source.sourceMap, { - line: lineNumber ?? 1, - column, - }) + return findOriginalSourcePositionAndContent(source.sourceMap, frame) })() if (!result) { diff --git a/test/development/app-dir/capture-console-error-owner-stack/capture-console-error-owner-stack.test.ts b/test/development/app-dir/capture-console-error-owner-stack/capture-console-error-owner-stack.test.ts index 94c1db4368eef..8c90e308f9dd8 100644 --- a/test/development/app-dir/capture-console-error-owner-stack/capture-console-error-owner-stack.test.ts +++ b/test/development/app-dir/capture-console-error-owner-stack/capture-console-error-owner-stack.test.ts @@ -41,54 +41,28 @@ describe('app-dir - capture-console-error-owner-stack', () => { const result = await getRedboxResult(browser) - // TODO(veil): Inconsistent cursor position for the "Page" frame - if (process.env.TURBOPACK) { - expect(result).toMatchInlineSnapshot(` - { - "callStacks": "onClick - app/browser/event/page.js (7:17) - button - (0:0) - Page - app/browser/event/page.js (5:5)", - "count": 1, - "description": "trigger an console ", - "source": "app/browser/event/page.js (7:17) @ onClick - - 5 |