Skip to content

Commit c1f4e3e

Browse files
authored
Turbopack: fix sourcemaps of scopehoisted comments (#80987)
There was a bug in the source mappings for scope hoisted comments. They actually contained invalid mappings, which was only caught by the invalid mapping landing exactly on an invalid multibyte character boundary, which triggered a debug assertion: ![Bildschirmfoto 2025-06-27 um 10 01 15](https://github.com/user-attachments/assets/90cd7771-72c2-41ea-bc54-05b3be5f17f7) What's happening is: 1. You do `comments.add_leading` with a `Comment` containing the real span 2. Traverse the AST and encode all bytepos in the soans 3. codegen, the bytepos in the AST are encoded, but the ones in the comments (Comment.span) are not (i.e. via `comments.take_leading` Now, they are all consistently encoded and correctly mapped: ![Bildschirmfoto 2025-06-27 um 10 02 47](https://github.com/user-attachments/assets/90b4bb58-e946-4781-b498-d88668193107)
1 parent c910be0 commit c1f4e3e

14 files changed

+190
-4
lines changed

turbopack/crates/turbopack-ecmascript/src/lib.rs

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2416,6 +2416,21 @@ enum CodeGenResultCommentsConsumable<'a> {
24162416
unsafe impl Send for CodeGenResultComments {}
24172417
unsafe impl Sync for CodeGenResultComments {}
24182418

2419+
/// All BytePos in Spans in the AST are encoded correctly in [`merge_modules`], but the Comments
2420+
/// also contain spans. These also need to be encoded so that all pos in `mappings` are consistently
2421+
/// encoded.
2422+
fn encode_module_into_comment_span(
2423+
modules_header_width: u32,
2424+
module: usize,
2425+
mut comment: Comment,
2426+
) -> Comment {
2427+
comment.span.lo =
2428+
CodeGenResultComments::encode_bytepos(modules_header_width, module as u32, comment.span.lo);
2429+
comment.span.hi =
2430+
CodeGenResultComments::encode_bytepos(modules_header_width, module as u32, comment.span.hi);
2431+
comment
2432+
}
2433+
24192434
impl Comments for CodeGenResultCommentsConsumable<'_> {
24202435
fn add_leading(&self, _pos: BytePos, _cmt: Comment) {
24212436
unimplemented!()
@@ -2459,7 +2474,12 @@ impl Comments for CodeGenResultCommentsConsumable<'_> {
24592474
} => {
24602475
let (module, pos) =
24612476
CodeGenResultComments::decode_bytepos(*modules_header_width, pos);
2462-
comments[module].take_leading(pos)
2477+
comments[module].take_leading(pos).map(|comments| {
2478+
comments
2479+
.into_iter()
2480+
.map(|c| encode_module_into_comment_span(*modules_header_width, module, c))
2481+
.collect()
2482+
})
24632483
}
24642484
Self::Empty => None,
24652485
}
@@ -2477,7 +2497,12 @@ impl Comments for CodeGenResultCommentsConsumable<'_> {
24772497
} => {
24782498
let (module, pos) =
24792499
CodeGenResultComments::decode_bytepos(*modules_header_width, pos);
2480-
comments[module].get_leading(pos)
2500+
comments[module].get_leading(pos).map(|comments| {
2501+
comments
2502+
.into_iter()
2503+
.map(|c| encode_module_into_comment_span(*modules_header_width, module, c))
2504+
.collect()
2505+
})
24812506
}
24822507
Self::Empty => None,
24832508
}
@@ -2528,7 +2553,12 @@ impl Comments for CodeGenResultCommentsConsumable<'_> {
25282553
} => {
25292554
let (module, pos) =
25302555
CodeGenResultComments::decode_bytepos(*modules_header_width, pos);
2531-
comments[module].take_trailing(pos)
2556+
comments[module].take_trailing(pos).map(|comments| {
2557+
comments
2558+
.into_iter()
2559+
.map(|c| encode_module_into_comment_span(*modules_header_width, module, c))
2560+
.collect()
2561+
})
25322562
}
25332563
Self::Empty => None,
25342564
}
@@ -2546,7 +2576,12 @@ impl Comments for CodeGenResultCommentsConsumable<'_> {
25462576
} => {
25472577
let (module, pos) =
25482578
CodeGenResultComments::decode_bytepos(*modules_header_width, pos);
2549-
comments[module].get_leading(pos)
2579+
comments[module].get_leading(pos).map(|comments| {
2580+
comments
2581+
.into_iter()
2582+
.map(|c| encode_module_into_comment_span(*modules_header_width, module, c))
2583+
.collect()
2584+
})
25502585
}
25512586
Self::Empty => None,
25522587
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { jsx as _jsx } from "./jsx-runtime";
2+
// eslint-disable-next-line import/no-extraneous-dependencies
3+
const createFromReadableStream = 123; //import { createFromReadableStream } from 'react-server-dom-webpack/client.edge';
4+
// eslint-disable-next-line import/no-extraneous-dependencies
5+
const prerender = 123; // import { unstable_prerender as prerender } from 'react-server-dom-webpack/static.edge';
6+
const streamFromBuffer=1,streamToBuffer=1; // import { streamFromBuffer, streamToBuffer } from '../stream-utils/node-web-streams-helper';
7+
const waitAtLeastOneReactRenderTask=1; //import { waitAtLeastOneReactRenderTask } from '../../lib/scheduler';
8+
// import './segment-value-encoding';
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import './params'
2+
import './collect-segment-data'
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// "entry-base.js [app-rsc] (ecmascript) <module evaluation>" with
2+
// ["reflect-utils.js [app-rsc] (ecmascript)",
3+
// "params.js [app-rsc] (ecmascript)",
4+
// "segment-value-encoding.js [app-rsc] (ecmascript)",
5+
// "collect-segment-data.js [app-rsc] (ecmascript)",
6+
// "entry-base.js [app-rsc] (ecmascript) <locals>"]
7+
8+
// "entry-base.js [test] (ecmascript) <module evaluation>" with
9+
// ["reflect-utils.js [test] (ecmascript)",
10+
// "params.js [test] (ecmascript)",
11+
// "segment-value-encoding.js [test] (ecmascript)",
12+
// "collect-segment-data.js [test] (ecmascript)",
13+
// "entry-base.js [test] (ecmascript) <locals>"]
14+
15+
if (Date.now() > 0) {
16+
require('./index1.js')
17+
}
18+
if (Date.now() > 0) {
19+
require('./index2.js')
20+
}
21+
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import "./entry-base.js";
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import "./entry-base.js";
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module.exports = {};
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import './reflect-utils'
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// This regex will have fast negatives meaning valid identifiers may not pass
2+
// this test. However this is only used during static generation to provide hints
3+
// about why a page bailed out of some or all prerendering and we can use bracket notation
4+
// for example while `ಠ_ಠ` is a valid identifier it's ok to print `searchParams['ಠ_ಠ']`
5+
// even if this would have been fine too `searchParams.ಠ_ಠ`
6+
const isDefinitelyAValidIdentifier = /s/;
7+
export function describeStringPropertyAccess(target, prop) {}
8+
export function describeHasCheckingStringProperty(target, prop) {}
9+
export const wellKnownProperties = new Set([])
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"treeShakingMode": "reexports-only",
3+
"scopeHoisting": true
4+
}

turbopack/crates/turbopack-tests/tests/snapshot/source_maps/merged-unicode/output/4e721_crates_turbopack-tests_tests_snapshot_source_maps_merged-unicode_input_795dc95a._.js

Lines changed: 82 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

turbopack/crates/turbopack-tests/tests/snapshot/source_maps/merged-unicode/output/4e721_crates_turbopack-tests_tests_snapshot_source_maps_merged-unicode_input_795dc95a._.js.map

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
(globalThis.TURBOPACK = globalThis.TURBOPACK || []).push([
2+
"output/b1abf_turbopack-tests_tests_snapshot_source_maps_merged-unicode_input_index_a93cc521.js",
3+
{},
4+
{"otherChunks":["output/4e721_crates_turbopack-tests_tests_snapshot_source_maps_merged-unicode_input_795dc95a._.js"],"runtimeModuleIds":["[project]/turbopack/crates/turbopack-tests/tests/snapshot/source_maps/merged-unicode/input/index.js [test] (ecmascript)"]}
5+
]);
6+
// Dummy runtime

turbopack/crates/turbopack-tests/tests/snapshot/source_maps/merged-unicode/output/b1abf_turbopack-tests_tests_snapshot_source_maps_merged-unicode_input_index_a93cc521.js.map

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)