feat(rspeedy): add css sourcemap option#2442
Conversation
🦋 Changeset detectedLatest commit: 019c264 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughAdds an optional Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds support for emitting CSS sourcemaps via output.sourceMap.css in the Rspeedy config, aligning config typing/validation with expected bundler behavior and extending integration tests to cover CSS map emission.
Changes:
- Extend
SourceMapconfig to includecss?: booleanand update extracted API docs. - Add config validation + d.ts type tests for the new
output.sourceMap.cssoption. - Update plugin-react sourcemap fixture/tests to include CSS and assert CSS maps are only emitted when enabled; add a changeset entry.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/rspeedy/plugin-react/test/sourcemap.test.ts | Refactors sourcemap tests and adds assertions for optional CSS sourcemap emission. |
| packages/rspeedy/plugin-react/test/fixtures/sourcemap/index.tsx | Imports fixture CSS to produce a CSS output for sourcemap verification. |
| packages/rspeedy/plugin-react/test/fixtures/sourcemap/index.css | New fixture stylesheet used to validate CSS sourcemap mapping. |
| packages/rspeedy/core/test/config/validate.test.ts | Adds validation coverage for output.sourceMap boolean/object forms including css. |
| packages/rspeedy/core/test/config/output.test-d.ts | Adds type-level assertions for output.sourceMap.css and rejects non-boolean values. |
| packages/rspeedy/core/src/config/output/source-map.ts | Introduces the css option in the public SourceMap interface with docs. |
| packages/rspeedy/core/etc/rspeedy.api.md | Updates extracted public API surface to include SourceMap.css. |
| .changeset/add-css-sourcemap-option.md | Documents the new option and bumps @lynx-js/rspeedy with a patch changeset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rspeedy/plugin-react/test/sourcemap.test.ts (1)
5-6:⚠️ Potential issue | 🟠 MajorReplace
fs/promises.globwith a Node.js 18+ compatible alternative.The package declares
"engines": { "node": ">=18" }butfs/promises.globwas introduced in Node.js v22.0.0. The ESLint suppression masks a real runtime error on Node 18–21. Remove the suppression and either update the package's engines field to"node": "^22 || ^24"to match the root requirement, or use a compatible glob utility (e.g.,globnpm package) that works across all declared Node versions. Per coding guidelines, ESLint rules must be followed, not suppressed to hide incompatibilities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/plugin-react/test/sourcemap.test.ts` around lines 5 - 6, The current import uses fs/promises.glob (seen on the import line importing glob, mkdtemp, readFile) which is only available in Node 22+, so remove the ESLint suppression and replace the use of fs/promises.glob with a Node‑18 compatible glob utility: install the "glob" npm package, import its promise API (or promisify glob) instead of destructuring glob from 'node:fs/promises', keep mkdtemp and readFile from 'node:fs/promises' as-is, and update any code referencing the old glob symbol to call the new glob promise function; alternatively, if you prefer to require Node ≥22, update the package's engines field to match (e.g., "^22 || ^24") and remove the suppression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/add-css-sourcemap-option.md:
- Around line 12-16: The example places sourceMap at the config root; update the
snippet so sourceMap is nested under output by moving the sourceMap block inside
an output object (i.e., use defineConfig with output.sourceMap.css: true) so
enabling CSS sourcemaps actually works; adjust the example to reference output
and the sourceMap.css option rather than a top-level sourceMap.
In `@packages/rspeedy/plugin-react/test/sourcemap.test.ts`:
- Around line 111-123: The helper getSourceMapFiles collects .map paths in
traversal order which can vary; update getSourceMapFiles to sort the resulting
sourceMapFiles array (e.g., lexicographically) before returning so tests
comparing the full array in order are deterministic; locate the function
getSourceMapFiles and add a stable sort on sourceMapFiles (or use .sort()) just
prior to returning the array.
- Around line 169-180: The test passes a 1-based generated column to
SourceMapConsumer.originalPositionFor, causing wrong mappings; update the
cssColumn computation used for cssConsumer.originalPositionFor so it uses the
zero-based index from .indexOf('.app') (remove the +1) and pass that zero-based
column into cssConsumer.originalPositionFor while keeping the line (cssLine) as
1-based; adjust the same for any analogous js/other source column calculations
(referencing cssColumn and cssConsumer.originalPositionFor).
---
Outside diff comments:
In `@packages/rspeedy/plugin-react/test/sourcemap.test.ts`:
- Around line 5-6: The current import uses fs/promises.glob (seen on the import
line importing glob, mkdtemp, readFile) which is only available in Node 22+, so
remove the ESLint suppression and replace the use of fs/promises.glob with a
Node‑18 compatible glob utility: install the "glob" npm package, import its
promise API (or promisify glob) instead of destructuring glob from
'node:fs/promises', keep mkdtemp and readFile from 'node:fs/promises' as-is, and
update any code referencing the old glob symbol to call the new glob promise
function; alternatively, if you prefer to require Node ≥22, update the package's
engines field to match (e.g., "^22 || ^24") and remove the suppression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 77a1206f-c53b-4d7a-b491-8f91d18d90de
📒 Files selected for processing (8)
.changeset/add-css-sourcemap-option.mdpackages/rspeedy/core/etc/rspeedy.api.mdpackages/rspeedy/core/src/config/output/source-map.tspackages/rspeedy/core/test/config/output.test-d.tspackages/rspeedy/core/test/config/validate.test.tspackages/rspeedy/plugin-react/test/fixtures/sourcemap/index.csspackages/rspeedy/plugin-react/test/fixtures/sourcemap/index.tsxpackages/rspeedy/plugin-react/test/sourcemap.test.ts
Merging this PR will improve performance by 11.56%
Performance Changes
Comparing Footnotes
|
React External#501 Bundle Size — 580.35KiB (0%).019c264(current) vs 95c2dc3 main#499(baseline) Bundle metrics
|
| Current #501 |
Baseline #499 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch luhc228:hc/css-sourcemap-option Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7384 Bundle Size — 223.33KiB (0%).019c264(current) vs 95c2dc3 main#7382(baseline) Bundle metrics
|
| Current #7384 |
Baseline #7382 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.48% |
44.48% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7384 |
Baseline #7382 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
77.58KiB |
77.58KiB |
Bundle analysis report Branch luhc228:hc/css-sourcemap-option Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#517 Bundle Size — 193.94KiB (0%).019c264(current) vs 95c2dc3 main#515(baseline) Bundle metrics
|
| Current #517 |
Baseline #515 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
43.94% |
43.94% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #517 |
Baseline #515 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
82.71KiB |
82.71KiB |
Bundle analysis report Branch luhc228:hc/css-sourcemap-option Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#8958 Bundle Size — 749.55KiB (0%).019c264(current) vs 95c2dc3 main#8956(baseline) Bundle metrics
Bundle size by type
|
| Current #8958 |
Baseline #8956 |
|
|---|---|---|
401.63KiB |
401.63KiB |
|
345.76KiB |
345.76KiB |
|
2.16KiB |
2.16KiB |
Bundle analysis report Branch luhc228:hc/css-sourcemap-option Project dashboard
Generated by RelativeCI Documentation Report issue
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Hengchang Lu <luhengchang228@126.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/rspeedy/plugin-react/test/sourcemap.test.ts (1)
197-232:⚠️ Potential issue | 🟡 Minor
originalPositionFor()still receives 1-based generated columns.The
+ 1on lines 203 and 230 converts the 0-basedindexOf()result into a 1-based column.SourceMapConsumer.originalPositionFor({ line, column })expectscolumnto be 0-based (onlylineis 1-based), so these lookups are shifted past the token start and can resolve to an adjacent/wrong segment, making the CSS/JS mapping assertions (basename check + inline snapshot) sensitive to minifier/formatter changes.Suggested fix
const cssColumn = cssSource .split('\n') .at(cssLine - 1)! - .indexOf('.app') + 1 + .indexOf('.app') @@ backgroundSourceLines.forEach((lineContent, index) => { if (lineContent.includes(name)) { line = index + 1 - column = lineContent.indexOf(name) + 1 + column = lineContent.indexOf(name) } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/plugin-react/test/sourcemap.test.ts` around lines 197 - 232, The test is passing 1-based columns to SourceMapConsumer.originalPositionFor (it adds +1 to indexOf results), which is wrong because originalPositionFor expects 0-based columns; locate the cssColumn computation (the indexOf('.app') + 1 used before calling cssConsumer.originalPositionFor) and the background lookup that sets column = lineContent.indexOf(name) + 1 inside the loop (used later with consumer.originalPositionFor) and change both to use the raw 0-based indexOf result (remove the +1), keeping line numbers 1-based, and ensure any downstream code that assumes 1-based columns is adjusted accordingly so mappings resolve to the correct segment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/rspeedy/plugin-react/test/sourcemap.test.ts`:
- Around line 197-232: The test is passing 1-based columns to
SourceMapConsumer.originalPositionFor (it adds +1 to indexOf results), which is
wrong because originalPositionFor expects 0-based columns; locate the cssColumn
computation (the indexOf('.app') + 1 used before calling
cssConsumer.originalPositionFor) and the background lookup that sets column =
lineContent.indexOf(name) + 1 inside the loop (used later with
consumer.originalPositionFor) and change both to use the raw 0-based indexOf
result (remove the +1), keeping line numbers 1-based, and ensure any downstream
code that assumes 1-based columns is adjusted accordingly so mappings resolve to
the correct segment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5416ace0-4650-44c0-824b-1cc443c03726
📒 Files selected for processing (1)
packages/rspeedy/plugin-react/test/sourcemap.test.ts
Related PR: luhc228#2
Summary by CodeRabbit
New Features
Tests
Chores
Checklist