Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dev-overlay] Always show relative paths #76742

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function shouldIgnorePath(modulePath: string): boolean {
type IgnorableStackFrame = StackFrame & { ignored: boolean }

const currentSourcesByFile: Map<string, Promise<string | null>> = new Map()
export async function batchedTraceSource(
async function batchedTraceSource(
project: Project,
frame: TurbopackStackFrame
): Promise<{ frame: IgnorableStackFrame; source: string | null } | undefined> {
Expand Down Expand Up @@ -316,9 +316,7 @@ async function nativeTraceSource(
?.replace('__WEBPACK_DEFAULT_EXPORT__', 'default')
?.replace('__webpack_exports__.', '') || '<unknown>',
column: (originalPosition.column ?? 0) + 1,
file: originalPosition.source?.startsWith('file://')
? relativeToCwd(originalPosition.source)
: originalPosition.source,
file: originalPosition.source,
lineNumber: originalPosition.line ?? 0,
// TODO: c&p from async createOriginalStackFrame but why not frame.arguments?
arguments: [],
Expand All @@ -335,14 +333,9 @@ 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,
projectPath: string,
frame: TurbopackStackFrame
): Promise<OriginalStackFrameResponse | null> {
const traced =
Expand All @@ -354,13 +347,31 @@ async function createOriginalStackFrame(
return null
}

let normalizedStackFrameLocation = traced.frame.file
if (
normalizedStackFrameLocation !== null &&
normalizedStackFrameLocation.startsWith('file://')
) {
normalizedStackFrameLocation = path.relative(
projectPath,
url.fileURLToPath(normalizedStackFrameLocation)
)
}

return {
originalStackFrame: traced.frame,
originalStackFrame: {
arguments: traced.frame.arguments,
column: traced.frame.column,
file: normalizedStackFrameLocation,
ignored: traced.frame.ignored,
lineNumber: traced.frame.lineNumber,
methodName: traced.frame.methodName,
},
originalCodeFrame: getOriginalCodeFrame(traced.frame, traced.source),
}
}

export function getOverlayMiddleware(project: Project) {
export function getOverlayMiddleware(project: Project, projectPath: string) {
return async function (
req: IncomingMessage,
res: ServerResponse,
Expand All @@ -387,7 +398,11 @@ export function getOverlayMiddleware(project: Project) {
const result: OriginalStackFramesResponse = await Promise.all(
stackFrames.map(async (frame) => {
try {
const stackFrame = await createOriginalStackFrame(project, frame)
const stackFrame = await createOriginalStackFrame(
project,
projectPath,
frame
)
if (stackFrame === null) {
return {
status: 'rejected',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,19 @@ async function getOriginalStackFrame({
},
})

let defaultNormalizedStackFrameLocation = frame.file
if (
defaultNormalizedStackFrameLocation !== null &&
defaultNormalizedStackFrameLocation.startsWith('file://')
) {
defaultNormalizedStackFrameLocation = path.relative(
rootDirectory,
fileURLToPath(defaultNormalizedStackFrameLocation)
)
}
// This stack frame is used for the one that couldn't locate the source or source mapped frame
const defaultStackFrame: IgnorableStackFrame = {
file: frame.file,
file: defaultNormalizedStackFrameLocation,
lineNumber: frame.lineNumber,
column: frame.column ?? 1,
methodName: frame.methodName,
Expand Down
16 changes: 8 additions & 8 deletions packages/next/src/server/dev/hot-reloader-turbopack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export async function createHotReloaderTurbopack(
): Promise<NextJsHotReloaderInterface> {
const dev = true
const buildId = 'development'
const { nextConfig, dir } = opts
const { nextConfig, dir: projectPath } = opts

const { loadBindings } =
require('../../build/swc') as typeof import('../../build/swc')
Expand All @@ -172,7 +172,7 @@ export async function createHotReloaderTurbopack(
// works correctly. Normally `run-test` hides output so only will be visible when `--debug` flag is used.
if (process.env.TURBOPACK && isTestMode) {
require('console').log('Creating turbopack project', {
dir,
dir: projectPath,
testMode: isTestMode,
})
}
Expand Down Expand Up @@ -207,14 +207,14 @@ export async function createHotReloaderTurbopack(

const project = await bindings.turbo.createProject(
{
projectPath: dir,
projectPath: projectPath,
rootPath:
opts.nextConfig.experimental.turbo?.root ||
opts.nextConfig.outputFileTracingRoot ||
dir,
projectPath,
distDir,
nextConfig: opts.nextConfig,
jsConfig: await getTurbopackJsConfig(dir, nextConfig),
jsConfig: await getTurbopackJsConfig(projectPath, nextConfig),
watch: {
enable: dev,
pollIntervalMs: nextConfig.watchOptions?.pollIntervalMs,
Expand Down Expand Up @@ -244,7 +244,7 @@ export async function createHotReloaderTurbopack(
}
)
setBundlerFindSourceMapImplementation(
getSourceMapFromTurbopack.bind(null, project, dir)
getSourceMapFromTurbopack.bind(null, project, projectPath)
)
opts.onDevServerCleanup?.(async () => {
setBundlerFindSourceMapImplementation(() => undefined)
Expand Down Expand Up @@ -634,7 +634,7 @@ export async function createHotReloaderTurbopack(
)

const middlewares = [
getOverlayMiddleware(project),
getOverlayMiddleware(project, projectPath),
getSourceMapMiddleware(project),
getNextErrorFeedbackMiddleware(opts.telemetry),
getDevOverlayFontMiddleware(),
Expand Down Expand Up @@ -957,7 +957,7 @@ export async function createHotReloaderTurbopack(
> =
definition ??
(await findPagePathData(
dir,
projectPath,
inputPage,
nextConfig.pageExtensions,
opts.pagesDir,
Expand Down
36 changes: 18 additions & 18 deletions test/development/acceptance-app/ReactRefreshLogBox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,15 @@ describe('ReactRefreshLogBox app', () => {
| ^",
"stack": [
"eval index.js (3:7)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"eval ./app/page.js",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
],
}
`)
Expand Down Expand Up @@ -1468,10 +1468,10 @@ describe('ReactRefreshLogBox app', () => {
"stack": [
"eval index.js (1:7)",
"<FIXME-next-dist-dir>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"eval ./app/server/page.js",
"<FIXME-next-dist-dir>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
],
}
`)
Expand Down Expand Up @@ -1639,15 +1639,15 @@ export default function Home() {
| ^",
"stack": [
"eval app/utils.ts (1:7)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"eval ./app/page.js",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
],
}
`)
Expand Down
75 changes: 38 additions & 37 deletions test/development/acceptance/ReactRefreshLogBox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ describe('ReactRefreshLogBox', () => {
| ^",
"stack": [
"[project]/index.js [ssr] (ecmascript) index.js (3:7)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
],
}
`)
Expand All @@ -149,18 +149,18 @@ describe('ReactRefreshLogBox', () => {
| ^",
"stack": [
"eval index.js (3:7)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"eval ./pages/index.js",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
],
}
`)
Expand All @@ -178,9 +178,9 @@ describe('ReactRefreshLogBox', () => {
| ^",
"stack": [
"[project]/index.js [ssr] (ecmascript) index.js (3:7)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
],
}
`)
Expand All @@ -196,18 +196,18 @@ describe('ReactRefreshLogBox', () => {
| ^",
"stack": [
"eval index.js (3:7)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"eval ./pages/index.js",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
"<FIXME-next-dist-dir>",
],
}
`)
Expand Down Expand Up @@ -284,8 +284,8 @@ describe('ReactRefreshLogBox', () => {
"stack": [
"FunctionDefault FunctionDefault.js (1:51)",
"Set.forEach <anonymous> (0:0)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
],
}
`)
Expand All @@ -302,8 +302,8 @@ describe('ReactRefreshLogBox', () => {
"stack": [
"FunctionDefault FunctionDefault.js (1:51)",
"Set.forEach <anonymous> (0:0)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
],
}
`)
Expand Down Expand Up @@ -413,7 +413,6 @@ describe('ReactRefreshLogBox', () => {
}
})

// Module trace is only available with webpack 5
test('conversion to class component (1)', async () => {
await using sandbox = await createSandbox(next)
const { browser, session } = sandbox
Expand Down Expand Up @@ -459,6 +458,8 @@ describe('ReactRefreshLogBox', () => {
`
)

// TODO(veil): ignore-list Webpack runtime (https://linear.app/vercel/issue/NDX-945)
// TODO(veil): Don't bail in Turbopack for sources outside of the project (https://linear.app/vercel/issue/NDX-944)
if (isReact18) {
await expect(browser).toDisplayRedbox(`
{
Expand All @@ -472,8 +473,8 @@ describe('ReactRefreshLogBox', () => {
"stack": [
"ClickCount.render Child.js (4:11)",
"Set.forEach <anonymous> (0:0)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
],
}
`)
Expand All @@ -490,8 +491,8 @@ describe('ReactRefreshLogBox', () => {
"stack": [
"ClickCount.render Child.js (4:11)",
"Set.forEach <anonymous> (0:0)",
"<FIXME-file-protocol>",
"<FIXME-file-protocol>",
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
],
}
`)
Expand Down
Loading
Loading