Use snapshots for component-stack tests (#60768)

### What? Follow-up to #60579 and #60750. Checking `startsWith` is not enough because it hides the rest of the stack. Changed the test to check the snapshot for Turbopack and webpack. Fixes a bug where the stack lines showed `http (NaN:NaN)` as the source lines. Added support for source lines that don't have a open in editor.  Closes NEXT-2128
vercel · Jan 18, 2024 · 299d145 · 299d145
1 parent 486567d
commit 299d145
Show file tree

Hide file tree

Showing 9 changed files with 254 additions and 93 deletions.
diff --git a/...t/components/react-dev-overlay/internal/container/RuntimeError/ComponentStackFrameRow.tsx b/...t/components/react-dev-overlay/internal/container/RuntimeError/ComponentStackFrameRow.tsx
@@ -2,9 +2,11 @@ import React from 'react'
 import type { ComponentStackFrame } from '../../helpers/parse-component-stack'
 import { useOpenInEditor } from '../../helpers/use-open-in-editor'
 
-export function ComponentStackFrameRow({
-  componentStackFrame: { component, file, lineNumber, column },
+function EditorLink({
+  children,
+  componentStackFrame: { file, column, lineNumber },
 }: {
+  children: React.ReactNode
   componentStackFrame: ComponentStackFrame
 }) {
   const open = useOpenInEditor({
@@ -13,34 +15,87 @@ export function ComponentStackFrameRow({
     lineNumber,
   })
 
+  return (
+    <div
+      tabIndex={10} // match CallStackFrame
+      role={'link'}
+      onClick={open}
+      title={'Click to open in your editor'}
+    >
+      {children}
+      <svg
+        xmlns="http://www.w3.org/2000/svg"
+        viewBox="0 0 24 24"
+        fill="none"
+        stroke="currentColor"
+        strokeWidth="2"
+        strokeLinecap="round"
+        strokeLinejoin="round"
+      >
+        <path d="M18 13v6a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2V8a2 2 0 0 1 2-2h6"></path>
+        <polyline points="15 3 21 3 21 9"></polyline>
+        <line x1="10" y1="14" x2="21" y2="3"></line>
+      </svg>
+    </div>
+  )
+}
+
+function formatLineNumber(lineNumber: number, column: number | undefined) {
+  if (!column) {
+    return lineNumber
+  }
+
+  return `${lineNumber}:${column}`
+}
+
+function LocationLine({
+  componentStackFrame,
+}: {
+  componentStackFrame: ComponentStackFrame
+}) {
+  const { file, lineNumber, column } = componentStackFrame
+  return (
+    <>
+      {file} {lineNumber ? `(${formatLineNumber(lineNumber, column)})` : ''}
+    </>
+  )
+}
+
+function SourceLocation({
+  componentStackFrame,
+}: {
+  componentStackFrame: ComponentStackFrame
+}) {
+  const { file, canOpenInEditor } = componentStackFrame
+
+  if (file && canOpenInEditor) {
+    return (
+      <EditorLink componentStackFrame={componentStackFrame}>
+        <span>
+          <LocationLine componentStackFrame={componentStackFrame} />
+        </span>
+      </EditorLink>
+    )
+  }
+
+  return (
+    <div>
+      <LocationLine componentStackFrame={componentStackFrame} />
+    </div>
+  )
+}
+
+export function ComponentStackFrameRow({
+  componentStackFrame,
+}: {
+  componentStackFrame: ComponentStackFrame
+}) {
+  const { component } = componentStackFrame
+
   return (
     <div data-nextjs-component-stack-frame>
       <h3>{component}</h3>
-      {file ? (
-        <div
-          tabIndex={10} // match CallStackFrame
-          role={'link'}
-          onClick={open}
-          title={'Click to open in your editor'}
-        >
-          <span>
-            {file} ({lineNumber}:{column})
-          </span>
-          <svg
-            xmlns="http://www.w3.org/2000/svg"
-            viewBox="0 0 24 24"
-            fill="none"
-            stroke="currentColor"
-            strokeWidth="2"
-            strokeLinecap="round"
-            strokeLinejoin="round"
-          >
-            <path d="M18 13v6a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2V8a2 2 0 0 1 2-2h6"></path>
-            <polyline points="15 3 21 3 21 9"></polyline>
-            <line x1="10" y1="14" x2="21" y2="3"></line>
-          </svg>
-        </div>
-      ) : null}
+      <SourceLocation componentStackFrame={componentStackFrame} />
     </div>
   )
 }
diff --git a/...es/next/src/client/components/react-dev-overlay/internal/container/RuntimeError/index.tsx b/...es/next/src/client/components/react-dev-overlay/internal/container/RuntimeError/index.tsx
@@ -158,7 +158,7 @@ export const styles = css`
     color: #999;
   }
   [data-nextjs-call-stack-frame] > div > svg,
-  [data-nextjs-component-stack-frame] > div > svg {
+  [data-nextjs-component-stack-frame] > [role='link'] > svg {
     width: auto;
     height: var(--size-font-small);
     margin-left: var(--size-gap);
@@ -168,15 +168,15 @@ export const styles = css`
   }
 
   [data-nextjs-call-stack-frame] > div[data-has-source],
-  [data-nextjs-component-stack-frame] > div {
+  [data-nextjs-component-stack-frame] > [role='link'] {
     cursor: pointer;
   }
   [data-nextjs-call-stack-frame] > div[data-has-source]:hover,
-  [data-nextjs-component-stack-frame] > div:hover {
+  [data-nextjs-component-stack-frame] > [role='link']:hover {
     text-decoration: underline dotted;
   }
   [data-nextjs-call-stack-frame] > div[data-has-source] > svg,
-  [data-nextjs-component-stack-frame] > div > svg {
+  [data-nextjs-component-stack-frame] > [role='link'] > svg {
     display: unset;
   }
 

diff --git a/...es/next/src/client/components/react-dev-overlay/internal/helpers/parse-component-stack.ts b/...es/next/src/client/components/react-dev-overlay/internal/helpers/parse-component-stack.ts
@@ -1,38 +1,100 @@
 export type ComponentStackFrame = {
+  canOpenInEditor: boolean
   component: string
   file?: string
   lineNumber?: number
   column?: number
 }
 
+enum LocationType {
+  FILE = 'file',
+  WEBPACK_INTERNAL = 'webpack-internal',
+  HTTP = 'http',
+  PROTOCOL_RELATIVE = 'protocol-relative',
+  UNKNOWN = 'unknown',
+}
+
+/**
+ * Get the type of frame line based on the location
+ */
+function getLocationType(location: string): LocationType {
+  if (location.startsWith('file://')) {
+    return LocationType.FILE
+  }
+  if (location.startsWith('webpack-internal://')) {
+    return LocationType.WEBPACK_INTERNAL
+  }
+  if (location.startsWith('http://') || location.startsWith('https://')) {
+    return LocationType.HTTP
+  }
+  if (location.startsWith('//')) {
+    return LocationType.PROTOCOL_RELATIVE
+  }
+  return LocationType.UNKNOWN
+}
+
+function parseStackFrameLocation(
+  location: string
+): Omit<ComponentStackFrame, 'component'> {
+  const locationType = getLocationType(location)
+
+  const modulePath = location?.replace(
+    /^(webpack-internal:\/\/\/|file:\/\/)(\(.*\)\/)?/,
+    ''
+  )
+  const [, file, lineNumber, column] =
+    modulePath?.match(/^(.+):(\d+):(\d+)/) ?? []
+
+  switch (locationType) {
+    case LocationType.FILE:
+    case LocationType.WEBPACK_INTERNAL:
+      return {
+        canOpenInEditor: true,
+        file,
+        lineNumber: lineNumber ? Number(lineNumber) : undefined,
+        column: column ? Number(column) : undefined,
+      }
+    // When the location is a URL we only show the file
+    // TODO: Resolve http(s) URLs through sourcemaps
+    case LocationType.HTTP:
+    case LocationType.PROTOCOL_RELATIVE:
+    case LocationType.UNKNOWN:
+    default: {
+      return {
+        canOpenInEditor: false,
+      }
+    }
+  }
+}
+
 export function parseComponentStack(
   componentStack: string
 ): ComponentStackFrame[] {
   const componentStackFrames: ComponentStackFrame[] = []
-
   for (const line of componentStack.trim().split('\n')) {
     // Get component and file from the component stack line
     const match = /at ([^ ]+)( \((.*)\))?/.exec(line)
     if (match?.[1]) {
       const component = match[1]
-      const webpackFile = match[3]
+      const location = match[3]
+
+      if (!location) {
+        componentStackFrames.push({
+          canOpenInEditor: false,
+          component,
+        })
+        continue
+      }
 
       // Stop parsing the component stack if we reach a Next.js component
-      if (webpackFile?.includes('next/dist')) {
+      if (location?.includes('next/dist')) {
         break
       }
 
-      const modulePath = webpackFile?.replace(
-        /^(webpack-internal:\/\/\/|file:\/\/)(\(.*\)\/)?/,
-        ''
-      )
-      const [file, lineNumber, column] = modulePath?.split(':', 3) ?? []
-
+      const frameLocation = parseStackFrameLocation(location)
       componentStackFrames.push({
         component,
-        file,
-        lineNumber: lineNumber ? Number(lineNumber) : undefined,
-        column: column ? Number(column) : undefined,
+        ...frameLocation,
       })
     }
   }

diff --git a/test/development/acceptance-app/component-stack.test.ts b/test/development/acceptance-app/component-stack.test.ts
@@ -2,11 +2,10 @@
 import { sandbox } from 'development-sandbox'
 import { FileRef, nextTestSetup } from 'e2e-utils'
 import path from 'path'
-import { outdent } from 'outdent'
 
 describe('Component Stack in error overlay', () => {
   const { next } = nextTestSetup({
-    files: new FileRef(path.join(__dirname, 'fixtures', 'default-template')),
+    files: new FileRef(path.join(__dirname, 'fixtures', 'component-stack')),
     dependencies: {
       react: 'latest',
       'react-dom': 'latest',
@@ -15,48 +14,52 @@ describe('Component Stack in error overlay', () => {
   })
 
   it('should show a component stack on hydration error', async () => {
-    const { cleanup, session } = await sandbox(
-      next,
-      new Map([
-        [
-          'app/component.js',
-          outdent`
-            'use client'
-            const isClient = typeof window !== 'undefined'
-            export default function Component() {
-              return (
-                <div>
-                  <p>{isClient ? "client" : "server"}</p>
-                </div>
-              );
-            }
-          `,
-        ],
-        [
-          'app/page.js',
-          outdent`
-            import Component from './component'
-            export default function Mismatch() {
-              return (
-                <main>
-                  <Component />
-                </main>
-              );
-            }
-          `,
-        ],
-      ])
-    )
+    const { cleanup, session } = await sandbox(next)
 
     await session.waitForAndOpenRuntimeError()
 
-    const expected = `p
-div
-Component
-main`
-    expect((await session.getRedboxComponentStack()).trim()).toStartWith(
-      expected
-    )
+    if (process.env.TURBOPACK) {
+      expect(await session.getRedboxComponentStack()).toMatchInlineSnapshot(`
+        "p
+        div
+        Component
+        main
+        InnerLayoutRouter
+        RedirectErrorBoundary
+        RedirectBoundary
+        NotFoundErrorBoundary
+        NotFoundBoundary
+        LoadingBoundary
+        ErrorBoundary
+        InnerScrollAndFocusHandler
+        ScrollAndFocusHandler
+        RenderFromTemplateContext
+        OuterLayoutRouter
+        body
+        html
+        RedirectErrorBoundary
+        RedirectBoundary
+        NotFoundErrorBoundary
+        NotFoundBoundary
+        DevRootNotFoundBoundary
+        ReactDevOverlay
+        HotReload
+        Router
+        ErrorBoundaryHandler
+        ErrorBoundary
+        AppRouter
+        ServerRoot
+        RSCComponent
+        Root"
+      `)
+    } else {
+      expect(await session.getRedboxComponentStack()).toMatchInlineSnapshot(`
+        "p
+        div
+        Component
+        main"
+      `)
+    }
 
     await cleanup()
   })

diff --git a/test/development/acceptance-app/fixtures/component-stack/app/component.js b/test/development/acceptance-app/fixtures/component-stack/app/component.js
@@ -0,0 +1,9 @@
+'use client'
+const isClient = typeof window !== 'undefined'
+export default function Component() {
+  return (
+    <div>
+      <p>{isClient ? 'client' : 'server'}</p>
+    </div>
+  )
+}
diff --git a/test/development/acceptance-app/fixtures/component-stack/app/layout.js b/test/development/acceptance-app/fixtures/component-stack/app/layout.js
@@ -0,0 +1,8 @@
+export default function RootLayout({ children }) {
+  return (
+    <html>
+      <head></head>
+      <body>{children}</body>
+    </html>
+  )
+}