Skip to content

fix: preserve sourcemap mappings for indented lines when a CJS module shares the chunk#10074

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/sourcemap-cjs-10070
Jul 2, 2026
Merged

fix: preserve sourcemap mappings for indented lines when a CJS module shares the chunk#10074
graphite-app[bot] merged 1 commit into
mainfrom
fix/sourcemap-cjs-10070

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 1, 2026

Copy link
Copy Markdown
Member

Workaround fix for #10070

Problem. When a CJS module shares a chunk with concatenated ESM modules, the ESM modules lose sourcemap mappings for their indented lines (globalThis.side = 1 / return 42 in the repro) — only when the CJS file is present.

Cause. A CJS module's require-init is prepended into a concatenated ESM module as a MagicString mutation, and NormalModule::render built that mutation map with the default Hires::False (one token per line, at column 0). collapse_sourcemaps composes by walking the last map's tokens and looking each up in the codegen map per line; the lookup returns None (dropping the token) when the line has no token at column ≤ the query. For indented lines the codegen token sits after the indent, so the column-0 query misses.

Fix. Build the mutation map with Hires::Boundary (a token at every identifier/punctuation boundary — a superset of codegen's columns), so the collapse always finds a match.

Not the root fix. Hires only controls maps rolldown generates itself; the same drop still happens when collapse_sourcemaps walks a coarse map from a plugin's renderChunk/transform hook. The real fix belongs in oxc_sourcemap: approximate to the line's nearest token on a same-line miss instead of returning None, as Rollup's traceSegment already does.

Testing. Added sourcemap/issue_10070; app.js body lines now map correctly. Only other snapshot change is cjs_compat/mix-cjs-esm — coarse per-line positions become accurate columns (strict improvement).

@netlify

netlify Bot commented Jul 1, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 047a136
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a45c5ff17ad320008c2b3b1

@hyfdev
hyfdev force-pushed the fix/sourcemap-cjs-10070 branch from 839b007 to e9fe506 Compare July 1, 2026 16:52
@hyfdev
hyfdev marked this pull request as ready for review July 2, 2026 01:19
Copilot AI review requested due to automatic review settings July 2, 2026 01:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a sourcemap correctness bug in rolldown’s Rust core when a CJS module shares a chunk with concatenated ESM modules, causing mappings for indented ESM lines to be dropped. The fix increases the sourcemap “hires” granularity used for MagicString mutation maps so collapse_sourcemaps can reliably find same-line tokens during composition, and adds a regression test covering issue #10070.

Changes:

  • Generate MagicString mutation sourcemaps with string_wizard::Hires::Boundary in NormalModule::render to preserve mappings for indented lines during sourcemap collapsing.
  • Add a new sourcemap/issue_10070 integration test fixture + snapshot with sourcemap visualization enabled.
  • Update the cjs_compat/mix-cjs-esm snapshot to reflect improved (more accurate) column mappings.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/rolldown_common/src/module/normal_module.rs Uses Hires::Boundary when generating mutation sourcemaps so collapse_sourcemaps doesn’t drop indented-line mappings.
crates/rolldown/tests/rolldown/sourcemap/issue_10070/_config.json Enables sourcemap visualization for the new regression test.
crates/rolldown/tests/rolldown/sourcemap/issue_10070/main.js New regression test entry that imports CJS + ESM and exposes values on globalThis.
crates/rolldown/tests/rolldown/sourcemap/issue_10070/dep.cjs New CJS dependency to reproduce the mixed-chunk condition.
crates/rolldown/tests/rolldown/sourcemap/issue_10070/app.js New ESM module with indented lines that previously lost mappings.
crates/rolldown/tests/rolldown/sourcemap/issue_10070/artifacts.snap Snapshot asserting the generated output and visualized sourcemap mappings for the repro.
crates/rolldown/tests/rolldown/cjs_compat/mix-cjs-esm/artifacts.snap Snapshot update reflecting the improved column-accurate mappings.

Comment thread crates/rolldown_common/src/module/normal_module.rs
@codspeed-hq

codspeed-hq Bot commented Jul 2, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/sourcemap-cjs-10070 (e9fe506) with main (6cbd233)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

hyfdev commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Merge activity

… shares the chunk (#10074)

Workaround fix for #10070

**Problem.** When a CJS module shares a chunk with concatenated ESM modules, the ESM modules lose sourcemap mappings for their indented lines (`globalThis.side = 1` / `return 42` in the repro) — only when the CJS file is present.

**Cause.** A CJS module's require-init is prepended into a concatenated ESM module as a MagicString mutation, and `NormalModule::render` built that mutation map with the default `Hires::False` (one token per line, at column 0). `collapse_sourcemaps` composes by walking the last map's tokens and looking each up in the codegen map per line; the lookup returns `None` (dropping the token) when the line has no token at column ≤ the query. For indented lines the codegen token sits after the indent, so the column-0 query misses.

**Fix.** Build the mutation map with `Hires::Boundary` (a token at every identifier/punctuation boundary — a superset of codegen's columns), so the collapse always finds a match.

**Not the root fix.** `Hires` only controls maps rolldown generates itself; the same drop still happens when `collapse_sourcemaps` walks a coarse map from a plugin's `renderChunk`/`transform` hook. The real fix belongs in `oxc_sourcemap`: approximate to the line's nearest token on a same-line miss instead of returning `None`, as Rollup's `traceSegment` already does.

**Testing.** Added `sourcemap/issue_10070`; app.js body lines now map correctly. Only other snapshot change is `cjs_compat/mix-cjs-esm` — coarse per-line positions become accurate columns (strict improvement).
@graphite-app
graphite-app Bot force-pushed the fix/sourcemap-cjs-10070 branch from e9fe506 to 047a136 Compare July 2, 2026 01:59
@graphite-app
graphite-app Bot merged commit 047a136 into main Jul 2, 2026
34 checks passed
@graphite-app
graphite-app Bot deleted the fix/sourcemap-cjs-10070 branch July 2, 2026 02:04
hyfdev added a commit that referenced this pull request Jul 2, 2026
…es::Boundary workaround

Switch `collapse_sourcemaps` to `lookup_source_view_token_approx`
(oxc-project/oxc-sourcemap#392): when a token's position falls before the
first token on its line in the previous map (an indented line), clamp to that
first token instead of dropping the mapping — the same approximation Rollup's
`collapseSourcemaps` does. This fixes the dropped-mapping class behind #10070
across all collapse paths, so the module-level `Hires::Boundary` workaround
from #10074 is removed.

Snapshot changes:
- 5 fixtures recover previously-dropped mappings (topics/deconflict/*,
  module_types/copy/*, misc/wrapped_esm).
- The two mutation-path fixtures (issue_10070, mix-cjs-esm) move from
  column-level to line-level source precision, since the mutation map is no
  longer hi-res.

Depends on oxc-project/oxc-sourcemap#392, pulled via [patch.crates-io] git
dependency; not for merge until that lands in a published oxc_sourcemap.
@rolldown-guard rolldown-guard Bot mentioned this pull request Jul 8, 2026
shulaoda added a commit that referenced this pull request Jul 8, 2026
## [1.1.5] - 2026-07-08

### 🚀 Features

- detect top-level import-binding reads as execution-order sensitive (#10180) by @hyf0
- sourcemap_filenames: add a sourcemapFileNames option (#9271) by @V1OL3TF0X
- binding: record plugin hook result kind in tracing spans (#10154) by @IWANABETHATGUY
- linking: skip side-effect-free modules in per-entry reachability (#10111) by @IWANABETHATGUY
- improve error message for unresolved virtual imports (#10156) by @sapphi-red
- add descriptive metadata to plugin API (#10106) by @sapphi-red
- add `--configLoader=native` option (#10118) by @sapphi-red

### 🐛 Bug Fixes

- improve invalid annotation warnings (#10185) by @hyf0
- keep deduplicated asset filenames stable once they can be observed (#10191) by @shulaoda
- sourcemap_filenames: use public option name in pattern errors (#10188) by @IWANABETHATGUY
- sourcemap_filenames: hash prepared sourcemap content (#10178) by @hyf0
- tree-shake unused circular declarators exported via export list (#10166) by @IWANABETHATGUY
- dev: don't panic when an HMR rebuild hits an unresolved import (#10162) by @shulaoda
- propagate errors from output.globals function (#9880) by @shulaoda
- dev: revert cache mutations when a partial scan fails (#10110) by @shulaoda
- dev: update importer relationships of cached modules in incremental build (#10107) by @shulaoda
- hmr: fall back to full reload when a changed module is not registered as executed (#10132) by @shulaoda
- chunk-optimizer: follow entry facade edges in runtime placement cycle check (#10101) by @hyf0
- dev: ignore watcher events after close (#10113) by @hyf0
- emit async wrapper for TLA modules under onDemandWrapping (#10086) by @IWANABETHATGUY
- gate sideEffects:false modules' side effects on body demand (#10080) by @IWANABETHATGUY
- rolldown_plugin_vite_resolve: return empty object for `browser: false` mapped modules (#10082) by @sapphi-red
- reset the word-boundary state on newline in Hires::Boundary sourcemaps (#10025) by @shulaoda
- trim an emptied chunk's outro/intro instead of skipping past it (#10029) by @shulaoda
- test each edited chunk's own start against indent exclude ranges (#10026) by @shulaoda
- preserve sourcemap mappings for indented lines when a CJS module shares the chunk (#10074) by @hyf0

### 🚜 Refactor

- separate tree-shaking side effects from execution order sensitivity (#10168) by @hyf0
- type construct_vite_preload_call to take an ObjectPattern (#10135) by @shulaoda
- treeshake: single-source the own-export classification shared with the lazy-barrel loader (#10098) by @IWANABETHATGUY
- dev: reuse Vite's bundledDev server (#10081) by @h-a-n-a
- clippy: ban std HashMap/HashSet in favour of FxHashMap/FxHashSet (#10108) by @Boshen
- treeshake: make body demand a second module bit instead of a stmt multimap (#10097) by @IWANABETHATGUY
- seal used_symbol_refs by construction after its last writer (#10091) by @hyf0
- treeshake: replace inclusion mutual recursion with a worklist engine (#10096) by @IWANABETHATGUY
- treeshake: split include_statements.rs into focused modules (#10095) by @IWANABETHATGUY
- drop redundant is_user_defined filter on partitioned entries (#10050) by @shulaoda
- project the retained export interface out of used_symbol_refs (#10089) by @hyf0
- track used external symbols separately from used_symbol_refs (#10088) by @hyf0
- make module namespace inclusion an explicit linking metadata field (#10087) by @hyf0
- rename statement evaluation metadata (#10078) by @hyf0

### 📚 Documentation

- virtual modules user-facing id convention (#10155) by @sapphi-red
- cli: clarify disabling boolean/object flags like codeSplitting (#10153) by @IWANABETHATGUY
- chore: remove Vite+ alpha banner (#10105) by @mdong1909
- write down the used_symbol_refs contract (#10090) by @hyf0
- dev/lazy: update design and implementation (#10079) by @h-a-n-a

### ⚡ Performance

- ast_scanner: stop order-sensitivity checks once a module is flagged (#10190) by @IWANABETHATGUY
- return impl ExactSizeIterator from slice-backed accessors (#10133) by @Boshen
- binding: box dev and watcher napi futures (#10103) by @Boshen

### 🧪 Testing

- move string_wizard replace unit tests to the JS magic-string suite (#10176) by @IWANABETHATGUY
- dev: assert incremental scan state matches a fresh full build after each HMR step (#10115) by @shulaoda
- dev: restore runtime assertions of delete_file_not_used_anymore (#10112) by @shulaoda
- dev: fix flaky dev server tests in CI (#10152) by @h-a-n-a
- add regression test for #10099 (lazyBarrel drops default-import binding but keeps its property reads) (#10109) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- deploy website to Void via GitHub OIDC (#10192) by @Boshen
- deps: update oxc to 0.139.0 (#10161) by @shulaoda
- deps: update test262 submodule for tests (#10160) by @rolldown-guard[bot]
- rolldown_plugin_utils: remove dead asset-url and css scaffolding (#10131) by @shulaoda
- deps: revert vite-plus to v0.2.1 (#10148) by @shulaoda
- deps: update github actions (#10141) by @renovate[bot]
- deps: update dependency rust to v1.96.1 (#10145) by @renovate[bot]
- deps: update npm packages (#10142) by @renovate[bot]
- deps: update rust crates (#10143) by @renovate[bot]
- deps: update napi to v3.10.3 (#10121) by @renovate[bot]
- rolldown_utils: remove unused time module (#10138) by @shulaoda
- remove dead CopyModulePlugin::is_active method (#10129) by @shulaoda
- remove dead LazyCompilationContext::is_lazy_module method (#10128) by @shulaoda
- remove dead BuildDiagnostic::downcast_ref method (#10127) by @shulaoda
- deps: update dependency vite-plus to v0.2.2 (#10084) by @renovate[bot]
- deps: update rust crate oxc_sourcemap to v8.1.0 (#10122) by @renovate[bot]
- deps: update crate-ci/typos action to v1.48.0 (#10124) by @renovate[bot]
- enable more clippy restriction lints (#10114) by @Boshen
- deps: update rust dependencies (#10100) by @Boshen
- deps: update oxc resolver to v11.23.0 (#10083) by @renovate[bot]

### ◀️ Revert

- Revert "chore(deps): revert vite-plus to v0.2.1" (#10157) by @h-a-n-a
- "fix(hmr): fall back to full reload when a changed module is not registered as executed (#10132)" (#10151) by @shulaoda

### ❤️ New Contributors

* @V1OL3TF0X made their first contribution in [#9271](#9271)

Co-authored-by: shulaoda <[email protected]>
graphite-app Bot pushed a commit that referenced this pull request Jul 15, 2026
refs #10070.

## The behavior to review

This PR makes one choice: when a coarse outer map points to a column before the first mapping on the same line of a more detailed inner map, use that first same-line mapping instead of dropping the generated position.

Rollup makes the same choice when tracing segments. The fallback never crosses a line.

## Concrete before and after

### User input

`main.js` is the source written by the user:

```js
export function app() {
  globalThis.side = 1;
  return 42;
}
```

### Generated output

Rolldown produces this chunk. Region comments and the `sourceMappingURL` line are omitted here, but the code shape is otherwise the fixture's real output:

```js
function app() {
	globalThis.side = 1;
	return 42;
}
export { app };
```

The generated JavaScript is identical before and after this PR. Only its source map changes.

### Before this PR

Asking the composed map where the two generated statements came from returns no original position:

```text
generated 3:1  globalThis.side = 1  ──X──>  null
generated 4:1  return 42             ──X──>  null
```

In a source-map visualizer, these generated statements have no colored counterpart in the Original pane. A debugger or stack-trace consumer cannot take these positions back to `main.js`.

### After this PR

The same generated positions resolve to the corresponding user-written statements:

```text
Generated output                              User input: main.js

3:1  globalThis.side = 1  ─────────────────>  2:2  globalThis.side = 1
4:1  return 42             ─────────────────>  3:2  return 42
```

Source-map consumer lines are one-based and columns are zero-based. These are the actual positions asserted by the integration fixture.

In a visualizer, each generated statement now shares a color with its corresponding statement in the Original pane. Selecting either side can identify the other side. Debuggers and stack traces can return to the correct original line; the column is the best same-line position available from the coarse map.

## Why the lookup changes

The map chain contains two different levels of detail:

```text
detailed map:  intermediate column 2 -> user source
coarse map:    final column 0        -> intermediate column 0
```

Before:

```text
final column 0
  -> intermediate column 0
  -> detailed map has no mapping at or before column 0
  -> drop the generated position
```

After:

```text
final column 0
  -> intermediate column 0
  -> detailed map's first mapping on this line is column 2
  -> use column 2 and preserve the generated position
```

Lookups between or after existing mappings still use the nearest mapping at or before the requested column. Missing and empty lines still fail. One-field explicitly unmapped segments are skipped in this PR, matching Rollup; #10254 separately preserves those boundaries.

#10074 remains necessary because it makes Rolldown-owned mutation maps more precise. This PR covers coarse external maps and other cases where the producer did not provide matching columns.

## Why the snapshots change

The snapshot visualizer writes mappings as:

```text
(original line:column) "original text" --> (generated line:column) "generated text"
```

Unlike the source-map consumer positions above, these snapshot coordinates are zero-based for both lines and columns. The three snapshots only add mappings; no existing mapping is removed or changed.

| Snapshot | Newly restored generated mapping | Previous first mapping on that generated line | Original position used |
| --- | --- | --- | --- |
| `misc/wrapped_esm` | `60:1 "("`, `60:2 "{"` | `60:3 "e} = "` | `foo.js 4:4` |
| `misc/wrapped_esm` | `61:1 "("`, `61:2 "{"` | `61:3 "h: "` | `foo.js 7:2` |
| `misc/wrapped_esm` | `73:1 "("`, `73:2 "{"` | `73:3 "destructuring} = "` | `foo.js 30:6` |
| `wrapped_esm_default_function` | `19:0 "init_foo("` | `19:9 ");"` | `bar.js 0:23` |
| `wrapped_esm_export_named_function` | `18:0 "init_foo("` | `18:9 ");"` | `bar.js 0:27` |

There are eight added rows in total: six in `misc/wrapped_esm`, one in each deconflict snapshot. Every addition follows the behavior under review:

```text
new generated column < previous first mapped column on that line
new original position = previous first mapping's original position
```

For example:

```diff
+(0:23) ";\n" --> (19:0) "init_foo("
 (0:23) ";\n" --> (19:9) ");\n"
```

The coarse map requested generated column 0, while the detailed map's first mapping was at column 9. The fallback associates column 0 with the same original position already used at column 9. It does not move or replace the existing mapping.

## Suggested review order

1. Review the lookup change and `source_id` guards in `crates/rolldown_sourcemap/src/lib.rs`.
2. Review the Rust test where a column-0 coarse token meets a column-2 detailed token.
3. Review the `renderChunk` fixture, which exercises the public plugin path and asserts the exact positions shown above.
4. Check the snapshot table against the eight added snapshot rows.

## User impact

- generated JavaScript, runtime behavior, and bundle size do not change
- users of source maps regain original locations for lines previously dropped during composition
- the change matters when a plugin or another transform provides a less column-detailed map than Rolldown's preceding map
- already resolvable mappings are unchanged

## Validation

- `cargo test -p rolldown_sourcemap`
- `just test-rust`
- `just test-node-rolldown-only fixtures-concurrent.test.ts -t plugin/render-chunk/coarse-sourcemap-composition`
- `cargo clippy -p rolldown_sourcemap --all-targets -- --deny warnings`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants