Skip to content

Commit

Permalink
Turbopack: use file:// uris for server resources in development
Browse files Browse the repository at this point in the history
Follows up from #71489, extending this to sourcemaps for server bundles.

Test Plan:

- [ ] CI
- [x] https://github.com/vercel/next.js/blob/61dbe6fb00885068e7ae89ed397e2112076a4577/test/development/app-dir/source-mapping/README.md
  • Loading branch information
wbinnssmith committed Oct 29, 2024
1 parent c8f7831 commit 19d9976
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 69 deletions.
21 changes: 15 additions & 6 deletions crates/next-core/src/next_server/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ pub async fn get_server_chunking_context_with_client_assets(
// TODO(alexkirsz) This should return a trait that can be implemented by the
// different server chunking contexts. OR the build chunking context should
// support both production and development modes.
Ok(NodeJsChunkingContext::builder(
let mut builder = NodeJsChunkingContext::builder(
project_path,
node_root,
client_root,
Expand All @@ -929,8 +929,12 @@ pub async fn get_server_chunking_context_with_client_assets(
)
.asset_prefix(asset_prefix)
.minify_type(next_mode.minify_type())
.module_id_strategy(module_id_strategy)
.build())
.module_id_strategy(module_id_strategy);

if next_mode.is_development() {
builder = builder.use_file_source_map_uris();
}
Ok(builder.build())
}

#[turbo_tasks::function]
Expand All @@ -945,7 +949,7 @@ pub async fn get_server_chunking_context(
// TODO(alexkirsz) This should return a trait that can be implemented by the
// different server chunking contexts. OR the build chunking context should
// support both production and development modes.
Ok(NodeJsChunkingContext::builder(
let mut builder = NodeJsChunkingContext::builder(
project_path,
node_root,
node_root,
Expand All @@ -955,6 +959,11 @@ pub async fn get_server_chunking_context(
next_mode.runtime_type(),
)
.minify_type(next_mode.minify_type())
.module_id_strategy(module_id_strategy)
.build())
.module_id_strategy(module_id_strategy);

if next_mode.is_development() {
builder = builder.use_file_source_map_uris()
}

Ok(builder.build())
}
97 changes: 35 additions & 62 deletions test/e2e/app-dir/server-source-maps/server-source-maps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,19 @@ describe('app-dir - server source maps', () => {
if (isNextDev) {
await retry(() => {
expect(normalizeCliOutput(next.cliOutput)).toContain(
isTurbopack
? '\nError: Boom' +
'\n at logError (turbopack://[project]/app/rsc-error-log/page.js:2:16)' +
'\n at Page (turbopack://[project]/app/rsc-error-log/page.js:7:2)' +
'\n 1 | function logError() {' +
"\n> 2 | const error = new Error('Boom')" +
'\n | ^' +
'\n 3 | console.error(error)' +
'\n 4 | }' +
'\n 5 |' +
'\n'
: '\nError: Boom' +
'\n at logError (app/rsc-error-log/page.js:2:16)' +
// FIXME: Method name should be "Page"
'\n at logError (app/rsc-error-log/page.js:7:2)' +
'\n 1 | function logError() {' +
"\n> 2 | const error = new Error('Boom')" +
'\n | ^' +
'\n 3 | console.error(error)' +
'\n 4 | }' +
'\n 5 |' +
'\n'
'\nError: Boom' +
'\n at logError (app/rsc-error-log/page.js:2:16)' +
(isTurbopack
? '\n at Page (app/rsc-error-log/page.js:7:2)'
: // FIXME: Method name should be "Page"
'\n at logError (app/rsc-error-log/page.js:7:2)') +
'\n 1 | function logError() {' +
"\n> 2 | const error = new Error('Boom')" +
'\n | ^' +
'\n 3 | console.error(error)' +
'\n 4 | }' +
'\n 5 |' +
'\n'
)
})
} else {
Expand All @@ -66,46 +57,28 @@ describe('app-dir - server source maps', () => {
if (isNextDev) {
await retry(() => {
expect(normalizeCliOutput(next.cliOutput)).toContain(
isTurbopack
? '\nError: Boom' +
'\n at logError (turbopack://[project]/app/rsc-error-log-cause/page.js:2:16)' +
'\n at Page (turbopack://[project]/app/rsc-error-log-cause/page.js:8:2)' +
'\n 1 | function logError(cause) {' +
"\n> 2 | const error = new Error('Boom', { cause })" +
'\n | ^' +
'\n 3 | console.error(error)' +
'\n 4 | }' +
'\n 5 | {' +
'\n [cause]: Error: Boom' +
'\n at Page (turbopack://[project]/app/rsc-error-log-cause/page.js:7:16)' +
'\n 5 |' +
'\n 6 | export default function Page() {' +
"\n > 7 | const error = new Error('Boom')" +
'\n | ^' +
'\n 8 | logError(error)' +
'\n 9 | return null' +
'\n 10 | }' +
'\n'
: '\nError: Boom' +
'\n at logError (app/rsc-error-log-cause/page.js:2:16)' +
// FIXME: Method name should be "Page"
'\n at logError (app/rsc-error-log-cause/page.js:8:2)' +
'\n 1 | function logError(cause) {' +
"\n> 2 | const error = new Error('Boom', { cause })" +
'\n | ^' +
'\n 3 | console.error(error)' +
'\n 4 | }' +
'\n 5 | {' +
'\n [cause]: Error: Boom' +
'\n at Page (app/rsc-error-log-cause/page.js:7:16)' +
'\n 5 |' +
'\n 6 | export default function Page() {' +
"\n > 7 | const error = new Error('Boom')" +
'\n | ^' +
'\n 8 | logError(error)' +
'\n 9 | return null' +
'\n 10 | }' +
'\n'
'\nError: Boom' +
'\n at logError (app/rsc-error-log-cause/page.js:2:16)' +
(isTurbopack
? '\n at Page (app/rsc-error-log-cause/page.js:8:2)'
: // FIXME: Method name should be "Page"
'\n at logError (app/rsc-error-log-cause/page.js:8:2)') +
'\n 1 | function logError(cause) {' +
"\n> 2 | const error = new Error('Boom', { cause })" +
'\n | ^' +
'\n 3 | console.error(error)' +
'\n 4 | }' +
'\n 5 | {' +
'\n [cause]: Error: Boom' +
'\n at Page (app/rsc-error-log-cause/page.js:7:16)' +
'\n 5 |' +
'\n 6 | export default function Page() {' +
"\n > 7 | const error = new Error('Boom')" +
'\n | ^' +
'\n 8 | logError(error)' +
'\n 9 | return null' +
'\n 10 | }' +
'\n'
)
})
} else {
Expand Down
10 changes: 9 additions & 1 deletion turbopack/crates/turbopack-nodejs/src/chunking_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ impl NodeJsChunkingContextBuilder {
self
}

pub fn use_file_source_map_uris(mut self) -> Self {
self.chunking_context.should_use_file_source_map_uris = true;
self
}

pub fn module_id_strategy(mut self, module_id_strategy: Vc<Box<dyn ModuleIdStrategy>>) -> Self {
self.chunking_context.module_id_strategy = module_id_strategy;
self
Expand Down Expand Up @@ -92,6 +97,8 @@ pub struct NodeJsChunkingContext {
manifest_chunks: bool,
/// The strategy to use for generating module ids
module_id_strategy: Vc<Box<dyn ModuleIdStrategy>>,
/// Whether to use file:// uris for source map sources
should_use_file_source_map_uris: bool,
}

impl NodeJsChunkingContext {
Expand All @@ -117,6 +124,7 @@ impl NodeJsChunkingContext {
runtime_type,
minify_type: MinifyType::NoMinify,
manifest_chunks: false,
should_use_file_source_map_uris: false,
module_id_strategy: Vc::upcast(DevModuleIdStrategy::new()),
},
}
Expand Down Expand Up @@ -233,7 +241,7 @@ impl ChunkingContext for NodeJsChunkingContext {

#[turbo_tasks::function]
fn should_use_file_source_map_uris(&self) -> Vc<bool> {
Vc::cell(false)
Vc::cell(self.should_use_file_source_map_uris)
}

#[turbo_tasks::function]
Expand Down

0 comments on commit 19d9976

Please sign in to comment.