fix: stack frame format (vercel#71366)

MLH-Fellowship · Oct 16, 2024 · 565ebea · 565ebea
1 parent dbb16a1
commit 565ebea
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 54 deletions.
diff --git a/packages/next/src/client/components/react-dev-overlay/internal/helpers/stack-frame.ts b/packages/next/src/client/components/react-dev-overlay/internal/helpers/stack-frame.ts
@@ -142,36 +142,43 @@ function formatFrameSourceFile(file: string) {
 export function getFrameSource(frame: StackFrame): string {
   if (!frame.file) return ''
 
+  const isWebpackFrame = isWebpackBundled(frame.file)
+
   let str = ''
-  try {
-    const u = new URL(frame.file)
-
-    // Strip the origin for same-origin scripts.
-    if (globalThis.location?.origin !== u.origin) {
-      // URLs can be valid without an `origin`, so long as they have a
-      // `protocol`. However, `origin` is preferred.
-      if (u.origin === 'null') {
-        str += u.protocol
-      } else {
-        str += u.origin
+  // Skip URL parsing for webpack internal file paths.
+  if (isWebpackFrame) {
+    str = formatFrameSourceFile(frame.file)
+  } else {
+    try {
+      const u = new URL(frame.file)
+
+      let parsedPath = ''
+      // Strip the origin for same-origin scripts.
+      if (globalThis.location?.origin !== u.origin) {
+        // URLs can be valid without an `origin`, so long as they have a
+        // `protocol`. However, `origin` is preferred.
+        if (u.origin === 'null') {
+          parsedPath += u.protocol
+        } else {
+          parsedPath += u.origin
+        }
       }
-    }
 
-    // Strip query string information as it's typically too verbose to be
-    // meaningful.
-    str += u.pathname
-    str += ' '
-    str = formatFrameSourceFile(str)
-  } catch {
-    str += formatFrameSourceFile(frame.file || '') + ' '
+      // Strip query string information as it's typically too verbose to be
+      // meaningful.
+      parsedPath += u.pathname
+      str = formatFrameSourceFile(parsedPath)
+    } catch {
+      str = formatFrameSourceFile(frame.file)
+    }
   }
 
   if (!isWebpackBundled(frame.file) && frame.lineNumber != null) {
     if (frame.column != null) {
-      str += `(${frame.lineNumber}:${frame.column}) `
+      str += ` (${frame.lineNumber}:${frame.column})`
     } else {
-      str += `(${frame.lineNumber}) `
+      str += ` (${frame.lineNumber})`
     }
   }
-  return str.slice(0, -1)
+  return str
 }
diff --git a/test/development/acceptance-app/ReactRefreshLogBox.test.ts b/test/development/acceptance-app/ReactRefreshLogBox.test.ts
@@ -1209,40 +1209,36 @@ export default function Home() {
     await cleanup()
   })
 
-  // TODO: Fails with Webpack since
-  // https://github.com/vercel/next.js/pull/71312, not reproducible locally,
-  // investigate why.
-  ;(isTurbopack ? test : test.skip)(
-    'Should collapse bundler internal stack frames',
-    async () => {
-      const { session, browser, cleanup } = await sandbox(
-        next,
-        new Map([
-          [
-            'app/utils.ts',
-            `throw new Error('utils error')
-export function foo(){}`,
-          ],
-          [
-            'app/page.js',
-            `"use client";
+  test('Should collapse bundler internal stack frames', async () => {
+    const { session, browser, cleanup } = await sandbox(
+      next,
+      new Map([
+        [
+          'app/utils.ts',
+          `throw new Error('utils error')
+export function foo(){}
+          `,
+        ],
+        [
+          'app/page.js',
+          `"use client";
 import { foo } from "./utils";
 
 export default function Home() {
   foo();
   return "hello";
 }`,
-          ],
-        ])
-      )
+        ],
+      ])
+    )
 
-      await session.assertHasRedbox()
+    await session.assertHasRedbox()
 
-      let stack = next.normalizeTestDirContent(
-        await getRedboxCallStackCollapsed(browser)
-      )
-      if (isTurbopack) {
-        expect(stack).toMatchInlineSnapshot(`
+    let stack = next.normalizeTestDirContent(
+      await getRedboxCallStackCollapsed(browser)
+    )
+    if (isTurbopack) {
+      expect(stack).toMatchInlineSnapshot(`
         "app/utils.ts (1:7) @ [project]/app/utils.ts [app-client] (ecmascript)
         ---
         Next.js
@@ -1254,8 +1250,8 @@ export default function Home() {
         ---
         React"
       `)
-      } else {
-        expect(stack).toMatchInlineSnapshot(`
+    } else {
+      expect(stack).toMatchInlineSnapshot(`
         "app/utils.ts (1:7) @ eval
         ---
         (app-pages-browser)/./app/utils.ts
@@ -1273,9 +1269,8 @@ export default function Home() {
         ---
         React"
       `)
-      }
-
-      await cleanup()
     }
-  )
+
+    await cleanup()
+  })
 })