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

Turbopack: trace HMR errors through sourcemaps correctly #61360

Closed
wants to merge 7 commits into from
Closed
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
53 changes: 39 additions & 14 deletions packages/next-swc/crates/napi/src/next_api/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@ use turbopack_binding::{
},
turbopack::{
core::{
chunk::ModuleId,
diagnostics::PlainDiagnostic,
error::PrettyPrintError,
issue::PlainIssue,
source_map::{GenerateSourceMap, Token},
version::{PartialUpdate, TotalUpdate, Update, VersionState},
},
dev::ecmascript::EcmascriptDevChunkContent,
ecmascript_hmr_protocol::{ClientUpdateInstruction, ResourceIdentifier},
trace_utils::{
exit::ExitGuard,
Expand Down Expand Up @@ -804,12 +806,16 @@ pub async fn project_trace_source(
let turbo_tasks = project.turbo_tasks.clone();
let traced_frame = turbo_tasks
.run_once(async move {
let file = match Url::parse(&frame.file) {
let (file, module) = match Url::parse(&frame.file) {
Ok(url) => match url.scheme() {
"file" => urlencoding::decode(url.path())?.to_string(),
"file" => {
let path = urlencoding::decode(url.path())?.to_string();
let module = url.query_pairs().find(|(k, _)| k == "id");
(path, module.map(|(_, m)| m.into_owned()))
}
_ => bail!("Unknown url scheme"),
},
Err(_) => frame.file.to_string(),
Err(_) => (frame.file.to_string(), None),
};

let Some(chunk_base) = file.strip_prefix(
Expand Down Expand Up @@ -837,19 +843,38 @@ pub async fn project_trace_source(
.join(chunk_base.to_owned())
};

let Some(versioned) = Vc::try_resolve_sidecast::<Box<dyn GenerateSourceMap>>(
project.container.get_versioned_content(path),
)
.await?
else {
bail!("Could not GenerateSourceMap")
let content_vc = project.container.get_versioned_content(path);
let map = match module {
Some(module) => {
let Some(content) =
Vc::try_resolve_downcast_type::<EcmascriptDevChunkContent>(content_vc)
.await?
else {
bail!("Was not EcmascriptDevChunkContent")
};

let entries = content.entries().await?;
let entry = entries.get(&ModuleId::String(module).cell().await?);
let map = match entry {
Some(entry) => *entry.code.generate_source_map().await?,
None => None,
};
map.context("Entry is missing sourcemap")?
}
None => {
let Some(versioned) =
Vc::try_resolve_sidecast::<Box<dyn GenerateSourceMap>>(content_vc).await?
else {
bail!("Could not GenerateSourceMap")
};

versioned
.generate_source_map()
.await?
.context("Chunk is missing a sourcemap")?
}
};

let map = versioned
.generate_source_map()
.await?
.context("Chunk is missing a sourcemap")?;

let token = map
.lookup_token(frame.line as usize, frame.column.unwrap_or(0) as usize)
.await?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function parseStack(stack: string): StackFrame[] {
?.replace(/\\/g, '/')
?.replace(/\/$/, '')
if (distDir) {
frame.file = 'file://' + distDir.concat(res.pop()!)
frame.file = 'file://' + distDir.concat(res.pop()!) + url.search
}
}
} catch {}
Expand Down
39 changes: 27 additions & 12 deletions test/development/acceptance-app/error-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import path from 'path'
import { outdent } from 'outdent'

describe.each(['default', 'turbo'])('Error recovery app %s', () => {
const { next } = nextTestSetup({
const { next, isTurbopack } = nextTestSetup({
files: new FileRef(path.join(__dirname, 'fixtures', 'default-template')),
dependencies: {
react: 'latest',
Expand Down Expand Up @@ -143,17 +143,32 @@ describe.each(['default', 'turbo'])('Error recovery app %s', () => {
).toBe('1')

await session.waitForAndOpenRuntimeError()
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"index.js (7:10) @ eval

5 | const increment = useCallback(() => {
6 | setCount(c => c + 1)
> 7 | throw new Error('oops')
| ^
8 | }, [setCount])
9 | return (
10 | <main>"
`)

if (isTurbopack) {
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"index.js (7:5) @ eval

5 | const increment = useCallback(() => {
6 | setCount(c => c + 1)
> 7 | throw new Error('oops')
| ^
8 | }, [setCount])
9 | return (
10 | <main>"
`)
} else {
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"index.js (7:10) @ eval

5 | const increment = useCallback(() => {
6 | setCount(c => c + 1)
> 7 | throw new Error('oops')
| ^
8 | }, [setCount])
9 | return (
10 | <main>"
`)
}

await session.patch(
'index.js',
Expand Down
2 changes: 1 addition & 1 deletion test/turbopack-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,7 @@
},
"test/development/acceptance-app/error-recovery.test.ts": {
"passed": [
"Error recovery app turbo can recover from a event handler error",
"Error recovery app turbo can recover from a syntax error without losing state",
"Error recovery app turbo client component can recover from a component error",
"Error recovery app turbo displays build error on initial page load",
Expand All @@ -1092,7 +1093,6 @@
"Error recovery app turbo syntax > runtime error"
],
"failed": [
"Error recovery app turbo can recover from a event handler error",
"Error recovery app turbo client component can recover from syntax error",
"Error recovery app turbo server component can recover from a component error",
"Error recovery app turbo stuck error"
Expand Down
Loading