feat(css): enable sourcemaps and remap diagnostics#2483
feat(css): enable sourcemaps and remap diagnostics#2483luhc228 merged 1 commit intolynx-family:mainfrom
Conversation
🦋 Changeset detectedLatest commit: a982b42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEnabled CSS source maps by default in Rspeedy; added location-preserving debundling and CSSSourceMap types in css-serializer; extended cssChunksToMap to accept chunk objects; added CSS diagnostics extraction, remapping, and deduplication in the template-webpack-plugin; updated tests, snapshots, and changesets. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds CSS source map plumbing across Rspeedy, the CSS serializer, and the template webpack plugin to support source-mapped CSS diagnostics and enables CSS source maps by default in Rspeedy’s output config.
Changes:
- Default
output.sourceMap.csstotruein Rspeedy and document/validate config behavior with tests. - Preserve bundle-space CSS
locdata through debundling so diagnostics can later be remapped using the main CSS source map. - Add template encode CSS diagnostics extraction + resolution (via
@jridgewell/trace-mapping) and corresponding tests.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds lock entries for new @jridgewell/* dependencies. |
| packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts | Adds unit tests for extracting/resolving CSS diagnostics and Tasm diagnostics. |
| packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts | Implements diagnostic extraction + source-map-based remapping helpers. |
| packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts | Passes CSS asset sourcemaps into CSS chunk mapping; updates JSDoc. |
| packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts | Converts Tasm css_diagnostics into webpack warnings with mapped locations when possible. |
| packages/webpack/template-webpack-plugin/package.json | Adds @jridgewell/trace-mapping dependency. |
| packages/tools/css-serializer/test/css.test.ts | Adds coverage for preserving bundle locations and css sourcemap-related behavior. |
| packages/tools/css-serializer/src/css/sourceMap.ts | Introduces CSSSourceMap type used across packages. |
| packages/tools/css-serializer/src/css/index.ts | Exports the new CSSSourceMap type. |
| packages/tools/css-serializer/src/css/debundle.ts | Adds optional location-preserving debundle mode built from original code segments. |
| packages/tools/css-serializer/src/css/cssChunksToMap.ts | Updates chunk input type (string or asset-like), always preserving bundle-space loc info. |
| packages/tools/css-serializer/src/css/ast.ts | Refactors AST parsing to use parse() and direct types imports. |
| packages/tools/css-serializer/package.json | Adds @jridgewell/gen-mapping for tests. |
| packages/rspeedy/core/test/config/output/source-map.test.ts | Adds tests for defaulting/merging output.sourceMap.css. |
| packages/rspeedy/core/src/config/output/source-map.ts | Documents and adds output.sourceMap.css option to the public config type. |
| packages/rspeedy/core/src/config/defaults.ts | Defaults output.sourceMap.css to true. |
| .github/css-source-map-diagnostics.instructions.md | Adds contributor guidance for keeping bundle-space loc and stable cssSource. |
| .changeset/enable-rspeedy-css-sourcemap.md | Changeset: enable CSS source maps by default in Rspeedy. |
| .changeset/add-css-source-map-diagnostics.md | Changeset: add CSS sourcemap support + source-mapped diagnostics. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts:435
- The
@examplestill shows callingconvertCSSChunksToMapwith a single string, but the method now requires an array (cssChunks: Array<string | { content; sourceMap? }>). Update the example to pass an array so it stays copy/paste-correct.
* (console.log(await convertCSSChunksToMap(
* '.red { color: red; }',
* {
* targetSdkVersion: '3.2',
* enableCSSSelector: true,
* },
* )));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aa55e18 to
fcb3d4f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bbacd7a to
c473971
Compare
c473971 to
5c3ec74
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/tools/css-serializer/test/css.test.ts (1)
442-459: Add column assertions for the preserved locations.These tests only assert
line, so a regression that resets or shiftsselectorText.loc.column,keyLoc.column, orvalLoc.columnwould still pass even though the PR is meant to preserve row and column diagnostics.Also applies to: 480-497
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tools/css-serializer/test/css.test.ts` around lines 442 - 459, The test currently only asserts line numbers for preserved locations; update the assertions in packages/tools/css-serializer/test/css.test.ts (the checks against cssMap[1]?.[0] and the similar block around the second occurrence) to also assert the column values for selectorText.loc.column, style[*].keyLoc.column, and style[*].valLoc.column so regressions that change column positions will fail; locate the selector/StyleRule assertions (selectorText, keyLoc, valLoc) and add explicit expectations for their column properties matching the intended preserved columns.packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts (1)
32-71: Consider adding coverage for webpack's namespaced source URLs.The fixture uses
webpack:/src/app.css(single slash, no namespace), but webpack/Rspack typically emitwebpack:///./src/App.cssorwebpack://<project>/./src/App.css. Adding a case that uses the namespaced three-slash form would exercisenormalizeTasmSourcePath's real-world input shape — which currently has a regex gap (see the comment oncssDiagnostics.ts). The existing tests may pass while production inputs still produce incorrectsourceFilevalues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts` around lines 32 - 71, Add a new test case to exercise webpack namespaced source URLs by creating a CSSSourceMap fixture whose sources include forms like "webpack:///./src/App.css" and "webpack://my-project/./src/App.css", then call resolveTasmCSSDiagnostics (in cssDiagnostics.ts) with that source map and assert the resolved.sourceFile points to the normalized path (e.g. /workspace/app/src/App.css); this will exercise normalizeTasmSourcePath and catch the regex gap when resolving namespaced webpack URLs.packages/tools/css-serializer/src/css/cssChunksToMap.ts (1)
30-71: Per-chunk debundling can produce synthetic ID collisions when multiple CSS chunks are present.The
reduceloop processes each chunk independently, anddebundleCSSassigns positional indices to unmarked global CSS rules starting atindex + 1within each chunk. IfcssChunkscontains multiple entries, different chunks can generate the same syntheticcssId(e.g., both chunk 1 and chunk 2 assign1to their first unmarked rule). These collide when merged into a singlecssMapat lines 42–48, and subsequent calls tocontent.join('\n')at line 55 re-parse CSS whose line positions no longer correspond to any single bundle file, invalidating thelocfields thatdebundleCSSworked to preserve.The codebase acknowledges this risk:
debundle.tsincludes a TODO noting potential ID conflicts. While webpack typically produces one main CSS chunk per entry (making this a low-risk scenario today), the implementation is fragile. Either document the single-chunk invariant or qualify synthetic IDs with chunk information to make the contract explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tools/css-serializer/src/css/cssChunksToMap.ts` around lines 30 - 71, Per-chunk synthetic cssId numbers from debundleCSS collide when merging chunks; change the merge to qualify each synthetic id with the chunk identity (e.g. prefix or join chunkIndex to the cssId) instead of using raw numeric keys. In the reduce over cssChunks (symbols: cssChunks, normalizeCSSChunk, debundleCSS, cssMap) compute a chunkKey (like `${chunkIndex}:${cssId}`) when calling cssMap.set/get and when iterating debundledMap.forEach; then adapt the later mapping that builds stylesheets (symbols: stylesheets, cssToAst, cssId, cssSource) to derive the original numeric id if needed for downstream consumers (store both the qualified id and the original numeric id in the returned object) and update cssSource to include the qualified id so the loc/positions remain consistent across chunks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rspeedy/core/src/config/defaults.ts`:
- Around line 33-35: The repo currently generates .css.map files by default via
the sourcemap plugin pattern '[file].map[query]' while package.json's "files"
only includes "dist" (so .map files may be published); decide whether source
maps should be published and if not, update the build/publish config to exclude
.map files (or adjust the sourcemap plugin usage) accordingly—look for
references to output.sourceMap, the sourcemap plugin (pattern
'[file].map[query]'), and the package.json "files" field and either filter out
*.map from published artifacts or change the plugin/config so CSS maps are not
emitted when publishing.
In `@packages/tools/css-serializer/src/css/debundle.ts`:
- Around line 211-226: The function getPositionAfterContent currently iterates
with "for (const char of content)" which advances by Unicode code points; change
it to iterate over UTF-16 code units (e.g., for (let i = 0; i < content.length;
i++)) and inspect content[i] (or content.charCodeAt(i)) so that each loop
increments column by 1 per UTF-16 code unit and treats '\n' (char code 10) as a
newline (line += 1; column = 1). Update getPositionAfterContent accordingly so
buildStylesheetFromSegments and debundleCSS (used with preserveLocations: true)
compute correct columns for supplementary-plane characters.
In `@packages/webpack/css-extract-webpack-plugin/package.json`:
- Around line 61-63: The package css-extract-webpack-plugin now narrows its
peerDependencies for "@lynx-js/template-webpack-plugin" to "^0.11.0" but lacks a
changeset entry; add a changeset that documents this peer-range change (or
update the existing changeset to include
"packages/webpack/css-extract-webpack-plugin" with a patch bump) so consumers
are notified of the peer dependency constraint—ensure the changeset references
the package name "css-extract-webpack-plugin" and the peer change to
"@lynx-js/template-webpack-plugin" ^0.11.0.
In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts`:
- Around line 132-143: normalizeTasmCSSDiagnostic currently just casts
line/column to number; update it to validate that value['line'] and
value['column'] are present and are finite numbers (e.g. typeof value['line']
=== 'number' && Number.isFinite(value['line']) and same for column) and return
null if they are missing/invalid so malformed diagnostics are dropped before
reaching originalPositionFor, dedupeTasmCSSDiagnostics, or compilation.warnings;
keep existing checks for isRecord and continue returning type/name as optional.
- Around line 145-171: normalizeTasmSourcePath currently strips only `webpack:`
or `webpack:///` but misses `webpack://<namespace>/` (two slashes), causing
namespaced sources to be mis-normalized; update the `webpack` prefix handling in
normalizeTasmSourcePath to accept both `//` and `///` (e.g. change the regex to
match `webpack://` and `webpack:///`, such as /^webpack:(?:\/\/{2,3})?/ or
/^webpack:(?:\/\/\/|\/\/)?/) so `webpack://my-app/...` is normalized correctly,
then add a unit test in cssDiagnostics.test.ts using
`webpack://my-app/./src/app.css` to prevent regression.
In `@packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts`:
- Line 112: The compilation-scoped emittedCSSDiagnosticWarnings and the use of
getMainCSSSourceMap(compilation) are too global and cause cross-template
sourcemap remapping/dedupe; update LynxEncodePlugin so that the current encode's
CSS asset names are passed into the diagnostic handling and used to both (a)
select the correct source map instead of getMainCSSSourceMap(compilation) and
(b) scope the dedupe key for emittedCSSDiagnosticWarnings (make it per-template
or per-css-asset, e.g., include the CSS asset name(s) in the Set key). Apply the
same change where emittedCSSDiagnosticWarnings is referenced and where source
maps are selected (the other occurrences around the getMainCSSSourceMap usage)
so diagnostics are chosen and deduped per-template/CSS asset.
---
Nitpick comments:
In `@packages/tools/css-serializer/src/css/cssChunksToMap.ts`:
- Around line 30-71: Per-chunk synthetic cssId numbers from debundleCSS collide
when merging chunks; change the merge to qualify each synthetic id with the
chunk identity (e.g. prefix or join chunkIndex to the cssId) instead of using
raw numeric keys. In the reduce over cssChunks (symbols: cssChunks,
normalizeCSSChunk, debundleCSS, cssMap) compute a chunkKey (like
`${chunkIndex}:${cssId}`) when calling cssMap.set/get and when iterating
debundledMap.forEach; then adapt the later mapping that builds stylesheets
(symbols: stylesheets, cssToAst, cssId, cssSource) to derive the original
numeric id if needed for downstream consumers (store both the qualified id and
the original numeric id in the returned object) and update cssSource to include
the qualified id so the loc/positions remain consistent across chunks.
In `@packages/tools/css-serializer/test/css.test.ts`:
- Around line 442-459: The test currently only asserts line numbers for
preserved locations; update the assertions in
packages/tools/css-serializer/test/css.test.ts (the checks against
cssMap[1]?.[0] and the similar block around the second occurrence) to also
assert the column values for selectorText.loc.column, style[*].keyLoc.column,
and style[*].valLoc.column so regressions that change column positions will
fail; locate the selector/StyleRule assertions (selectorText, keyLoc, valLoc)
and add explicit expectations for their column properties matching the intended
preserved columns.
In `@packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts`:
- Around line 32-71: Add a new test case to exercise webpack namespaced source
URLs by creating a CSSSourceMap fixture whose sources include forms like
"webpack:///./src/App.css" and "webpack://my-project/./src/App.css", then call
resolveTasmCSSDiagnostics (in cssDiagnostics.ts) with that source map and assert
the resolved.sourceFile points to the normalized path (e.g.
/workspace/app/src/App.css); this will exercise normalizeTasmSourcePath and
catch the regex gap when resolving namespaced webpack URLs.
🪄 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: 5512c0e6-d66b-40c0-80bb-3a8ffb907d16
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.changeset/add-css-source-map-diagnostics.md.changeset/enable-rspeedy-css-sourcemap.md.github/css-source-map-diagnostics.instructions.mdpackages/rspeedy/core/src/config/defaults.tspackages/rspeedy/core/src/config/output/source-map.tspackages/rspeedy/core/test/config/output/source-map.test.tspackages/tools/css-serializer/src/css/ast.tspackages/tools/css-serializer/src/css/cssChunksToMap.tspackages/tools/css-serializer/src/css/debundle.tspackages/tools/css-serializer/src/css/index.tspackages/tools/css-serializer/src/css/sourceMap.tspackages/tools/css-serializer/test/css.test.tspackages/webpack/css-extract-webpack-plugin/package.jsonpackages/webpack/react-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/src/LynxEncodePlugin.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/cssDiagnostics.tspackages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
Merging this PR will degrade performance by 7.97%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | transform 1000 view elements |
43.1 ms | 46.8 ms | -7.97% |
Comparing luhc228:hc/tasm-remap-css-col-and-row (a982b42) with main (d8cdd7c)
Footnotes
-
26 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. ↩
React External#648 Bundle Size — 679.93KiB (0%).a982b42(current) vs d8cdd7c main#643(baseline) Bundle metrics
|
| Current #648 |
Baseline #643 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
39.21% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch luhc228:hc/tasm-remap-css-col-an... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#662 Bundle Size — 196.39KiB (0%).a982b42(current) vs d8cdd7c main#657(baseline) Bundle metrics
|
| Current #662 |
Baseline #657 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
43.12% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
44.07% |
44.07% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #662 |
Baseline #657 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
85.15KiB |
85.15KiB |
Bundle analysis report Branch luhc228:hc/tasm-remap-css-col-an... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7530 Bundle Size — 225.23KiB (0%).a982b42(current) vs d8cdd7c main#7525(baseline) Bundle metrics
|
| Current #7530 |
Baseline #7525 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
35.05% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.57% |
44.57% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7530 |
Baseline #7525 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
79.47KiB |
79.47KiB |
Bundle analysis report Branch luhc228:hc/tasm-remap-css-col-an... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9102 Bundle Size — 899.78KiB (0%).a982b42(current) vs d8cdd7c main#9097(baseline) Bundle metrics
Bundle size by type
|
| Current #9102 |
Baseline #9097 |
|
|---|---|---|
495.64KiB |
495.64KiB |
|
401.92KiB |
401.92KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch luhc228:hc/tasm-remap-css-col-an... Project dashboard
Generated by RelativeCI Documentation Report issue
5c3ec74 to
ee1d0d3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts (1)
137-141:⚠️ Potential issue | 🟡 MinorReject non-finite diagnostic coordinates too.
NaN/Infinitystill pass the currenttypeof === 'number'check and can leak into source-map lookup orWebpackError.loc. Please make the runtime validation finite.Proposed fix
const line = value['line']; const column = value['column']; - if (typeof line !== 'number' || typeof column !== 'number') { + if ( + typeof line !== 'number' + || typeof column !== 'number' + || !Number.isFinite(line) + || !Number.isFinite(column) + ) { return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts` around lines 137 - 141, The current runtime check only tests typeof for value['line'] and value['column'] (assigned to variables line and column) which allows NaN/Infinity through; update the validation to require finite numbers (e.g., use Number.isFinite(line) && Number.isFinite(column')) and return null if either coordinate is not finite so invalid values cannot reach source-map lookup or WebpackError.loc.packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts (1)
112-112:⚠️ Potential issue | 🟠 MajorScope CSS diagnostic remapping and dedupe to the current template/CSS asset.
This still uses a compilation-wide warning set and the first
.csssource map in the compilation. In multi-entry/template builds, a diagnostic can be remapped through another CSS asset’s map or deduped against an unrelated warning. Please pass the current encode’s CSS asset name(s) into this path and include that scope in both source-map selection and the dedupe key.Also applies to: 244-250, 390-400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts` at line 112, The compilation-wide emittedCSSDiagnosticWarnings Set is causing cross-template dedupe and the source-map selection uses the first .css map in the compilation; change this to be scoped per encode's CSS asset(s): make emittedCSSDiagnosticWarnings keyed by CSS asset name (or maintain a Map<string, Set<string>>) and update the diagnostic-remapping call sites (e.g., wherever remap/emit diagnostics for an encode) to accept the current encode's cssAssetNames and use that name when checking/adding to the dedupe set; likewise, when picking a source map for remapping, select the source map that matches the current cssAssetName(s) instead of the first .css map in the compilation so remapping and dedupe occur only within the same template/CSS asset scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts`:
- Around line 160-168: The current regex that strips the webpack scheme in
cssDiagnostics.ts uses /^webpack:\/\/[^/]*\// (assigned to withoutScheme) and
fails to match single-slash inputs like "webpack:/namespace/..."; update that
regex to accept one or more slashes (e.g. /^webpack:\/+[^/]*\//) so both
"webpack://..." and "webpack:/..." are normalized, keep the subsequent
.replace(...) calls and return resolvePath(context, normalized) as before.
---
Duplicate comments:
In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts`:
- Around line 137-141: The current runtime check only tests typeof for
value['line'] and value['column'] (assigned to variables line and column) which
allows NaN/Infinity through; update the validation to require finite numbers
(e.g., use Number.isFinite(line) && Number.isFinite(column')) and return null if
either coordinate is not finite so invalid values cannot reach source-map lookup
or WebpackError.loc.
In `@packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts`:
- Line 112: The compilation-wide emittedCSSDiagnosticWarnings Set is causing
cross-template dedupe and the source-map selection uses the first .css map in
the compilation; change this to be scoped per encode's CSS asset(s): make
emittedCSSDiagnosticWarnings keyed by CSS asset name (or maintain a Map<string,
Set<string>>) and update the diagnostic-remapping call sites (e.g., wherever
remap/emit diagnostics for an encode) to accept the current encode's
cssAssetNames and use that name when checking/adding to the dedupe set;
likewise, when picking a source map for remapping, select the source map that
matches the current cssAssetName(s) instead of the first .css map in the
compilation so remapping and dedupe occur only within the same template/CSS
asset scope.
🪄 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: 2d822b0e-31ad-43b0-bde2-4e98a5a39800
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (51)
.changeset/add-css-source-map-diagnostics.md.changeset/enable-rspeedy-css-sourcemap.md.changeset/legal-phones-tell.md.changeset/tender-bees-lick.md.github/css-source-map-diagnostics.instructions.mdpackages/rspeedy/core/src/config/defaults.tspackages/rspeedy/core/src/config/output/source-map.tspackages/rspeedy/core/test/config/output/source-map.test.tspackages/tools/css-serializer/src/css/ast.tspackages/tools/css-serializer/src/css/cssChunksToMap.tspackages/tools/css-serializer/src/css/debundle.tspackages/tools/css-serializer/src/css/index.tspackages/tools/css-serializer/src/css/sourceMap.tspackages/tools/css-serializer/test/css.test.tspackages/webpack/css-extract-webpack-plugin/package.jsonpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/2.snap.txtpackages/webpack/react-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/src/LynxEncodePlugin.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/cssDiagnostics.tspackages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
✅ Files skipped from review due to trivial changes (20)
- .changeset/legal-phones-tell.md
- .changeset/tender-bees-lick.md
- .changeset/enable-rspeedy-css-sourcemap.md
- packages/webpack/react-webpack-plugin/package.json
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/0.snap.txt
- packages/webpack/template-webpack-plugin/package.json
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/1.snap.txt
- packages/tools/css-serializer/src/css/index.ts
- packages/webpack/css-extract-webpack-plugin/package.json
- .github/css-source-map-diagnostics.instructions.md
- packages/tools/css-serializer/src/css/sourceMap.ts
- packages/tools/css-serializer/test/css.test.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/snapshot/rspack/1.snap.txt
- packages/rspeedy/core/src/config/output/source-map.ts
- packages/rspeedy/core/test/config/output/source-map.test.ts
- packages/tools/css-serializer/src/css/ast.ts
- packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/0.snap.txt
🚧 Files skipped from review as they are similar to previous changes (5)
- .changeset/add-css-source-map-diagnostics.md
- packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts
- packages/rspeedy/core/src/config/defaults.ts
- packages/tools/css-serializer/src/css/debundle.ts
- packages/tools/css-serializer/src/css/cssChunksToMap.ts
ee1d0d3 to
462ca2b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts (1)
112-112:⚠️ Potential issue | 🟠 MajorScope CSS diagnostic remapping and dedupe to the current CSS assets.
getMainCSSSourceMap(compilation)still picks the first.csssourcemap in the whole compilation, whileemittedCSSDiagnosticWarningsis shared across encode calls. Multi-entry or lazy-template builds can remap diagnostics through another template’s CSS map, or dedupe distinct diagnostics. Pass the current encode’s CSS asset names into this path and include that scope in both source-map selection and the dedupe key.Also applies to: 244-250, 390-400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts` at line 112, The CSS sourcemap selection and diagnostic dedupe are global; scope them to the current encode by changing calls to getMainCSSSourceMap and the emittedCSSDiagnosticWarnings usage to accept the current encode's CSS asset names (e.g., pass the array/set of CSS asset names from the current encode invocation in LynxEncodePlugin.encode or the method that processes a template) and use that scope when selecting the source map and building the dedupe key; specifically, modify getMainCSSSourceMap(compilation) to getMainCSSSourceMap(compilation, currentCssAssetNames) and only consider sourcemaps whose asset name is in currentCssAssetNames, and change emittedCSSDiagnosticWarnings from a single Set<string> to either a Map keyed by encode id or include the current asset names in the dedupe key (e.g., `${assetName}|${originalKey}`) so warnings are deduped per-encode/current-CSS-assets rather than across the whole compilation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tools/css-serializer/src/css/cssChunksToMap.ts`:
- Around line 30-56: The merge currently concatenates debundled fragments with
content.join('\n') after debundleCSS(normalizeCSSChunk(...).content,
debundledMap, enableCSSSelector, true), which shifts preserved per-chunk
coordinates; instead preserve chunk boundaries and source locations by parsing
or remapping per-origin asset rather than as one synthetic stylesheet: keep the
original asset identifiers when calling debundleCSS (or attach origin metadata
to each fragment), then for each cssId call cssToAst per-origin fragment (or
compute exact offsets against the owning combined asset and apply those offsets
when remapping) and only then merge ASTs/remapped locations into stylesheets so
diagnostics align with original sourcemaps (refer to debundleCSS,
normalizeCSSChunk, cssToAst, cssMap and the cssChunks input).
---
Duplicate comments:
In `@packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts`:
- Line 112: The CSS sourcemap selection and diagnostic dedupe are global; scope
them to the current encode by changing calls to getMainCSSSourceMap and the
emittedCSSDiagnosticWarnings usage to accept the current encode's CSS asset
names (e.g., pass the array/set of CSS asset names from the current encode
invocation in LynxEncodePlugin.encode or the method that processes a template)
and use that scope when selecting the source map and building the dedupe key;
specifically, modify getMainCSSSourceMap(compilation) to
getMainCSSSourceMap(compilation, currentCssAssetNames) and only consider
sourcemaps whose asset name is in currentCssAssetNames, and change
emittedCSSDiagnosticWarnings from a single Set<string> to either a Map keyed by
encode id or include the current asset names in the dedupe key (e.g.,
`${assetName}|${originalKey}`) so warnings are deduped
per-encode/current-CSS-assets rather than across the whole compilation.
🪄 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: 5301a6f0-815e-4e8a-ac27-a33e53060fe1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
.changeset/add-css-source-map-diagnostics.md.changeset/enable-rspeedy-css-sourcemap.md.changeset/legal-phones-tell.md.changeset/tender-bees-lick.md.github/css-source-map-diagnostics.instructions.mdpackages/rspeedy/core/src/config/defaults.tspackages/rspeedy/core/src/config/output/source-map.tspackages/rspeedy/core/test/config/output/source-map.test.tspackages/rspeedy/core/test/plugins/output.plugin.test.tspackages/tools/css-serializer/src/css/ast.tspackages/tools/css-serializer/src/css/cssChunksToMap.tspackages/tools/css-serializer/src/css/debundle.tspackages/tools/css-serializer/src/css/index.tspackages/tools/css-serializer/src/css/sourceMap.tspackages/tools/css-serializer/test/css.test.tspackages/webpack/css-extract-webpack-plugin/package.jsonpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/2.snap.txtpackages/webpack/react-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/src/LynxEncodePlugin.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/cssDiagnostics.tspackages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
✅ Files skipped from review due to trivial changes (25)
- .github/css-source-map-diagnostics.instructions.md
- .changeset/enable-rspeedy-css-sourcemap.md
- .changeset/legal-phones-tell.md
- .changeset/tender-bees-lick.md
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/snapshot/rspack/0.snap.txt
- .changeset/add-css-source-map-diagnostics.md
- packages/webpack/react-webpack-plugin/package.json
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/snapshot/rspack/0.snap.txt
- packages/webpack/template-webpack-plugin/package.json
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/1.snap.txt
- packages/tools/css-serializer/src/css/index.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/package.json
- packages/rspeedy/core/src/config/output/source-map.ts
- packages/tools/css-serializer/src/css/sourceMap.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/snapshot/rspack/1.snap.txt
- packages/rspeedy/core/test/config/output/source-map.test.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/snapshot/rspack/0.snap.txt
🚧 Files skipped from review as they are similar to previous changes (19)
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/snapshot/rspack/0.snap.txt
- packages/rspeedy/core/src/config/defaults.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/snapshot/rspack/1.snap.txt
- packages/tools/css-serializer/src/css/ast.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/snapshot/rspack/2.snap.txt
- packages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.md
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/0.snap.txt
- packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
- packages/tools/css-serializer/test/css.test.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/2.snap.txt
- packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts
462ca2b to
131c68f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/tools/css-serializer/src/css/cssChunksToMap.ts (1)
30-82:⚠️ Potential issue | 🟠 MajorPer-chunk preserved locations are lost when multiple chunks share a
cssId.
debundleCSS(..., true)emits content whoselocoffsets are keyed to each chunk's own source, butcontent.join('\n')on line 55 concatenates every chunk's fragments for the samecssIdinto a single synthetic stylesheet beforecssToAst. When two CSS assets feed the samecssId(the common case forcssId: 0/global CSS, and any scoped id that appears in more than one chunk), rules from the second and later fragments get line offsets shifted by the preceding fragments, so thelocvalues written intotasm.jsoncan no longer be resolved against the correspondingmain.css.map— diagnostics will point at the wrong file or wrong line.Preserve per-asset boundaries through AST construction (e.g. parse each chunk's fragments separately per
cssIdand carry origin metadata through to remapping), or compute and apply explicit offsets against the exact combined CSS asset whose source map will be used downstream.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tools/css-serializer/src/css/cssChunksToMap.ts` around lines 30 - 82, The current code concatenates fragments via content.join('\n') before calling cssToAst, which shifts per-chunk locs emitted by debundleCSS; instead, for each cssId in cssMap, iterate the original fragments (from the debundledMap entries produced by debundleCSS/normalizeCSSChunk) and parse each fragment separately with cssToAst (or compute and apply precise line/column offsets to each fragment's AST), preserving or annotating origin metadata so locs remain resolvable; update the stylesheets construction that uses cssToAst, cssId, root, and content (avoid using content.join('\n') directly) so you either (a) combine per-fragment roots while keeping their original locs/metadata, or (b) adjust each fragment's locs by exact offsets before merging, and ensure the resulting root and contentMap keep the mapping between fragments and their original assets.
🧹 Nitpick comments (1)
packages/rspeedy/plugin-react/test/sourcemap.test.ts (1)
154-164: Rename the test to include CSS sourcemaps.Line 161 now verifies default CSS sourcemap emission too, so the current
"js sourcemaps are emitted by default"name is a bit narrower than the assertion.Proposed rename
- test('js sourcemaps are emitted by default', async () => { + test('js and css sourcemaps are emitted by default', async () => {🤖 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 154 - 164, The test name "js sourcemaps are emitted by default" is too narrow because the assertions also check for CSS sourcemaps; rename the test description string in the test block (the test(...) call in sourcemap.test.ts that calls buildSourcemapFixture and getSourceMapFiles) to something like "js and css sourcemaps are emitted by default" so it accurately reflects the assertions and includes both JS and CSS sourcemap checks.
🤖 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/tools/css-serializer/src/css/cssChunksToMap.ts`:
- Around line 30-82: The current code concatenates fragments via
content.join('\n') before calling cssToAst, which shifts per-chunk locs emitted
by debundleCSS; instead, for each cssId in cssMap, iterate the original
fragments (from the debundledMap entries produced by
debundleCSS/normalizeCSSChunk) and parse each fragment separately with cssToAst
(or compute and apply precise line/column offsets to each fragment's AST),
preserving or annotating origin metadata so locs remain resolvable; update the
stylesheets construction that uses cssToAst, cssId, root, and content (avoid
using content.join('\n') directly) so you either (a) combine per-fragment roots
while keeping their original locs/metadata, or (b) adjust each fragment's locs
by exact offsets before merging, and ensure the resulting root and contentMap
keep the mapping between fragments and their original assets.
---
Nitpick comments:
In `@packages/rspeedy/plugin-react/test/sourcemap.test.ts`:
- Around line 154-164: The test name "js sourcemaps are emitted by default" is
too narrow because the assertions also check for CSS sourcemaps; rename the test
description string in the test block (the test(...) call in sourcemap.test.ts
that calls buildSourcemapFixture and getSourceMapFiles) to something like "js
and css sourcemaps are emitted by default" so it accurately reflects the
assertions and includes both JS and CSS sourcemap checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0cdf7a10-2d44-4dff-a265-8d17ffde3ee2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
.changeset/add-css-source-map-diagnostics.md.changeset/enable-rspeedy-css-sourcemap.md.changeset/legal-phones-tell.md.changeset/tender-bees-lick.md.github/css-source-map-diagnostics.instructions.mdpackages/rspeedy/core/src/config/defaults.tspackages/rspeedy/core/src/config/output/source-map.tspackages/rspeedy/core/test/config/output/source-map.test.tspackages/rspeedy/core/test/plugins/output.plugin.test.tspackages/rspeedy/plugin-react/test/sourcemap.test.tspackages/tools/css-serializer/src/css/ast.tspackages/tools/css-serializer/src/css/cssChunksToMap.tspackages/tools/css-serializer/src/css/debundle.tspackages/tools/css-serializer/src/css/index.tspackages/tools/css-serializer/src/css/sourceMap.tspackages/tools/css-serializer/test/css.test.tspackages/webpack/css-extract-webpack-plugin/package.jsonpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/2.snap.txtpackages/webpack/react-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/src/LynxEncodePlugin.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/cssDiagnostics.tspackages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
✅ Files skipped from review due to trivial changes (27)
- .changeset/legal-phones-tell.md
- .changeset/add-css-source-map-diagnostics.md
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/snapshot/rspack/0.snap.txt
- .changeset/tender-bees-lick.md
- .changeset/enable-rspeedy-css-sourcemap.md
- packages/webpack/react-webpack-plugin/package.json
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/0.snap.txt
- .github/css-source-map-diagnostics.instructions.md
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/package.json
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/snapshot/rspack/0.snap.txt
- packages/webpack/template-webpack-plugin/package.json
- packages/tools/css-serializer/src/css/index.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/snapshot/rspack/0.snap.txt
- packages/tools/css-serializer/src/css/sourceMap.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/1.snap.txt
- packages/rspeedy/core/test/config/output/source-map.test.ts
- packages/rspeedy/core/src/config/output/source-map.ts
- packages/rspeedy/core/test/plugins/output.plugin.test.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/0.snap.txt
- packages/tools/css-serializer/test/css.test.ts
- packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/2.snap.txt
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/1.snap.txt
- packages/rspeedy/core/src/config/defaults.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/snapshot/rspack/2.snap.txt
- packages/tools/css-serializer/src/css/ast.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/2.snap.txt
- packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/0.snap.txt
- packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts
- packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts
131c68f to
a982b42
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts (1)
112-112:⚠️ Potential issue | 🟠 MajorScope CSS diagnostic remapping and dedupe to the current template.
Line 248 resolves every diagnostic through the first
.csssource map found by Lines 389-400, while Line 112 dedupes warnings compilation-wide. In multi-entry or lazy-template compilations, diagnostics can be remapped through the wrong CSS asset or deduped across unrelated templates. Pass the current encode’s CSS asset names into this path and use them for both source-map selection and the dedupe key.Also applies to: 243-252, 389-400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts` at line 112, The current dedupe set emittedCSSDiagnosticWarnings and the source-map selection logic resolve diagnostics across the entire compilation, causing cross-template remapping/deduping; modify the encode flow to accept and pass the current encode's CSS asset name list into the diagnostic remapping path, scope emittedCSSDiagnosticWarnings to a per-encode (per-template) Set instead of the module/global one, and use the CSS asset names as part of the dedupe key and when choosing the correct .css source-map in the remapping function (the code that currently performs diagnostics lookup/resolution and the emittedCSSDiagnosticWarnings usage).packages/tools/css-serializer/src/css/cssChunksToMap.ts (1)
30-56:⚠️ Potential issue | 🟠 MajorAvoid shifting preserved locations when merging chunks.
Line 47 merges debundled fragments from multiple chunks under the same
cssId, then Line 55 reparsescontent.join('\n'). BecausedebundleCSS(..., true)preserves coordinates relative to each original chunk, later fragments can be shifted by earlier fragments, causing TASM diagnostics to miss the owning CSS source map. Keep chunk/asset boundaries through parsing or apply offsets against the exact combined CSS asset before merging AST nodes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tools/css-serializer/src/css/cssChunksToMap.ts` around lines 30 - 56, The current merge in cssChunksToMap collects debundled fragments into cssMap and then reparses content.join('\n') which shifts preserved coordinates from debundleCSS and breaks source mapping; modify the merge so you do not simply join fragments—either parse each normalizedCSSChunk/debundledMap entry via cssToAst individually (preserving original chunk boundaries and positions) and then combine ASTs, or track exact byte/line offsets for each debundled fragment when pushing into cssMap and apply those offsets to the AST nodes before merging; update the logic around normalizeCSSChunk, debundleCSS, cssMap/debundledMap population and the stylesheets creation (cssToAst usage) to preserve original positions instead of using content.join('\n').
🧹 Nitpick comments (5)
packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts (3)
49-107: Minor:TraceMapis reconstructed on every call.
new TraceMap(mainCSSSourceMap)parses themappingsVLQ on each invocation ofresolveTasmCSSDiagnostics. If this is called once per compilation per template chunk it's negligible, but if called per-diagnostic or per-chunk in a larger build, caching theTraceMapkeyed by the source-map reference would help. Not critical for the current call site inLynxEncodePlugin.ts.Also worth noting: the
column - 1/column + 1adjustments across lines 72 and 104 correctly convert between TASM's 1-indexed columns and trace-mapping's 0-indexed columns. A short comment on line 72 stating "trace-mapping uses 0-indexed columns" would prevent accidental removal during future refactors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts` around lines 49 - 107, resolveTasmCSSDiagnostics currently recreates a TraceMap for every call (new TraceMap(mainCSSSourceMap)), which is wasteful; change the implementation to cache TraceMap instances keyed by the mainCSSSourceMap (use a module-level Map or WeakMap keyed by the SourceMapInput) and reuse the cached traceMap when mainCSSSourceMap is unchanged, and add a short inline comment by the column adjustment in the mapped call (the Math.max(diagnostic.column - 1, 0) line) stating that "trace-mapping uses 0-indexed columns" so future maintainers don't remove the -1/+1 conversions; keep behavior identical aside from reusing TraceMap instances.
151-180: Absolute-path check on Line 171 is POSIX-only.
source.startsWith('/')treats POSIX absolute paths as-is but doesn't handle Windows absolute paths (e.g.,C:\src\app.cssorC:/src/app.css). In practice, webpack/rspack normalize sources towebpack://…or forward-slash-prefixed relative paths, so this path is rarely hit on Windows—but if a source ever arrives as a Windows absolute path, it would be re-resolved againstcontext(line 179), producing a nonsensical path.Consider using
node:path'sisAbsolutefor cross-platform correctness:Proposed fix
-import { resolve as resolvePath } from 'node:path'; +import { isAbsolute, resolve as resolvePath } from 'node:path'; @@ - if (source.startsWith('/')) { + if (isAbsolute(source)) { return source; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts` around lines 151 - 180, The absolute-path check in normalizeTasmSourcePath is POSIX-only (source.startsWith('/')) and will mis-handle Windows absolute paths; change the check to use Node's path.isAbsolute (import isAbsolute from 'path') and treat any isAbsolute(source) as the absolute case (return source unchanged), leaving the webpack:, file://, sourceRoot, and resolvePath(context, source) branches intact so Windows absolute paths are not re-resolved against context.
193-195:isRecordreturns true for arrays.
typeof [] === 'object'and arrays are notnull, soisRecord([])istrue. Currently benign becausenormalizeTasmCSSDiagnosticthen readsvalue['line']/value['column']and rejects when those aren't numbers—but a stricter guard would document intent and prevent future callers from being surprised:Proposed refinement
function isRecord(value: unknown): value is Record<string, unknown> { - return value !== null && typeof value === 'object'; + return value !== null && typeof value === 'object' && !Array.isArray(value); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts` around lines 193 - 195, The isRecord type guard currently returns true for arrays; update isRecord to exclude arrays by adding a check like !Array.isArray(value) (e.g., function isRecord(value: unknown): value is Record<string, unknown> { return value !== null && typeof value === 'object' && !Array.isArray(value); }) so callers such as normalizeTasmCSSDiagnostic receive a stricter plain-object guard and arrays are not treated as records.packages/tools/css-serializer/src/css/debundle.ts (2)
228-241:getLoccoupling topreserveLocations: trueis implicit.
getLocthrows whennode.locis missing, which only holds when the AST was parsed withpositions: true. This invariant is only enforced indirectly by gating callers (getBlockSegments,getTopLevelSegments,buildStylesheetFromSegments) behindpreserveLocations. Consider adding a short comment ongetLocnoting this coupling so a future caller doesn't accidentally invoke it from thepreserveLocations: falsepath.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tools/css-serializer/src/css/debundle.ts` around lines 228 - 241, getLoc currently throws if node.loc is missing but that invariant depends on parsing with positions enabled (preserveLocations); add a short explanatory comment above getLoc stating it requires the AST to have positions (e.g., parsed with positions: true / preserveLocations) and note the intended gated callers (getBlockSegments, getTopLevelSegments, buildStylesheetFromSegments) so future maintainers won't call getLoc from a preserveLocations: false path; optionally, improve the thrown Error message to explicitly mention the missing positions requirement for clarity.
149-166: Consider renaminggetBlockSegmentssince it returns at most one segment.The function returns a single-element array covering the entire block (first child start → last child end). The plural "Segments" naming and array return type suggest multiple segments. Either return a single
CSSSegment | null(and wrap at the call site) or keep the current shape intentionally for future extension—but a brief comment clarifying this would help future readers. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tools/css-serializer/src/css/debundle.ts` around lines 149 - 166, Rename getBlockSegments to getBlockSegment (singular) and change its return type from CSSSegment[] to CSSSegment | null so it returns a single segment or null when empty; update the implementation to return the single CSSSegment object (not an array) using the same logic (use getLoc on first/last child and slice code) and then update all call sites that expect an array (e.g., places iterating or destructuring the array) to handle a possibly null value or wrap the returned segment in an array where multiple-segment semantics are still needed; alternatively, if you prefer to keep the array shape, add a concise comment above getBlockSegments explaining it intentionally returns at most one segment to cover the entire block.
🤖 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/tools/css-serializer/src/css/cssChunksToMap.ts`:
- Around line 30-56: The current merge in cssChunksToMap collects debundled
fragments into cssMap and then reparses content.join('\n') which shifts
preserved coordinates from debundleCSS and breaks source mapping; modify the
merge so you do not simply join fragments—either parse each
normalizedCSSChunk/debundledMap entry via cssToAst individually (preserving
original chunk boundaries and positions) and then combine ASTs, or track exact
byte/line offsets for each debundled fragment when pushing into cssMap and apply
those offsets to the AST nodes before merging; update the logic around
normalizeCSSChunk, debundleCSS, cssMap/debundledMap population and the
stylesheets creation (cssToAst usage) to preserve original positions instead of
using content.join('\n').
In `@packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts`:
- Line 112: The current dedupe set emittedCSSDiagnosticWarnings and the
source-map selection logic resolve diagnostics across the entire compilation,
causing cross-template remapping/deduping; modify the encode flow to accept and
pass the current encode's CSS asset name list into the diagnostic remapping
path, scope emittedCSSDiagnosticWarnings to a per-encode (per-template) Set
instead of the module/global one, and use the CSS asset names as part of the
dedupe key and when choosing the correct .css source-map in the remapping
function (the code that currently performs diagnostics lookup/resolution and the
emittedCSSDiagnosticWarnings usage).
---
Nitpick comments:
In `@packages/tools/css-serializer/src/css/debundle.ts`:
- Around line 228-241: getLoc currently throws if node.loc is missing but that
invariant depends on parsing with positions enabled (preserveLocations); add a
short explanatory comment above getLoc stating it requires the AST to have
positions (e.g., parsed with positions: true / preserveLocations) and note the
intended gated callers (getBlockSegments, getTopLevelSegments,
buildStylesheetFromSegments) so future maintainers won't call getLoc from a
preserveLocations: false path; optionally, improve the thrown Error message to
explicitly mention the missing positions requirement for clarity.
- Around line 149-166: Rename getBlockSegments to getBlockSegment (singular) and
change its return type from CSSSegment[] to CSSSegment | null so it returns a
single segment or null when empty; update the implementation to return the
single CSSSegment object (not an array) using the same logic (use getLoc on
first/last child and slice code) and then update all call sites that expect an
array (e.g., places iterating or destructuring the array) to handle a possibly
null value or wrap the returned segment in an array where multiple-segment
semantics are still needed; alternatively, if you prefer to keep the array
shape, add a concise comment above getBlockSegments explaining it intentionally
returns at most one segment to cover the entire block.
In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts`:
- Around line 49-107: resolveTasmCSSDiagnostics currently recreates a TraceMap
for every call (new TraceMap(mainCSSSourceMap)), which is wasteful; change the
implementation to cache TraceMap instances keyed by the mainCSSSourceMap (use a
module-level Map or WeakMap keyed by the SourceMapInput) and reuse the cached
traceMap when mainCSSSourceMap is unchanged, and add a short inline comment by
the column adjustment in the mapped call (the Math.max(diagnostic.column - 1, 0)
line) stating that "trace-mapping uses 0-indexed columns" so future maintainers
don't remove the -1/+1 conversions; keep behavior identical aside from reusing
TraceMap instances.
- Around line 151-180: The absolute-path check in normalizeTasmSourcePath is
POSIX-only (source.startsWith('/')) and will mis-handle Windows absolute paths;
change the check to use Node's path.isAbsolute (import isAbsolute from 'path')
and treat any isAbsolute(source) as the absolute case (return source unchanged),
leaving the webpack:, file://, sourceRoot, and resolvePath(context, source)
branches intact so Windows absolute paths are not re-resolved against context.
- Around line 193-195: The isRecord type guard currently returns true for
arrays; update isRecord to exclude arrays by adding a check like
!Array.isArray(value) (e.g., function isRecord(value: unknown): value is
Record<string, unknown> { return value !== null && typeof value === 'object' &&
!Array.isArray(value); }) so callers such as normalizeTasmCSSDiagnostic receive
a stricter plain-object guard and arrays are not treated as records.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b4216e6d-3d5c-4b7d-9214-f6d019b68398
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
.changeset/add-css-source-map-diagnostics.md.changeset/enable-rspeedy-css-sourcemap.md.changeset/legal-phones-tell.md.changeset/tender-bees-lick.md.github/css-source-map-diagnostics.instructions.mdpackages/rspeedy/core/src/config/defaults.tspackages/rspeedy/core/src/config/output/source-map.tspackages/rspeedy/core/test/config/output/source-map.test.tspackages/rspeedy/core/test/plugins/output.plugin.test.tspackages/rspeedy/plugin-react/test/sourcemap.test.tspackages/tools/css-serializer/src/css/ast.tspackages/tools/css-serializer/src/css/cssChunksToMap.tspackages/tools/css-serializer/src/css/debundle.tspackages/tools/css-serializer/src/css/index.tspackages/tools/css-serializer/src/css/sourceMap.tspackages/tools/css-serializer/test/css.test.tspackages/webpack/css-extract-webpack-plugin/package.jsonpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/__snapshot__/rspack/2.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/0.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/1.snap.txtpackages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/__snapshot__/rspack/2.snap.txtpackages/webpack/react-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/package.jsonpackages/webpack/template-webpack-plugin/src/LynxEncodePlugin.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/cssDiagnostics.tspackages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
✅ Files skipped from review due to trivial changes (25)
- .changeset/enable-rspeedy-css-sourcemap.md
- .changeset/legal-phones-tell.md
- packages/webpack/react-webpack-plugin/package.json
- .changeset/tender-bees-lick.md
- packages/webpack/template-webpack-plugin/package.json
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/snapshot/rspack/2.snap.txt
- .changeset/add-css-source-map-diagnostics.md
- packages/tools/css-serializer/src/css/index.ts
- packages/rspeedy/core/src/config/output/source-map.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/0.snap.txt
- .github/css-source-map-diagnostics.instructions.md
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/basic/snapshot/rspack/2.snap.txt
- packages/tools/css-serializer/src/css/sourceMap.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/bundle-splitting/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/export-default/snapshot/rspack/1.snap.txt
- packages/rspeedy/core/test/config/output/source-map.test.ts
- packages/webpack/css-extract-webpack-plugin/package.json
- packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/nested/snapshot/rspack/0.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/snapshot/rspack/0.snap.txt
- packages/rspeedy/core/src/config/defaults.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/default/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/snapshot/rspack/1.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/modules/basic/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/chunk-splitting/snapshot/rspack/2.snap.txt
- packages/webpack/css-extract-webpack-plugin/test/hotCases/hot-update-json/filename/snapshot/rspack/2.snap.txt
- packages/rspeedy/core/test/plugins/output.plugin.test.ts
- packages/tools/css-serializer/test/css.test.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/scoped/snapshot/rspack/0.snap.txt
- packages/rspeedy/plugin-react/test/sourcemap.test.ts
- packages/tools/css-serializer/src/css/ast.ts
- packages/webpack/css-extract-webpack-plugin/test/hotCases/css/enable-css-selector/snapshot/rspack/0.snap.txt
- packages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.md
upupming
left a comment
There was a problem hiding this comment.
LGTM, btw does it work for scoped css too?
In this commit, @lynx-js/css-serializer generates cssMap with bundle-CSS line and column locations preserved on style rules and declarations. These locations are written into tasm.json as part of the encoder input, so TASM can report CSS diagnostics using coordinates that correspond to the bundled CSS output.
Those bundle-space coordinates are then remapped in template-webpack-plugin through main.css.map, allowing emitted webpack warnings to point back to the original source CSS files. The cssMap itself is still grouped by cssId and consumed as the style AST for template encoding; its preserved loc fields are what make later diagnostic remapping accurate.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
Checklist