Skip to content

Commit

Permalink
Turbopack: Use bundler to retrieve sourcemap when native methods are …
Browse files Browse the repository at this point in the history
…not available
  • Loading branch information
eps1lon committed Dec 4, 2024
1 parent faf8ea6 commit 93c99c4
Show file tree
Hide file tree
Showing 12 changed files with 346 additions and 154 deletions.
12 changes: 11 additions & 1 deletion crates/napi/src/next_api/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{path::PathBuf, sync::Arc, thread, time::Duration};

use anyhow::{anyhow, bail, Context, Result};
use napi::{
bindgen_prelude::External,
bindgen_prelude::{within_runtime_if_available, External},
threadsafe_function::{ThreadsafeFunction, ThreadsafeFunctionCallMode},
JsFunction, Status,
};
Expand Down Expand Up @@ -1202,6 +1202,16 @@ pub async fn project_get_source_map(
Ok(source_map)
}

#[napi]
pub fn project_get_source_map_sync(
#[napi(ts_arg_type = "{ __napiType: \"Project\" }")] project: External<ProjectInstance>,
file_path: String,
) -> napi::Result<Option<String>> {
within_runtime_if_available(|| {
tokio::runtime::Handle::current().block_on(project_get_source_map(project, file_path))
})
}

/// Runs exit handlers for the project registered using the [`ExitHandler`] API.
#[napi]
pub async fn project_on_exit(
Expand Down
4 changes: 4 additions & 0 deletions packages/next/src/build/swc/generated-native.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,10 @@ export function projectGetSourceMap(
project: { __napiType: 'Project' },
filePath: string
): Promise<string | null>
export function projectGetSourceMapSync(
project: { __napiType: 'Project' },
filePath: string
): string | null
/** Runs exit handlers for the project registered using the [`ExitHandler`] API. */
export function projectOnExit(project: { __napiType: 'Project' }): Promise<void>
export function rootTaskDispose(rootTask: { __napiType: 'RootTask' }): void
Expand Down
4 changes: 4 additions & 0 deletions packages/next/src/build/swc/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,10 @@ function bindingToApi(
return binding.projectGetSourceMap(this._nativeProject, filePath)
}

getSourceMapSync(filePath: string): string | null {
return binding.projectGetSourceMapSync(this._nativeProject, filePath)
}

updateInfoSubscribe(aggregationMs: number) {
return subscribe<TurbopackResult<UpdateMessage>>(true, async (callback) =>
binding.projectUpdateInfoSubscribe(
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/build/swc/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ export interface Project {
getSourceForAsset(filePath: string): Promise<string | null>

getSourceMap(filePath: string): Promise<string | null>
getSourceMapSync(filePath: string): string | null

traceSource(
stackFrame: TurbopackStackFrame
Expand Down
59 changes: 58 additions & 1 deletion packages/next/src/server/dev/hot-reloader-turbopack.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Socket } from 'net'
import { mkdir, writeFile } from 'fs/promises'
import { join, extname } from 'path'
import { pathToFileURL } from 'url'

import ws from 'next/dist/compiled/ws'

Expand All @@ -20,6 +21,7 @@ import type {
Endpoint,
WrittenEndpoint,
TurbopackResult,
Project,
} from '../../build/swc/types'
import { createDefineEnv } from '../../build/swc'
import * as Log from '../../build/output/log'
Expand Down Expand Up @@ -85,6 +87,10 @@ import { isAppPageRouteDefinition } from '../route-definitions/app-page-route-de
import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths'
import { getNodeDebugType } from '../lib/utils'
import { isMetadataRouteFile } from '../../lib/metadata/is-metadata-route'
import {
setBundlerFindSourceMapImplementation,
type ModernSourceMapPayload,
} from '../patch-error-inspect'
// import { getSupportedBrowsers } from '../../build/utils'

const wsServer = new ws.Server({ noServer: true })
Expand All @@ -96,6 +102,51 @@ const isTestMode = !!(

const sessionId = Math.floor(Number.MAX_SAFE_INTEGER * Math.random())

/**
* Replaces turbopack://[project] with the specified project in the `source` field.
*/
function rewriteTurbopackSources(
projectRoot: string,
sourceMap: ModernSourceMapPayload
): void {
if ('sections' in sourceMap) {
for (const section of sourceMap.sections) {
rewriteTurbopackSources(projectRoot, section.map)
}
} else {
for (let i = 0; i < sourceMap.sources.length; i++) {
sourceMap.sources[i] = pathToFileURL(
join(
projectRoot,
sourceMap.sources[i].replace(/turbopack:\/\/\[project\]/, '')
)
).toString()
}
}
}

function getSourceMapFromTurbopack(
project: Project,
projectRoot: string,
sourceURL: string
): ModernSourceMapPayload | undefined {
let sourceMapJson: string | null = null

try {
sourceMapJson = project.getSourceMapSync(sourceURL)
} catch (err) {}

if (sourceMapJson === null) {
return undefined
} else {
const payload: ModernSourceMapPayload = JSON.parse(sourceMapJson)
// The sourcemap from Turbopack is not yet written to disk so its `sources`
// are not absolute paths yet. We need to rewrite them to be absolute paths.
rewriteTurbopackSources(projectRoot, payload)
return payload
}
}

export async function createHotReloaderTurbopack(
opts: SetupOpts,
serverFields: ServerFields,
Expand Down Expand Up @@ -185,7 +236,13 @@ export async function createHotReloaderTurbopack(
memoryLimit: opts.nextConfig.experimental.turbo?.memoryLimit,
}
)
opts.onDevServerCleanup?.(() => project.onExit())
setBundlerFindSourceMapImplementation(
getSourceMapFromTurbopack.bind(null, project, dir)
)
opts.onDevServerCleanup?.(async () => {
setBundlerFindSourceMapImplementation(() => undefined)
await project.onExit()
})
const entrypointsSubscription = project.entrypointsSubscribe()

const currentWrittenEntrypoints: Map<EntryKey, WrittenEndpoint> = new Map()
Expand Down
42 changes: 31 additions & 11 deletions packages/next/src/server/patch-error-inspect.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { findSourceMap, type SourceMapPayload } from 'module'
import {
findSourceMap as nativeFindSourceMap,
type SourceMapPayload,
} from 'module'
import * as path from 'path'
import * as url from 'url'
import type * as util from 'util'
Expand All @@ -9,6 +12,21 @@ import { getOriginalCodeFrame } from '../client/components/react-dev-overlay/ser
import { workUnitAsyncStorage } from './app-render/work-unit-async-storage.external'
import { dim } from '../lib/picocolors'

type FindSourceMapPayload = (
sourceURL: string
) => ModernSourceMapPayload | undefined
// Find a source map using the bundler's API.
// This is only a fallback for when Node.js fails to due to bugs e.g. https://github.com/nodejs/node/issues/52102
// TODO: Remove once all supported Node.js versions are fixed.
// TODO(veil): Set from Webpack as well
let bundlerFindSourceMapPayload: FindSourceMapPayload = () => undefined

export function setBundlerFindSourceMapImplementation(
findSourceMapImplementation: FindSourceMapPayload
): void {
bundlerFindSourceMapPayload = findSourceMapImplementation
}

/**
* https://tc39.es/source-map/#index-map
*/
Expand All @@ -31,7 +49,7 @@ interface ModernRawSourceMap extends SourceMapPayload {
ignoreList?: number[]
}

type ModernSourceMapPayload = ModernRawSourceMap | IndexSourceMap
export type ModernSourceMapPayload = ModernRawSourceMap | IndexSourceMap

interface IgnoreableStackFrame extends StackFrame {
ignored: boolean
Expand Down Expand Up @@ -138,7 +156,7 @@ function getSourcemappedFrameIfPossible(
}

const sourceMapCacheEntry = sourceMapCache.get(frame.file)
let sourceMap: SyncSourceMapConsumer
let sourceMapConsumer: SyncSourceMapConsumer
let sourceMapPayload: ModernSourceMapPayload
if (sourceMapCacheEntry === undefined) {
let sourceURL = frame.file
Expand All @@ -148,25 +166,27 @@ function getSourcemappedFrameIfPossible(
if (sourceURL.startsWith('/')) {
sourceURL = url.pathToFileURL(frame.file).toString()
}
const moduleSourceMap = findSourceMap(sourceURL)
if (moduleSourceMap === undefined) {
const maybeSourceMapPayload =
nativeFindSourceMap(sourceURL)?.payload ??
bundlerFindSourceMapPayload(sourceURL)
if (maybeSourceMapPayload === undefined) {
return null
}
sourceMapPayload = moduleSourceMap.payload
sourceMap = new SyncSourceMapConsumer(
sourceMapPayload = maybeSourceMapPayload
sourceMapConsumer = new SyncSourceMapConsumer(
// @ts-expect-error -- Module.SourceMap['version'] is number but SyncSourceMapConsumer wants a string
sourceMapPayload
)
sourceMapCache.set(frame.file, {
map: sourceMap,
map: sourceMapConsumer,
payload: sourceMapPayload,
})
} else {
sourceMap = sourceMapCacheEntry.map
sourceMapConsumer = sourceMapCacheEntry.map
sourceMapPayload = sourceMapCacheEntry.payload
}

const sourcePosition = sourceMap.originalPositionFor({
const sourcePosition = sourceMapConsumer.originalPositionFor({
column: frame.column ?? 0,
line: frame.lineNumber ?? 1,
})
Expand All @@ -176,7 +196,7 @@ function getSourcemappedFrameIfPossible(
}

const sourceContent: string | null =
sourceMap.sourceContentFor(
sourceMapConsumer.sourceContentFor(
sourcePosition.source,
/* returnNullOnMissing */ true
) ?? null
Expand Down
45 changes: 33 additions & 12 deletions test/development/middleware-errors/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,25 @@ describe('middleware - development errors', () => {
isTurbopack
? '\n ⨯ Error: boom' +
// TODO(veil): Should be sourcemapped
'\n at __TURBOPACK__default__export__ ('
'\n at __TURBOPACK__default__export__ (middleware.js:3:14)' +
'\n 1 |' +
'\n 2 | export default function () {' +
"\n> 3 | throw new Error('boom')" +
'\n | ^' +
'\n 4 | }' +
'\n'
: '\n ⨯ Error: boom' +
'\n at default (middleware.js:3:14)' +
// TODO(veil): Should be ignore-listed
'\n at eval (webpack'
)
if (isTurbopack) {
// TODO(veil): Should have codeframe
// already asserted on codeframe earlier
} else {
expect(stripAnsi(next.cliOutput)).toContain(
"\n> 3 | throw new Error('boom')"
'' +
"\n> 3 | throw new Error('boom')" +
'\n | ^'
)
}
})
Expand Down Expand Up @@ -92,19 +100,30 @@ describe('middleware - development errors', () => {
})
expect(stripAnsi(next.cliOutput)).toContain(
isTurbopack
? // TODO(veil): Should be sourcemapped
' ⨯ unhandledRejection: Error: async boom!\n at throwError (/'
? ' ⨯ unhandledRejection: Error: async boom!' +
'\n at throwError (middleware.js:4:14)' +
// TODO(veil): Sourcemap to original name i.e. "default"
'\n at __TURBOPACK__default__export__ (middleware.js:7:8)' +
"\n 2 | import { NextResponse } from 'next/server'" +
'\n 3 | async function throwError() {' +
"\n> 4 | throw new Error('async boom!')" +
'\n | ^' +
'\n 5 | }' +
'\n 6 | export default function () {' +
'\n 7 | throwError()'
: '\n ⨯ unhandledRejection: Error: async boom!' +
'\n at throwError (middleware.js:4:14)' +
'\n at throwError (middleware.js:7:8)' +
// TODO(veil): Should be ignore-listed
'\n at eval (webpack'
)
if (isTurbopack) {
// TODO(veil): Should have codeframe
// already asserted on codeframe earlier
} else {
expect(stripAnsi(next.cliOutput)).toContain(
"> 4 | throw new Error('async boom!')"
'' +
"\n> 4 | throw new Error('async boom!')" +
'\n | ^'
)
}
})
Expand Down Expand Up @@ -142,19 +161,21 @@ describe('middleware - development errors', () => {
// In CI, it prefixes "Dynamic Code Evaluation".
expect(stripAnsi(next.cliOutput)).toContain(
// TODO(veil): Should be sourcemapped
'\n at __TURBOPACK__default__export__ (/'
'\n at __TURBOPACK__default__export__ (.next/'
)
}
expect(stripAnsi(next.cliOutput)).toContain(
isTurbopack
? '\n ⨯ Error [ReferenceError]: test is not defined' +
// TODO(veil): Should be sourcemapped
'\n at eval '
'\n at eval (middleware.js:4:8)' +
'\n at <unknown> (middleware.js:4:8)' +
// TODO(veil): Should be ignore-listed
'\n at fn (node_modules'
: '\n ⨯ Error [ReferenceError]: test is not defined' +
// TODO: Redundant and not clickable
// TODO(veil): Redundant and not clickable
'\n at eval (file://webpack-internal:///(middleware)/./middleware.js)' +
'\n at eval (middleware.js:4:8)' +
// TODO: Should be ignore-listed
// TODO(veil): Should be ignore-listed
'\n at fn (node_modules'
)
expect(stripAnsi(next.cliOutput)).toContain(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@ describe('app-dir - server source maps edge runtime', () => {
expect(normalizeCliOutput(next.cliOutput.slice(outputIndex))).toContain(
isTurbopack
? '\nError: Boom' +
// TODO(veil): Should be sourcemapped
'\n at logError (.next'
'\n at logError (app/rsc-error-log/page.js:2:16)' +
'\n at Page (app/rsc-error-log/page.js:6:2)' +
'\n 1 | function logError() {' +
"\n> 2 | console.error(new Error('Boom'))" +
'\n | ^' +
'\n 3 | }' +
'\n 4 |' +
'\n 5 | export default function Page() { {' +
'\n ' +
'\n}'
: '\nError: Boom' +
'\n at logError (app/rsc-error-log/page.js:2:16)' +
'\n at logError (app/rsc-error-log/page.js:6:2)' +
Expand Down Expand Up @@ -61,8 +69,16 @@ describe('app-dir - server source maps edge runtime', () => {
expect(cliOutput).toContain(
isTurbopack
? '\n ⨯ Error: Boom' +
// TODO(veil): Apply sourcemap
'\n at throwError (/'
'\n at throwError (app/ssr-throw/page.js:4:8)' +
'\n at Page (app/ssr-throw/page.js:8:2)' +
'\n 2 |' +
'\n 3 | function throwError() {' +
"\n> 4 | throw new Error('Boom')" +
'\n | ^' +
'\n 5 | }' +
'\n 6 |' +
'\n 7 | export default function Page() { {' +
"\n digest: '"
: '\n ⨯ Error: Boom' +
'\n at throwError (app/ssr-throw/page.js:4:8)' +
// TODO(veil): Method name should be "Page"
Expand Down Expand Up @@ -96,8 +112,15 @@ describe('app-dir - server source maps edge runtime', () => {
expect(cliOutput).toContain(
isTurbopack
? '\n ⨯ Error: Boom' +
// TODO(veil): Apply sourcemap
'\n at throwError (/'
'\n at throwError (app/rsc-throw/page.js:2:8)' +
'\n at Page (app/rsc-throw/page.js:6:2)' +
'\n 1 | function throwError() {' +
"\n> 2 | throw new Error('Boom')" +
'\n | ^' +
'\n 3 | }' +
'\n 4 |' +
'\n 5 | export default function Page() { {' +
"\n digest: '"
: '\n ⨯ Error: Boom' +
'\n at throwError (app/rsc-throw/page.js:2:8)' +
// TODO(veil): Method name should be "Page"
Expand Down
Loading

0 comments on commit 93c99c4

Please sign in to comment.