Turbopack: convert between locations correctly (#61477)

- parsed stack traces (and error stack locations in js) have 1-based lines and 1-based columns - source map tokens have 0-based lines and 0-based columns - babel code frames use 1-based lines and 0-based columns This was not always respected. This preserves the 1-based lines and columns in anything called a stack frame, 0-based lines and columns for source map apis, and converts to babel’s format as needed. Closes PACK-2341 --------- Co-authored-by: Tobias Koppers <[email protected]>
vercel · Feb 6, 2024 · b19585f · b19585f
1 parent daf8912
commit b19585f
Show file tree

Hide file tree

Showing 21 changed files with 127 additions and 116 deletions.
diff --git a/packages/next-swc/crates/napi/src/next_api/project.rs b/packages/next-swc/crates/napi/src/next_api/project.rs
@@ -791,7 +791,9 @@ pub fn project_update_info_subscribe(
 #[derive(Debug)]
 #[napi(object)]
 pub struct StackFrame {
+    // 1-indexed, unlike source map tokens
     pub column: Option<u32>,
+    // 1-indexed, unlike source map tokens
     pub file: String,
     pub is_server: bool,
     pub line: u32,
@@ -876,7 +878,10 @@ pub async fn project_trace_source(
             };
 
             let token = map
-                .lookup_token(frame.line as usize, frame.column.unwrap_or(0) as usize)
+                .lookup_token(
+                    (frame.line as usize).saturating_sub(1),
+                    (frame.column.unwrap_or(1) as usize).saturating_sub(1),
+                )
                 .await?
                 .clone_value()
                 .context("Unable to trace token from sourcemap")?;
@@ -893,8 +898,8 @@ pub async fn project_trace_source(
             Ok(Some(StackFrame {
                 file: source_file.to_string(),
                 method_name: token.name,
-                line: token.original_line as u32,
-                column: Some(token.original_column as u32),
+                line: token.original_line as u32 + 1,
+                column: Some(token.original_column as u32 + 1),
                 is_server: frame.is_server,
             }))
         })

diff --git a/packages/next/src/build/swc/index.ts b/packages/next/src/build/swc/index.ts
@@ -507,8 +507,18 @@ export interface Issue {
       content?: string
     }
     range?: {
-      start: { line: number; column: number }
-      end: { line: number; column: number }
+      start: {
+        // 0-indexed
+        line: number
+        // 0-indexed
+        column: number
+      }
+      end: {
+        // 0-indexed
+        line: number
+        // 0-indexed
+        column: number
+      }
     }
   }
   documentationLink: string

diff --git a/packages/next/src/server/lib/router-utils/setup-dev-bundler.ts b/packages/next/src/server/lib/router-utils/setup-dev-bundler.ts
@@ -339,7 +339,7 @@ async function startWatcher(opts: SetupOpts) {
       if (source && source.range) {
         const { start } = source.range
         message = `${formattedFilePath}:${start.line + 1}:${
-          start.column
+          start.column + 1
         }\n${formattedTitle}`
       } else if (formattedFilePath) {
         message = `${formattedFilePath}\n${formattedTitle}`

diff --git a/packages/react-dev-overlay/src/middleware-turbopack.ts b/packages/react-dev-overlay/src/middleware-turbopack.ts
@@ -16,7 +16,9 @@ interface Project {
 }
 
 interface TurbopackStackFrame {
+  // 1-based
   column: number | null
+  // 1-based
   file: string
   isServer: boolean
   line: number
@@ -87,7 +89,7 @@ export async function createOriginalStackFrame(
             {
               start: {
                 line: traced.frame.lineNumber,
-                column: traced.frame.column ?? 0,
+                column: traced.frame.column ?? 1,
               },
             },
             { forceColor: true }

diff --git a/packages/react-dev-overlay/src/middleware.ts b/packages/react-dev-overlay/src/middleware.ts
@@ -194,6 +194,7 @@ export async function createOriginalStackFrame({
 
       return moduleNotFoundResult
     }
+    // This returns 1-based lines and 0-based columns
     return await findOriginalSourcePositionAndContent(source, {
       line,
       column,
@@ -225,7 +226,7 @@ export async function createOriginalStackFrame({
       ? path.relative(rootDirectory, filePath)
       : sourcePosition.source,
     lineNumber: sourcePosition.line,
-    column: sourcePosition.column,
+    column: (sourcePosition.column ?? 0) + 1,
     methodName:
       sourcePosition.name ||
       // default is not a valid identifier in JS so webpack uses a custom variable when it's an unnamed default export
@@ -245,7 +246,7 @@ export async function createOriginalStackFrame({
           {
             start: {
               line: sourcePosition.line,
-              column: sourcePosition.column ?? 0,
+              column: (sourcePosition.column ?? 0) + 1,
             },
           },
           { forceColor: true }

diff --git a/test/development/acceptance-app/ReactRefreshLogBox-builtins.test.ts b/test/development/acceptance-app/ReactRefreshLogBox-builtins.test.ts
@@ -52,7 +52,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
     expect(await session.hasRedbox()).toBe(true)
     if (process.env.TURBOPACK) {
       expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
-        "./node_modules/my-package/index.js:1:12
+        "./node_modules/my-package/index.js:1:13
         Module not found: Can't resolve 'dns'
         > 1 | const dns = require('dns')
             |             ^^^^^^^^^^^^^^
@@ -62,7 +62,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
       `)
     } else {
       expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
-        "./node_modules/my-package/index.js:1:12
+        "./node_modules/my-package/index.js:1:13
         Module not found: Can't resolve 'dns'
 
         https://nextjs.org/docs/messages/module-not-found
@@ -98,7 +98,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
     const source = await session.getRedboxSource()
     if (process.env.TURBOPACK) {
       expect(source).toMatchInlineSnapshot(`
-        "./index.js:1:0
+        "./index.js:1:1
         Module not found: Can't resolve 'b'
         > 1 | import Comp from 'b'
             | ^^^^^^^^^^^^^^^^^^^^
@@ -110,9 +110,10 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
       `)
     } else {
       expect(source).toMatchInlineSnapshot(`
-        "./index.js:1:0
+        "./index.js:1:1
         Module not found: Can't resolve 'b'
         > 1 | import Comp from 'b'
+            | ^
           2 | export default function Oops() {
           3 |   return (
           4 |     <div>
@@ -150,7 +151,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
     const source = await session.getRedboxSource()
     if (process.env.TURBOPACK) {
       expect(source).toMatchInlineSnapshot(`
-        "./app/page.js:2:0
+        "./app/page.js:2:1
         Module not found: Can't resolve 'b'
           1 | 'use client'
         > 2 | import Comp from 'b'
@@ -163,10 +164,11 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
       `)
     } else {
       expect(source).toMatchInlineSnapshot(`
-        "./app/page.js:2:0
+        "./app/page.js:2:1
         Module not found: Can't resolve 'b'
           1 | 'use client'
         > 2 | import Comp from 'b'
+            | ^
           3 | export default function Oops() {
           4 |   return (
           5 |     <div>
@@ -199,7 +201,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
     const source = await session.getRedboxSource()
     if (process.env.TURBOPACK) {
       expect(source).toMatchInlineSnapshot(`
-        "./app/page.js:2:0
+        "./app/page.js:2:1
         Module not found: Can't resolve './non-existent.css'
           1 | 'use client'
         > 2 | import './non-existent.css'
@@ -212,10 +214,11 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
       `)
     } else {
       expect(source).toMatchInlineSnapshot(`
-        "./app/page.js:2:0
+        "./app/page.js:2:1
         Module not found: Can't resolve './non-existent.css'
           1 | 'use client'
         > 2 | import './non-existent.css'
+            | ^
           3 | export default function Page(props) {
           4 |   return <p>index page</p>
           5 | }

diff --git a/test/development/acceptance-app/ReactRefreshLogBox.test.ts b/test/development/acceptance-app/ReactRefreshLogBox.test.ts
@@ -222,7 +222,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
     const source = next.normalizeTestDirContent(await session.getRedboxSource())
     if (IS_TURBOPACK) {
       expect(source).toMatchInlineSnapshot(`
-        "./index.js:7:0
+        "./index.js:7:1
         Parsing ecmascript source code failed
           5 |     div
           6 |   )
@@ -357,7 +357,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
     expect(await session.hasRedbox()).toBe(true)
     const source = await session.getRedboxSource()
     expect(source).toMatch(
-      IS_TURBOPACK ? './index.module.css:1:8' : './index.module.css:1:1'
+      IS_TURBOPACK ? './index.module.css:1:9' : './index.module.css:1:1'
     )
     if (!IS_TURBOPACK) {
       expect(source).toMatch('Syntax error: ')

diff --git a/test/development/acceptance-app/ReactRefreshRegression.test.ts b/test/development/acceptance-app/ReactRefreshRegression.test.ts
@@ -286,7 +286,7 @@ describe('ReactRefreshRegression app', () => {
     const source = await session.getRedboxSource()
     expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(`
       "> 1 | export default function () { throw new Error('boom'); }
-          |                                   ^"
+          |                                    ^"
     `)
 
     await cleanup()
@@ -305,7 +305,7 @@ describe('ReactRefreshRegression app', () => {
     const source = await session.getRedboxSource()
     expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(`
       "> 1 | export default function Page() { throw new Error('boom'); }
-          |                                       ^"
+          |                                        ^"
     `)
 
     await cleanup()
@@ -326,10 +326,10 @@ describe('ReactRefreshRegression app', () => {
 
     const source = await session.getRedboxSource()
     expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(`
-      "  1 | 'use client'
-      > 2 | export default function Page() { throw new Error('boom'); }
-          |                                       ^"
-    `)
+        "  1 | 'use client'
+        > 2 | export default function Page() { throw new Error('boom'); }
+            |                                        ^"
+      `)
 
     await cleanup()
   })

diff --git a/test/development/acceptance-app/__snapshots__/ReactRefreshLogBox.test.ts.snap b/test/development/acceptance-app/__snapshots__/ReactRefreshLogBox.test.ts.snap
@@ -11,9 +11,10 @@ Import trace for requested module:
 `;
 
 exports[`ReactRefreshLogBox app default Import trace when module not found in layout 1`] = `
-"./app/module.js:1:0
+"./app/module.js:1:1
 Module not found: Can't resolve 'non-existing-module'
 > 1 | import "non-existing-module"
+    | ^
 
 https://nextjs.org/docs/messages/module-not-found
 
@@ -22,31 +23,31 @@ Import trace for requested module:
 `;
 
 exports[`ReactRefreshLogBox app default Should not show __webpack_exports__ when exporting anonymous arrow function 1`] = `
-"index.js (3:10) @ default
+"index.js (3:11) @ default
 
   1 | export default () => {
   2 |   if (typeof window !== 'undefined') {
 > 3 |     throw new Error('test')
-    |          ^
+    |           ^
   4 |   }
   5 |
   6 |   return null"
 `;
 
 exports[`ReactRefreshLogBox app default boundaries 1`] = `
-"FunctionDefault.js (1:50) @ FunctionDefault
+"FunctionDefault.js (1:51) @ FunctionDefault
 
 > 1 | export default function FunctionDefault() { throw new Error('no'); }
-    |                                                  ^"
+    |                                                   ^"
 `;
 
 exports[`ReactRefreshLogBox app default conversion to class component (1) 1`] = `
-"Child.js (4:10) @ ClickCount.render
+"Child.js (4:11) @ ClickCount.render
 
   2 | export default class ClickCount extends Component {
   3 |   render() {
 > 4 |     throw new Error()
-    |          ^
+    |           ^
   5 |   }
   6 | }"
 `;
@@ -76,31 +77,31 @@ exports[`ReactRefreshLogBox app default logbox: anchors links in error messages
 exports[`ReactRefreshLogBox app default logbox: anchors links in error messages 9`] = `"http://example.com/"`;
 
 exports[`ReactRefreshLogBox app default module init error not shown 1`] = `
-"index.js (3:6) @ eval
+"index.js (3:7) @ eval
 
   1 | // top offset for snapshot
   2 | import * as React from 'react';
 > 3 | throw new Error('no')
-    |      ^
+    |       ^
   4 | class ClassDefault extends React.Component {
   5 |   render() {
   6 |     return <h1>Default Export</h1>;"
 `;
 
 exports[`ReactRefreshLogBox app default should strip whitespace correctly with newline 1`] = `
-"index.js (7:14) @ onClick
+"index.js (7:15) @ onClick
 
    5 |
    6 |       <a onClick={() => {
 >  7 |         throw new Error('idk')
-     |              ^
+     |               ^
    8 |       }}>
    9 |         click me
   10 |       </a>"
 `;
 
 exports[`ReactRefreshLogBox app turbo Can't resolve @import in CSS file 1`] = `
-"./app/styles1.css:1:0
+"./app/styles1.css:1:1
 Parsing css source code failed
 > 1 | @import "./styles2.css"
     | ^^^^^^^^^^^^^^^^^^^^^^^
@@ -109,7 +110,7 @@ Unexpected end of file, but expected ';' or '{'"
 `;
 
 exports[`ReactRefreshLogBox app turbo Import trace when module not found in layout 1`] = `
-"./app/module.js:1:0
+"./app/module.js:1:1
 Module not found: Can't resolve 'non-existing-module'
 > 1 | import "non-existing-module"
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -118,21 +119,22 @@ https://nextjs.org/docs/messages/module-not-found"
 `;
 
 exports[`ReactRefreshLogBox app turbo Should not show __webpack_exports__ when exporting anonymous arrow function 1`] = `
-"index.js (3:2) @ {default export}
+"index.js (3:11) @ {default export}
 
   1 | export default () => {
   2 |   if (typeof window !== 'undefined') {
 > 3 |     throw new Error('test')
-    |  ^
+    |           ^
   4 |   }
   5 |
   6 |   return null"
 `;
 
 exports[`ReactRefreshLogBox app turbo boundaries 1`] = `
-"FunctionDefault.js (0:67) @ FunctionDefault
+"FunctionDefault.js (1:51) @ FunctionDefault
 
-  1 | export default function FunctionDefault() { throw new Error('no'); }"
+> 1 | export default function FunctionDefault() { throw new Error('no'); }
+    |                                                   ^"
 `;
 
 exports[`ReactRefreshLogBox app turbo logbox: anchors links in error messages 1`] = `"Error: end http://nextjs.org"`;
@@ -152,24 +154,24 @@ exports[`ReactRefreshLogBox app turbo logbox: anchors links in error messages 8`
 exports[`ReactRefreshLogBox app turbo logbox: anchors links in error messages 9`] = `"http://example.com/"`;
 
 exports[`ReactRefreshLogBox app turbo module init error not shown 1`] = `
-"index.js (3:6) @ <unknown>
+"index.js (3:7) @ <unknown>
 
   1 | // top offset for snapshot
   2 | import * as React from 'react';
 > 3 | throw new Error('no')
-    |      ^
+    |       ^
   4 | class ClassDefault extends React.Component {
   5 |   render() {
   6 |     return <h1>Default Export</h1>;"
 `;
 
 exports[`ReactRefreshLogBox app turbo should strip whitespace correctly with newline 1`] = `
-"index.js (7:6) @ onClick
+"index.js (7:15) @ onClick
 
    5 |
    6 |       <a onClick={() => {
 >  7 |         throw new Error('idk')
-     |      ^
+     |               ^
    8 |       }}>
    9 |         click me
   10 |       </a>"