Repository navigation
fix(core): ignore source map directives in JavaScript strings - #15969
LingyuCoder merged 2 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes core source-map extraction semantics by integrating full SWC parsing and altering directive-removal logic, which warrants human verification of the parser-interaction edge cases and performance impact on large vendor bundles.
Review effort: Balanced
Findings: None
What changed in this PR
This PR fixes issue #15950, where extractSourceMap incorrectly treated a sourceMappingURL substring inside a JavaScript template literal (Angular's dom_renderer.ts) as a real source-map directive, causing Rspack to try to open a literal ${resolvedSourceMapUrl} path and emit a missing-file warning. The fix cross-checks regex matches against real comment positions obtained from the SWC parser, so directives embedded in strings, template literals, and regex literals are ignored, while genuine comment directives are still extracted. It lives in the core extract_source_map utility, which is the loader-side path that resolves and inlines external source maps.
Changes:
- For JavaScript, gate the extraction regex by parsing with SWC and keeping only matches whose start offset coincides with an actual comment start; non-JS inputs and parse failures fall back to the existing regex-only behavior.
- Switch comment removal from a first-occurrence string replace (
cow_replace) to removing the exact matched byte range (replace_range), so an identical directive inside a string is preserved. - Add a
configCasesfixture covering Angular-style interpolation, multiline/nested/quoted/escaped string text, a regex literal, a real external map, and an identical directive preserved inside a string.
| File | Description |
|---|---|
| crates/rspack_core/src/utils/extract_source_map.rs | Adds javascript_comment_starts + range-aware extraction, filters regex matches to real comment positions, and removes directives by exact byte range. |
| tests/rspack-test/configCases/source-map/extract-source-map-template-literal/rspack.config.ts | New config enabling extractSourceMap for the fixture (target node, source-map devtool). |
| tests/rspack-test/configCases/source-map/extract-source-map-template-literal/strings.js | Fixture strings with in-string/regex/template-literal directives that must stay unchanged. |
| tests/rspack-test/configCases/source-map/extract-source-map-template-literal/mapped.js | Fixture with an in-string directive plus a real trailing directive to be extracted. |
| tests/rspack-test/configCases/source-map/extract-source-map-template-literal/mapped.js.map | External source map referencing original.js, used to assert real extraction. |
| tests/rspack-test/configCases/source-map/extract-source-map-template-literal/index.js | Test entry asserting in-string directives are preserved and the real comment is extracted. |
Files not reviewed (1)
- tests/rspack-test/configCases/source-map/extract-source-map-template-literal/mapped.js: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Merging this PR will not alter performance
Comparing 
Footnotes
|
Motivation
Closes #15950
With
extractSourceMapenabled, source-map text in Angular-style template literals can trigger a missing-file lookup. Extraction must also leave identical directive text inside a string intact.Changes
Replace full-program parsing and comment-position collection with a borrowed reverse scan of the trailing comment suffix, following ECMA-426 extraction without parsing. There is no AST, collected line vector, comment-position set, or new dependency.
Keep the existing URL/whitespace compatibility and JavaScript/CSS block directives, with guards against earlier block terminators, literal delimiters and ambiguous opening markers. Remove only the selected byte range; the public Rust result and function signature remain unchanged.
The config/runtime regression covers original template, nested/quoted/escaped string and regex cases, exact literal preservation, real JS/CSS maps with independently asserted sources, BOM whitespace, all five JavaScript line terminators, and nested block-marker/terminal-regex boundaries. Line-terminator resources use the existing VirtualModulesPlugin so Git normalization cannot change their bytes. Mapped functions prevent constant optimization from hiding map assertions.
Compatibility limit: quoted/backtick candidate comments and ambiguous block tails stop extraction. A quoted malformed directive no longer yields a truncated URL. An otherwise-valid directive followed by a quoted ordinary block, or after unmatched raw
/*text in an earlier literal, can remain unextracted. A quoted ordinary block before a final unambiguous directive is accepted. Original source stays intact rather than risking removal across literal or executable text.Validation
rs test --project base -t extract-source-map-template-literal: 2 passed, 4.71s (the explicit filter skips the other cases).rs test --project base: 43 files passed; 9,048 tests passed, 4 skipped; 1m 29s.cargo test -p rspack_core utils::extract_source_map::tests -- --nocapture: 3 passed, 275 filtered.cargo check --workspace --all-targets --lockedand repositorycargo lint: passed with two build jobs, run separately from JS tests.cargo fmt --all --check, scoped config formatting/check and stagedgit diff --check: passed. Ordinary fixture ignores apply; the scoped JS check is not fixture-JS lint coverage.Cargo emitted the existing build-script
rustc-cdylib-link-argwarnings also present before this follow-up. No warning suppression or test-timeout/assertion changes were made. These are local results forbc93abe21e2d; upstream CI/review remain pending.The old implementation failed the actual config/runtime no-warning regression. Review-added cases also reproduced native parsing corruption and changed template-literal contents before repair. Fixture/tool errors and the optimized-constant BOM observation are not counted as production RED evidence.