add ./ to relative paths to make it easier to understand

vercel · Dec 9, 2024 · 262b7a8 · 262b7a8
1 parent 466d6b8
commit 262b7a8
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 74 deletions.
diff --git a/crates/napi/src/next_api/project.rs b/crates/napi/src/next_api/project.rs
@@ -1124,44 +1124,37 @@ pub async fn project_trace_source(
 
             let project_root_uri =
                 uri_from_file(project.container.project().project_root_path(), None).await? + "/";
-            let (file, original_file, is_internal) = if let Some(source_file) =
-                original_file.strip_prefix(&project_root_uri)
-            {
-                // Client code uses file://
-                (
-                    get_relative_path_to(&current_directory_file_url, &original_file)
-                        // TODO(sokra) remove this to include a ./ here to make it a relative path
-                        .trim_start_matches("./")
-                        .to_string(),
-                    Some(source_file.to_string()),
-                    false,
-                )
-            } else if let Some(source_file) =
-                original_file.strip_prefix(&*SOURCE_MAP_PREFIX_PROJECT)
-            {
-                // Server code uses turbopack://[project]
-                // TODO should this also be file://?
-                (
-                    get_relative_path_to(
-                        &current_directory_file_url,
-                        &format!("{}{}", project_root_uri, source_file),
+            let (file, original_file, is_internal) =
+                if let Some(source_file) = original_file.strip_prefix(&project_root_uri) {
+                    // Client code uses file://
+                    (
+                        get_relative_path_to(&current_directory_file_url, &original_file),
+                        Some(source_file.to_string()),
+                        false,
                     )
-                    // TODO(sokra) remove this to include a ./ here to make it a relative path
-                    .trim_start_matches("./")
-                    .to_string(),
-                    Some(source_file.to_string()),
-                    false,
-                )
-            } else if let Some(source_file) = original_file.strip_prefix(SOURCE_MAP_PREFIX) {
-                // All other code like turbopack://[turbopack] is internal code
-                (source_file.to_string(), None, true)
-            } else {
-                bail!(
-                    "Original file ({}) outside project ({})",
-                    original_file,
-                    project_root_uri
-                )
-            };
+                } else if let Some(source_file) =
+                    original_file.strip_prefix(&*SOURCE_MAP_PREFIX_PROJECT)
+                {
+                    // Server code uses turbopack://[project]
+                    // TODO should this also be file://?
+                    (
+                        get_relative_path_to(
+                            &current_directory_file_url,
+                            &format!("{}{}", project_root_uri, source_file),
+                        ),
+                        Some(source_file.to_string()),
+                        false,
+                    )
+                } else if let Some(source_file) = original_file.strip_prefix(SOURCE_MAP_PREFIX) {
+                    // All other code like turbopack://[turbopack] is internal code
+                    (source_file.to_string(), None, true)
+                } else {
+                    bail!(
+                        "Original file ({}) outside project ({})",
+                        original_file,
+                        project_root_uri
+                    )
+                };
 
             Ok(Some(StackFrame {
                 file,

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
@@ -19,6 +19,7 @@ import type { Project, TurbopackStackFrame } from '../../../../build/swc/types'
 import { getSourceMapFromFile } from '../internal/helpers/get-source-map-from-file'
 import { findSourceMap, type SourceMapPayload } from 'node:module'
 import { pathToFileURL } from 'node:url'
+import { relativeToCwd } from '../../../../server/lib/stack-trace-utils'
 
 function shouldIgnorePath(modulePath: string): boolean {
   return (
@@ -234,9 +235,7 @@ async function nativeTraceSource(
             ?.replace('__webpack_exports__.', '') ||
           '<unknown>',
         column: (originalPosition.column ?? 0) + 1,
-        file: originalPosition.source?.startsWith('file://')
-          ? relativeToCwd(originalPosition.source)
-          : originalPosition.source,
+        file: relativeToCwd(originalPosition.source),
         lineNumber: originalPosition.line ?? 0,
         // TODO: c&p from async createOriginalStackFrame but why not frame.arguments?
         arguments: [],
@@ -253,12 +252,6 @@ async function nativeTraceSource(
   return undefined
 }
 
-function relativeToCwd(file: string): string {
-  const relPath = path.relative(process.cwd(), url.fileURLToPath(file))
-  // TODO(sokra) include a ./ here to make it a relative path
-  return relPath
-}
-
 async function createOriginalStackFrame(
   project: Project,
   frame: TurbopackStackFrame

diff --git a/packages/next/src/server/lib/stack-trace-utils.ts b/packages/next/src/server/lib/stack-trace-utils.ts
@@ -0,0 +1,28 @@
+import { relative, isAbsolute } from 'path'
+import { fileURLToPath } from 'url'
+
+// Formats a file url or absolute path as relative path relative to the current working directory.
+// It will start with ./ or ../ if it's a relative path.
+// It might be an absolute path if it's on a different drive on windows.
+// When the argument is not a file url or a absolute path, it will return the argument as is.
+export function relativeToCwd(file: string): string
+export function relativeToCwd(file: null): null
+export function relativeToCwd(file: string | null): string | null
+export function relativeToCwd(file: string | null): string | null {
+  if (!file) {
+    return file
+  }
+  if (file.startsWith('file://')) {
+    file = fileURLToPath(file)
+  } else if (!isAbsolute(file)) {
+    return file
+  }
+  const relPath = relative(process.cwd(), file)
+  if (isAbsolute(relPath)) {
+    return relPath
+  }
+  if (relPath.startsWith('../')) {
+    return relPath
+  }
+  return './' + relPath
+}
diff --git a/packages/next/src/server/patch-error-inspect.ts b/packages/next/src/server/patch-error-inspect.ts
@@ -2,7 +2,6 @@ import {
   findSourceMap as nativeFindSourceMap,
   type SourceMapPayload,
 } from 'module'
-import * as path from 'path'
 import * as url from 'url'
 import type * as util from 'util'
 import { SourceMapConsumer as SyncSourceMapConsumer } from 'next/dist/compiled/source-map'
@@ -11,6 +10,7 @@ import { parseStack } from '../client/components/react-dev-overlay/server/middle
 import { getOriginalCodeFrame } from '../client/components/react-dev-overlay/server/shared'
 import { workUnitAsyncStorage } from './app-render/work-unit-async-storage.external'
 import { dim } from '../lib/picocolors'
+import { relativeToCwd } from './lib/stack-trace-utils'
 
 type FindSourceMapPayload = (
   sourceURL: string
@@ -66,22 +66,11 @@ function frameToString(frame: StackFrame): string {
     sourceLocation += `:${frame.column}`
   }
 
-  let fileLocation: string | null
-  if (
-    frame.file !== null &&
-    frame.file.startsWith('file://') &&
-    URL.canParse(frame.file)
-  ) {
-    // If not relative to CWD, the path is ambiguous to IDEs and clicking will prompt to select the file first.
-    // In a multi-app repo, this leads to potentially larger file names but will make clicking snappy.
-    // There's no tradeoff for the cases where `dir` in `next dev [dir]` is omitted
-    // since relative to cwd is both the shortest and snappiest.
-    fileLocation = path.relative(process.cwd(), url.fileURLToPath(frame.file))
-  } else if (frame.file !== null && frame.file.startsWith('/')) {
-    fileLocation = path.relative(process.cwd(), frame.file)
-  } else {
-    fileLocation = frame.file
-  }
+  // If not relative to CWD, the path is ambiguous to IDEs and clicking will prompt to select the file first.
+  // In a multi-app repo, this leads to potentially larger file names but will make clicking snappy.
+  // There's no tradeoff for the cases where `dir` in `next dev [dir]` is omitted
+  // since relative to cwd is both the shortest and snappiest.
+  let fileLocation = relativeToCwd(frame.file)
 
   return frame.methodName
     ? `    at ${frame.methodName} (${fileLocation}${sourceLocation})`

diff --git a/packages/next/src/shared/lib/stack-trace-utils.ts b/packages/next/src/shared/lib/stack-trace-utils.ts
@@ -0,0 +1,6 @@
+// Hide the method name when showing a stack trace in the error overlay or when displaying a stack trace in the console.
+export function isHiddenMethodName(methodName: string) {
+  return /^(?:Object\.|Module\.)?(?:<anonymous>|eval|__TURBOPACK__module__evaluation__)$/.test(
+    methodName
+  )
+}
diff --git a/test/e2e/app-dir/non-root-project-monorepo/non-root-project-monorepo.test.ts b/test/e2e/app-dir/non-root-project-monorepo/non-root-project-monorepo.test.ts
@@ -66,7 +66,7 @@ describe('non-root-project-monorepo', () => {
         if (isTurbopack) {
           // TODO the function name should be hidden
           expect(await getRedboxSource(browser)).toMatchInlineSnapshot(`
-            "app/source-maps-rsc/page.tsx (9:28) @ innerArrowFunction
+            "./app/source-maps-rsc/page.tsx (9:28) @ innerArrowFunction
 
                7 | }
                8 |
@@ -82,9 +82,9 @@ describe('non-root-project-monorepo', () => {
             "<unknown>
             [project]/apps/web/app/separate-file.ts [app-rsc] (ecmascript) (rsc://React/Server/file://<full-path>/apps/web/.next/server/chunks/ssr/apps_web_8d1c0a._.js (7:7)
             innerFunction
-            app/source-maps-rsc/page.tsx (6:3)
+            ./app/source-maps-rsc/page.tsx (6:3)
             Page
-            app/source-maps-rsc/page.tsx (2:3)"
+            ./app/source-maps-rsc/page.tsx (2:3)"
           `)
         } else {
           // TODO the function name is incorrect
@@ -120,7 +120,7 @@ describe('non-root-project-monorepo', () => {
         if (isTurbopack) {
           // TODO the function name should be hidden
           expect(await getRedboxSource(browser)).toMatchInlineSnapshot(`
-            "app/separate-file.ts (1:7) @ [project]/apps/web/app/separate-file.ts [app-client] (ecmascript)
+            "./app/separate-file.ts (1:7) @ [project]/apps/web/app/separate-file.ts [app-client] (ecmascript)
 
             > 1 | throw new Error('Expected error')
                 |       ^
@@ -129,11 +129,11 @@ describe('non-root-project-monorepo', () => {
           expect(normalizeStackTrace(await getRedboxCallStack(browser)))
             .toMatchInlineSnapshot(`
             "innerArrowFunction
-            app/source-maps-ssr/page.tsx (11:28)
+            ./app/source-maps-ssr/page.tsx (11:28)
             innerFunction
-            app/source-maps-ssr/page.tsx (8:3)
+            ./app/source-maps-ssr/page.tsx (8:3)
             Page
-            app/source-maps-ssr/page.tsx (4:3)"
+            ./app/source-maps-ssr/page.tsx (4:3)"
           `)
         } else {
           // TODO the function name should be hidden
@@ -173,7 +173,7 @@ describe('non-root-project-monorepo', () => {
         if (isTurbopack) {
           // TODO the function name should be hidden
           expect(await getRedboxSource(browser)).toMatchInlineSnapshot(`
-            "app/separate-file.ts (1:7) @ [project]/apps/web/app/separate-file.ts [app-client] (ecmascript)
+            "./app/separate-file.ts (1:7) @ [project]/apps/web/app/separate-file.ts [app-client] (ecmascript)
 
             > 1 | throw new Error('Expected error')
                 |       ^
@@ -182,11 +182,11 @@ describe('non-root-project-monorepo', () => {
           expect(normalizeStackTrace(await getRedboxCallStack(browser)))
             .toMatchInlineSnapshot(`
             "innerArrowFunction
-            app/source-maps-client/page.tsx (16:28)
+            ./app/source-maps-client/page.tsx (16:28)
             innerFunction
-            app/source-maps-client/page.tsx (13:3)
+            ./app/source-maps-client/page.tsx (13:3)
             effectCallback
-            app/source-maps-client/page.tsx (7:5)"
+            ./app/source-maps-client/page.tsx (7:5)"
           `)
         } else {
           // TODO the function name should be hidden