Stop sourcemapping function names (#74085)

vercel · Dec 19, 2024 · 848c229 · 848c229
1 parent f533b97
commit 848c229
Show file tree

Hide file tree

Showing 20 changed files with 319 additions and 495 deletions.
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
@@ -84,7 +84,12 @@ export async function batchedTraceSource(
     file: sourceFrame.file,
     lineNumber: sourceFrame.line ?? 0,
     column: sourceFrame.column ?? 0,
-    methodName: sourceFrame.methodName ?? frame.methodName ?? '<unknown>',
+    methodName:
+      // We ignore the sourcemapped name since it won't be the correct name.
+      // The callsite will point to the column of the variable name instead of the
+      // name of the enclosing function.
+      // TODO(NDX-531): Spy on prepareStackTrace to get the enclosing line number for method name mapping.
+      frame.methodName ?? '<unknown>',
     ignored,
     arguments: [],
   }
@@ -226,13 +231,13 @@ async function nativeTraceSource(
 
       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.
+          // We ignore the sourcemapped name since it won't be the correct name.
+          // The callsite will point to the column of the variable name instead of the
+          // name of the enclosing function.
+          // TODO(NDX-531): Spy on prepareStackTrace to get the enclosing line number for method name mapping.
           frame.methodName
             ?.replace('__WEBPACK_DEFAULT_EXPORT__', 'default')
-            ?.replace('__webpack_exports__.', '') ||
-          '<unknown>',
+            ?.replace('__webpack_exports__.', '') || '<unknown>',
         column: (originalPosition.column ?? 0) + 1,
         file: originalPosition.source?.startsWith('file://')
           ? relativeToCwd(originalPosition.source)

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
@@ -232,7 +232,10 @@ export async function createOriginalStackFrame({
     lineNumber: sourcePosition.line,
     column: (sourcePosition.column ?? 0) + 1,
     methodName:
-      sourcePosition.name ||
+      // We ignore the sourcemapped name since it won't be the correct name.
+      // The callsite will point to the column of the variable name instead of the
+      // name of the enclosing function.
+      // TODO(NDX-531): Spy on prepareStackTrace to get the enclosing line number for method name mapping.
       // 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

diff --git a/packages/next/src/server/patch-error-inspect.ts b/packages/next/src/server/patch-error-inspect.ts
@@ -255,13 +255,13 @@ function getSourcemappedFrameIfPossible(
   }
 
   const originalFrame: IgnoreableStackFrame = {
-    methodName:
-      sourcePosition.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__.', ''),
+    // We ignore the sourcemapped name since it won't be the correct name.
+    // The callsite will point to the column of the variable name instead of the
+    // name of the enclosing function.
+    // TODO(NDX-531): Spy on prepareStackTrace to get the enclosing line number for method name mapping.
+    methodName: frame.methodName
+      ?.replace('__WEBPACK_DEFAULT_EXPORT__', 'default')
+      ?.replace('__webpack_exports__.', ''),
     column: sourcePosition.column,
     file: sourcePosition.source,
     lineNumber: sourcePosition.line,

diff --git a/test/development/acceptance/__snapshots__/ReactRefreshLogBox.test.ts.snap b/test/development/acceptance/__snapshots__/ReactRefreshLogBox.test.ts.snap
diff --git a/test/development/acceptance/__snapshots__/error-recovery.test.ts.snap b/test/development/acceptance/__snapshots__/error-recovery.test.ts.snap
diff --git a/...pment/app-dir/capture-console-error-owner-stack/capture-console-error-owner-stack.test.ts b/...pment/app-dir/capture-console-error-owner-stack/capture-console-error-owner-stack.test.ts
@@ -39,6 +39,7 @@ 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(`
         {
@@ -65,11 +66,11 @@ describe('app-dir - capture-console-error-owner-stack', () => {
         {
           "callStacks": "button
         <anonymous> (0:0)
-        button
+        Page
         app/browser/event/page.js (5:6)",
           "count": 1,
           "description": "trigger an console <error>",
-          "source": "app/browser/event/page.js (7:17) @ error
+          "source": "app/browser/event/page.js (7:17) @ onClick
 
            5 |     <button
            6 |       onClick={() => {
@@ -90,44 +91,23 @@ describe('app-dir - capture-console-error-owner-stack', () => {
     await openRedbox(browser)
 
     const result = await getRedboxResult(browser)
-
-    if (process.env.TURBOPACK) {
-      expect(result).toMatchInlineSnapshot(`
-        {
-          "callStacks": "",
-          "count": 1,
-          "description": "trigger an console.error in render",
-          "source": "app/browser/render/page.js (4:11) @ Page
-
-          2 |
-          3 | export default function Page() {
-        > 4 |   console.error('trigger an console.error in render')
-            |           ^
-          5 |   return <p>render</p>
-          6 | }
-          7 |",
-          "title": "Console Error",
-        }
-      `)
-    } else {
-      expect(result).toMatchInlineSnapshot(`
-        {
-          "callStacks": "",
-          "count": 1,
-          "description": "trigger an console.error in render",
-          "source": "app/browser/render/page.js (4:11) @ error
-
-          2 |
-          3 | export default function Page() {
-        > 4 |   console.error('trigger an console.error in render')
-            |           ^
-          5 |   return <p>render</p>
-          6 | }
-          7 |",
-          "title": "Console Error",
-        }
-      `)
-    }
+    expect(result).toMatchInlineSnapshot(`
+       {
+         "callStacks": "",
+         "count": 1,
+         "description": "trigger an console.error in render",
+         "source": "app/browser/render/page.js (4:11) @ Page
+
+         2 |
+         3 | export default function Page() {
+       > 4 |   console.error('trigger an console.error in render')
+           |           ^
+         5 |   return <p>render</p>
+         6 | }
+         7 |",
+         "title": "Console Error",
+       }
+    `)
   })
 
   it('should capture browser console error in render and dedupe when multi same errors logged', async () => {
@@ -136,44 +116,23 @@ describe('app-dir - capture-console-error-owner-stack', () => {
     await openRedbox(browser)
 
     const result = await getRedboxResult(browser)
-
-    if (process.env.TURBOPACK) {
-      expect(result).toMatchInlineSnapshot(`
-        {
-          "callStacks": "",
-          "count": 1,
-          "description": "trigger an console.error in render",
-          "source": "app/browser/render/page.js (4:11) @ Page
-
-          2 |
-          3 | export default function Page() {
-        > 4 |   console.error('trigger an console.error in render')
-            |           ^
-          5 |   return <p>render</p>
-          6 | }
-          7 |",
-          "title": "Console Error",
-        }
-      `)
-    } else {
-      expect(result).toMatchInlineSnapshot(`
-        {
-          "callStacks": "",
-          "count": 1,
-          "description": "trigger an console.error in render",
-          "source": "app/browser/render/page.js (4:11) @ error
-
-          2 |
-          3 | export default function Page() {
-        > 4 |   console.error('trigger an console.error in render')
-            |           ^
-          5 |   return <p>render</p>
-          6 | }
-          7 |",
-          "title": "Console Error",
-        }
-      `)
-    }
+    expect(result).toMatchInlineSnapshot(`
+      {
+        "callStacks": "",
+        "count": 1,
+        "description": "trigger an console.error in render",
+        "source": "app/browser/render/page.js (4:11) @ Page
+
+        2 |
+        3 | export default function Page() {
+      > 4 |   console.error('trigger an console.error in render')
+          |           ^
+        5 |   return <p>render</p>
+        6 | }
+        7 |",
+        "title": "Console Error",
+      }
+    `)
   })
 
   it('should capture server replay string error from console error', async () => {
@@ -182,44 +141,23 @@ describe('app-dir - capture-console-error-owner-stack', () => {
     await openRedbox(browser)
 
     const result = await getRedboxResult(browser)
-
-    if (process.env.TURBOPACK) {
-      expect(result).toMatchInlineSnapshot(`
-        {
-          "callStacks": "",
-          "count": 1,
-          "description": "ssr console error:client",
-          "source": "app/ssr/page.js (4:11) @ Page
-
-          2 |
-          3 | export default function Page() {
-        > 4 |   console.error(
-            |           ^
-          5 |     'ssr console error:' + (typeof window === 'undefined' ? 'server' : 'client')
-          6 |   )
-          7 |   return <p>ssr</p>",
-          "title": "Console Error",
-        }
-      `)
-    } else {
-      expect(result).toMatchInlineSnapshot(`
-        {
-          "callStacks": "",
-          "count": 1,
-          "description": "ssr console error:client",
-          "source": "app/ssr/page.js (4:11) @ error
-
-          2 |
-          3 | export default function Page() {
-        > 4 |   console.error(
-            |           ^
-          5 |     'ssr console error:' + (typeof window === 'undefined' ? 'server' : 'client')
-          6 |   )
-          7 |   return <p>ssr</p>",
-          "title": "Console Error",
-        }
-      `)
-    }
+    expect(result).toMatchInlineSnapshot(`
+       {
+         "callStacks": "",
+         "count": 1,
+         "description": "ssr console error:client",
+         "source": "app/ssr/page.js (4:11) @ Page
+
+         2 |
+         3 | export default function Page() {
+       > 4 |   console.error(
+           |           ^
+         5 |     'ssr console error:' + (typeof window === 'undefined' ? 'server' : 'client')
+         6 |   )
+         7 |   return <p>ssr</p>",
+         "title": "Console Error",
+       }
+    `)
   })
 
   it('should capture server replay error instance from console error', async () => {
@@ -229,43 +167,23 @@ describe('app-dir - capture-console-error-owner-stack', () => {
 
     const result = await getRedboxResult(browser)
 
-    if (process.env.TURBOPACK) {
-      expect(result).toMatchInlineSnapshot(`
-        {
-          "callStacks": "",
-          "count": 1,
-          "description": "Error: page error",
-          "source": "app/ssr-error-instance/page.js (4:17) @ Page
-
-          2 |
-          3 | export default function Page() {
-        > 4 |   console.error(new Error('page error'))
-            |                 ^
-          5 |   return <p>ssr</p>
-          6 | }
-          7 |",
-          "title": "Console Error",
-        }
-      `)
-    } else {
-      expect(result).toMatchInlineSnapshot(`
-        {
-          "callStacks": "",
-          "count": 1,
-          "description": "Error: page error",
-          "source": "app/ssr-error-instance/page.js (4:17) @ Page
-
-          2 |
-          3 | export default function Page() {
-        > 4 |   console.error(new Error('page error'))
-            |                 ^
-          5 |   return <p>ssr</p>
-          6 | }
-          7 |",
-          "title": "Console Error",
-        }
-      `)
-    }
+    expect(result).toMatchInlineSnapshot(`
+      {
+        "callStacks": "",
+        "count": 1,
+        "description": "Error: page error",
+        "source": "app/ssr-error-instance/page.js (4:17) @ Page
+
+        2 |
+        3 | export default function Page() {
+      > 4 |   console.error(new Error('page error'))
+          |                 ^
+        5 |   return <p>ssr</p>
+        6 | }
+        7 |",
+        "title": "Console Error",
+      }
+    `)
   })
 
   it('should be able to capture rsc logged error', async () => {