Ensure Issue Overlay sourcemaps externals in Turbopack (#73439)

Closes https://linear.app/vercel/issue/NDX-473/

Author: eps1lon

packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts

@@ -14,9 +14,10 @@ import path from 'path'
 import url from 'url'
 import { launchEditor } from '../internal/helpers/launchEditor'
 import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
+import { SourceMapConsumer } from 'next/dist/compiled/source-map08'
 import type { Project, TurbopackStackFrame } from '../../../../build/swc/types'
 import { getSourceMapFromFile } from '../internal/helpers/get-source-map-from-file'
-import { findSourceMap } from 'node:module'
+import { findSourceMap, type SourceMapPayload } from 'node:module'
 
 function shouldIgnorePath(modulePath: string): boolean {
   return (
@@ -33,7 +34,10 @@ export async function batchedTraceSource(
   project: Project,
   frame: TurbopackStackFrame
 ): Promise<{ frame: IgnorableStackFrame; source: string | null } | undefined> {
-  const file = frame.file ? decodeURIComponent(frame.file) : undefined
+  const file = frame.file
+    ? // TODO(veil): Why are the frames sent encoded?
+      decodeURIComponent(frame.file)
+    : undefined
   if (!file) return
 
   const sourceFrame = await project.traceSource(frame)
@@ -108,11 +112,155 @@ function createStackFrame(searchParams: URLSearchParams) {
   } satisfies TurbopackStackFrame
 }
 
+/**
+ * https://tc39.es/source-map/#index-map
+ */
+interface IndexSourceMapSection {
+  offset: {
+    line: number
+    column: number
+  }
+  map: ModernRawSourceMap
+}
+
+// TODO(veil): Upstream types
+interface IndexSourceMap {
+  version: number
+  file: string
+  sections: IndexSourceMapSection[]
+}
+
+interface ModernRawSourceMap extends SourceMapPayload {
+  ignoreList?: number[]
+}
+
+type ModernSourceMapPayload = ModernRawSourceMap | IndexSourceMap
+
+/**
+ * Finds the sourcemap payload applicable to a given frame.
+ * Equal to the input unless an Index Source Map is used.
+ */
+function findApplicableSourceMapPayload(
+  frame: TurbopackStackFrame,
+  payload: ModernSourceMapPayload
+): ModernRawSourceMap | undefined {
+  if ('sections' in payload) {
+    const frameLine = frame.line ?? 0
+    const frameColumn = frame.column ?? 0
+    // Sections must not overlap and must be sorted: https://tc39.es/source-map/#section-object
+    // Therefore the last section that has an offset less than or equal to the frame is the applicable one.
+    // TODO(veil): Binary search
+    let section: IndexSourceMapSection | undefined = payload.sections[0]
+    for (
+      let i = 0;
+      i < payload.sections.length &&
+      payload.sections[i].offset.line <= frameLine &&
+      payload.sections[i].offset.column <= frameColumn;
+      i++
+    ) {
+      section = payload.sections[i]
+    }
+
+    return section === undefined ? undefined : section.map
+  } else {
+    return payload
+  }
+}
+
+async function nativeTraceSource(
+  frame: TurbopackStackFrame
+): Promise<{ frame: IgnorableStackFrame; source: string | null } | undefined> {
+  const sourceMap = findSourceMap(
+    // TODO(veil): Why are the frames sent encoded?
+    decodeURIComponent(frame.file)
+  )
+  if (sourceMap !== undefined) {
+    const traced = await SourceMapConsumer.with(
+      sourceMap.payload,
+      null,
+      async (consumer) => {
+        const originalPosition = consumer.originalPositionFor({
+          line: frame.line ?? 1,
+          column: frame.column ?? 1,
+        })
+
+        if (originalPosition.source === null) {
+          return null
+        }
+
+        const sourceContent: string | null =
+          consumer.sourceContentFor(
+            originalPosition.source,
+            /* returnNullOnMissing */ true
+          ) ?? null
+
+        return { originalPosition, sourceContent }
+      }
+    )
+
+    if (traced !== null) {
+      const { originalPosition, sourceContent } = traced
+      const applicableSourceMap = findApplicableSourceMapPayload(
+        frame,
+        sourceMap.payload
+      )
+
+      // TODO(veil): Upstream a method to sourcemap consumer that immediately says if a frame is ignored or not.
+      let ignored = false
+      if (applicableSourceMap === undefined) {
+        console.error(
+          'No applicable source map found in sections for frame',
+          frame
+        )
+      } else {
+        // TODO: O(n^2). Consider moving `ignoreList` into a Set
+        const sourceIndex = applicableSourceMap.sources.indexOf(
+          originalPosition.source!
+        )
+        ignored = applicableSourceMap.ignoreList?.includes(sourceIndex) ?? false
+      }
+
+      const originalStackFrame: IgnorableStackFrame = {
+        methodName:
+          originalPosition.name ||
+          // default is not a valid identifier in JS so webpack uses a custom variable when it's an unnamed default export
+          // Resolve it back to `default` for the method name if the source position didn't have the method.
+          frame.methodName
+            ?.replace('__WEBPACK_DEFAULT_EXPORT__', 'default')
+            ?.replace('__webpack_exports__.', '') ||
+          '<unknown>',
+        column: (originalPosition.column ?? 0) + 1,
+        file: originalPosition.source?.startsWith('file://')
+          ? path.relative(
+              process.cwd(),
+              url.fileURLToPath(originalPosition.source)
+            )
+          : originalPosition.source,
+        lineNumber: originalPosition.line ?? 0,
+        // TODO: c&p from async createOriginalStackFrame but why not frame.arguments?
+        arguments: [],
+        ignored,
+      }
+
+      return {
+        frame: originalStackFrame,
+        source: sourceContent,
+      }
+    }
+  }
+
+  return undefined
+}
+
 export async function createOriginalStackFrame(
   project: Project,
   frame: TurbopackStackFrame
 ): Promise<OriginalStackFrameResponse | null> {
-  const traced = await batchedTraceSource(project, frame)
+  const traced =
+    (await nativeTraceSource(frame)) ??
+    // TODO(veil): When would the bundler know more than native?
+    // If it's faster, try the bundler first and fall back to native later.
+    (await batchedTraceSource(project, frame))
   if (!traced) {
     return null
   }
@@ -193,6 +341,8 @@ export function getSourceMapMiddleware(project: Project) {
       return badRequest(res)
     }
 
+    // TODO(veil): Always try the native version first.
+    // Externals could also be files that aren't bundled via Webpack.
     if (
       filename.startsWith('webpack://') ||
       filename.startsWith('webpack-internal:///')

test/development/app-dir/dynamic-error-trace/index.test.ts

@@ -41,16 +41,15 @@ describe('app dir - dynamic error trace', () => {
     )
 
     const codeframe = await getRedboxSource(browser)
-    // TODO(NDX-115): column for "^"" marker is inconsistent between native, Webpack, and Turbopack
     expect(codeframe).toEqual(
       process.env.TURBOPACK
         ? outdent`
-            app/lib.js (4:12) @ Foo
+            app/lib.js (4:13) @ Foo
             
               2 |
               3 | export function Foo() {
             > 4 |   useHeaders()
-                |            ^
+                |             ^
               5 |   return 'foo'
               6 | }
               7 |

test/development/app-dir/owner-stack-invalid-element-type/invalid-element-type.test.ts

@@ -66,12 +66,12 @@ const isOwnerStackEnabled =
       if (process.env.TURBOPACK) {
         expect(stackFramesContent).toMatchInlineSnapshot(`""`)
         expect(source).toMatchInlineSnapshot(`
-          "app/rsc/page.js (5:10) @ Inner
+          "app/rsc/page.js (5:11) @ Inner
 
             3 | // Intermediate component for testing owner stack
             4 | function Inner() {
           > 5 |   return <Foo />
-              |          ^
+              |           ^
             6 | }
             7 |
             8 | export default function Page() {"

test/development/app-dir/owner-stack-invalid-element-type/owner-stack-invalid-element-type.test.ts

@@ -70,15 +70,15 @@ const isOwnerStackEnabled =
 
       if (process.env.TURBOPACK) {
         expect(stackFramesContent).toMatchInlineSnapshot(
-          `"at Page (app/rsc/page.js (11:7))"`
+          `"at Page (app/rsc/page.js (11:8))"`
         )
         expect(source).toMatchInlineSnapshot(`
-          "app/rsc/page.js (5:10) @ Inner
+          "app/rsc/page.js (5:11) @ Inner
 
             3 | // Intermediate component for testing owner stack
             4 | function Inner() {
           > 5 |   return <Foo />
-              |          ^
+              |           ^
             6 | }
             7 |
             8 | export default function Page() {"

test/development/app-dir/owner-stack-react-missing-key-prop/owner-stack-react-missing-key-prop.test.ts

@@ -29,12 +29,12 @@ const isOwnerStackEnabled =
           `"at Page (app/rsc/page.tsx (6:13))"`
         )
         expect(source).toMatchInlineSnapshot(`
-        "app/rsc/page.tsx (7:9) @ <anonymous>
+        "app/rsc/page.tsx (7:10) @ <anonymous>
 
            5 |     <div>
            6 |       {list.map((item, index) => (
         >  7 |         <span>{item}</span>
-             |         ^
+             |          ^
            8 |       ))}
            9 |     </div>
           10 |   )"

test/integration/edge-runtime-streaming-error/test/index.test.ts

@@ -50,24 +50,22 @@ function createContext() {
   return ctx
 }
 
-;(process.env.TURBOPACK_BUILD ? describe.skip : describe)(
-  'development mode',
-  () => {
-    const context = createContext()
+// TODO(veil): Missing `cause` in Turbopack
+;(process.env.TURBOPACK ? describe.skip : describe)('development mode', () => {
+  const context = createContext()
 
-    beforeAll(async () => {
-      context.appPort = await findPort()
-      context.app = await launchApp(appDir, context.appPort, {
-        ...context.handler,
-        env: { __NEXT_TEST_WITH_DEVTOOL: '1' },
-      })
+  beforeAll(async () => {
+    context.appPort = await findPort()
+    context.app = await launchApp(appDir, context.appPort, {
+      ...context.handler,
+      env: { __NEXT_TEST_WITH_DEVTOOL: '1' },
     })
+  })
 
-    afterAll(() => killApp(context.app))
+  afterAll(() => killApp(context.app))
 
-    it('logs the error correctly', test(context))
-  }
-)
+  it('logs the error correctly', test(context))
+})
 ;(process.env.TURBOPACK_DEV ? describe.skip : describe)(
   'production mode',
   () => {