fix(template): resolve css diagnostics sourcemaps per template#2519
Conversation
🦋 Changeset detectedLatest commit: d544afc The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
📝 WalkthroughWalkthroughThis PR refactors CSS diagnostics source map handling in the template-webpack-plugin. The public API replaces webpack Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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
Updates template webpack plugin CSS diagnostics handling so warnings can be source-mapped using the CSS chunks/source maps associated with the current template (incl. multi-entry/MPA scenarios).
Changes:
- Thread
cssDiagnosticsthroughencode→beforeEmithooks and resolve diagnostics against per-template CSS chunk source maps. - Update CSS diagnostics utilities to accept/parse multiple source maps and ignore invalid map payloads.
- Add/extend tests and fixtures to cover multi-entry mapping and invalid source map inputs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts | Switch resolution to multiple CSS source maps; add CSS chunk map collection/parsing helpers. |
| packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts | Pass cssDiagnostics through hooks and resolve warnings using per-template CSS chunk source maps. |
| packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts | Extend hook types/plumbing to carry optional cssDiagnostics. |
| packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts | Update tests for new API shape; add coverage for invalid maps + first-mappable behavior + map collection. |
| packages/webpack/template-webpack-plugin/test/basic.test.ts | Add integration-ish tests for beforeEmit diagnostics and MPA entry-to-sourcemap mapping. |
| packages/webpack/template-webpack-plugin/test/fixtures/css-diagnostics-mpa/* | New MPA fixture entries and CSS with intentionally-unknown properties. |
| packages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.md | API report updated for changed types/options. |
| .changeset/fix-template-css-diagnostics-sourcemap.md | Added changeset file (currently empty). |
Comments suppressed due to low confidence (1)
packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts:81
processTasmCSSDiagnosticsis re-exported from the package entrypoint (src/index.ts), so changing its options from{ compilation }to{ cssSourceMaps }is a breaking public API change. If you need this new input, consider preserving backwards compatibility (e.g., accept bothcompilationandcssSourceMapswith a clear precedence/deprecation path) or ensure the changeset reflects a major version bump and release notes call out the breaking change.
export interface ProcessTasmCSSDiagnosticsOptions {
/**
* The raw `css_diagnostics` value returned by TASM.
*/
cssDiagnostics: unknown;
/**
* CSS source map contents from the CSS chunks for the current template.
*/
cssSourceMaps: string[];
/**
* The webpack compiler context used to resolve relative source paths.
*/
context: string;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts (2)
240-245: Misleadingas stringcast on optional field.
cssDiagnosticsin theencodehook payload is declared ascssDiagnostics?: string, so returningundefinedis valid. Castingcss_diagnostics as stringwhen the value can beundefinedhides that runtime truth from the type system and misleads downstream consumers (e.g.,processTasmCSSDiagnosticssees it as non-optional).🔧 Suggested change
return { buffer, debugInfo: lepus_debug, - cssDiagnostics: css_diagnostics as string, + cssDiagnostics: css_diagnostics as string | undefined, };🤖 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` around lines 240 - 245, The return in the encode hook is casting css_diagnostics to a non-optional string which hides that cssDiagnostics can be undefined; update the object returned by the encode hook to return cssDiagnostics: css_diagnostics (no "as string" cast) so the optional type is preserved, and verify callers such as processTasmCSSDiagnostics handle the undefined case appropriately (adjust callers if they currently assume a non-optional string).
247-290:beforeEmitruns diagnostics processing unconditionally.
processTasmCSSDiagnosticsandcollectCSSSourceMapContentsare invoked on everybeforeEmiteven whenargs.cssDiagnosticsisundefined/empty (i.e., no diagnostics were produced duringencode). Depending on howextractTasmCSSDiagnosticshandlesundefined, this may be a harmless no-op, but you're still iteratingargs.cssChunksand stringifying source maps in the common happy path where there's nothing to report.Consider early-returning when there are no diagnostics:
🔧 Suggested change
templateHooks.beforeEmit.tapPromise({ name: this.name, stage: LynxEncodePlugin.BEFORE_EMIT_STAGE, }, async (args) => { + if (!args.cssDiagnostics) { + return args; + } const resolvedDiagnostics = processTasmCSSDiagnostics({ cssDiagnostics: args.cssDiagnostics, cssSourceMaps: collectCSSSourceMapContents(args.cssChunks), context: compiler.context, emittedWarnings: emittedCSSDiagnosticWarnings, });🤖 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` around lines 247 - 290, The beforeEmit hook currently always calls processTasmCSSDiagnostics and collectCSSSourceMapContents even when there are no diagnostics; modify the templateHooks.beforeEmit tapPromise handler to first check args.cssDiagnostics (e.g., if falsy or length === 0) and early return args before calling collectCSSSourceMapContents/processTasmCSSDiagnostics or iterating cssChunks, so you only perform source-map collection and diagnostic resolution when diagnostics exist; keep existing behavior for building WebpackError and pushing to compilation.warnings unchanged when diagnostics are present.packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts (1)
285-303: Consider tightening the test cast.
as neverworks but discards type safety for the chunk shape expected bycollectCSSSourceMapContents. A typed partial would document the minimum contract expected fromAsset(justsource.map()):🔧 Suggested change
- const result = collectCSSSourceMapContents( - [{ - name: 'main.css', - info: {}, - source: { - map: () => sourceMap, - }, - } as never], - ); + const result = collectCSSSourceMapContents([ + { + name: 'main.css', + info: {}, + source: { map: () => sourceMap }, + } as unknown as Parameters<typeof collectCSSSourceMapContents>[0][number], + ]);🤖 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 285 - 303, The test currently uses an unsafe `as never` cast for the chunk passed to collectCSSSourceMapContents; replace it with a minimal typed object that documents the expected Asset contract (e.g. a Partial or Pick of the Asset type that includes name, info and a source with map(): any) and cast that to Asset only if necessary so the test keeps type-safety and documents the required shape for collectCSSSourceMapContents.packages/webpack/template-webpack-plugin/test/basic.test.ts (1)
360-367: PreferJSON.stringifyover hand-concatenated JSON.Manually interpolating
name/typeinto a JSON string will break if any field ever contains a quote, backslash, or newline. Safer and equivalent:♻️ Proposed refactor
-function createCSSDiagnosticsJSON(diagnostic: { - type: string; - name: string; - line: number; - column: number; -}): string { - return `[{"type":"${diagnostic.type}","name":"${diagnostic.name}","line":${diagnostic.line},"column":${diagnostic.column}}]`; -} +function createCSSDiagnosticsJSON(diagnostic: { + type: string; + name: string; + line: number; + column: number; +}): string { + return JSON.stringify([diagnostic]); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/template-webpack-plugin/test/basic.test.ts` around lines 360 - 367, The createCSSDiagnosticsJSON function builds JSON by string interpolation which breaks for quotes/backslashes/newlines; update createCSSDiagnosticsJSON to construct the diagnostic object/array and return JSON.stringify(...) instead of manual concatenation (use the diagnostic fields to create an object and stringify it), referencing the createCSSDiagnosticsJSON function name to locate the change.packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts (1)
233-269: Optional: avoid stringify → parse round-trip on source maps.
collectCSSSourceMapContentsJSON.stringifys each map (for string-basedSetdedup), andparseCSSSourceMapsimmediatelyJSON.parses them again before constructingTraceMaps. For large CSS bundles with many chunks, that’s wasted CPU. If dedup is the sole reason for stringifying, you can dedup with aMap<string, CSS.CSSSourceMap>keyed by the stringified form and carry the parsed object through, or (simpler) accept the duplication cost in the rare case two chunks emit identical maps.Not blocking — the current design is correct, this is purely a micro-optimization on a hot-ish path.
🤖 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 233 - 269, collectCSSSourceMapContents currently JSON.stringifys source maps to dedupe and parseCSSSourceMaps JSON.parses them back; avoid the stringify→parse round-trip by deduping while retaining parsed objects: have collectCSSSourceMapContents build a Map keyed by JSON.stringify(sourceMap) but store the normalized parsed object (using normalizeCSSSourceMap) as the value, then return an array of CSS.CSSSourceMap values; update parseCSSSourceMaps to accept CSS.CSSSourceMap[] (or remove it and use the collected array directly) and keep the existing isCSSSourceMap validation (referencing collectCSSSourceMapContents, parseCSSSourceMaps, normalizeCSSSourceMap, and isCSSSourceMap).
🤖 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/fix-template-css-diagnostics-sourcemap.md:
- Around line 1-3: The changeset is empty but the PR introduces a breaking
public API change to `@lynx-js/template-webpack-plugin`:
ProcessTasmCSSDiagnosticsOptions now requires cssSourceMaps: string[] and
removes compilation, affecting processTasmCSSDiagnostics (see
etc/template-webpack-plugin.api.md). Add a non-empty changeset entry for
`@lynx-js/template-webpack-plugin` describing the API change and bump the package
version (at least patch, likely minor) so downstream users are notified; include
a short note about the required cssSourceMaps field and removal of compilation
to guide consumers.
---
Nitpick comments:
In `@packages/webpack/template-webpack-plugin/src/cssDiagnostics.ts`:
- Around line 233-269: collectCSSSourceMapContents currently JSON.stringifys
source maps to dedupe and parseCSSSourceMaps JSON.parses them back; avoid the
stringify→parse round-trip by deduping while retaining parsed objects: have
collectCSSSourceMapContents build a Map keyed by JSON.stringify(sourceMap) but
store the normalized parsed object (using normalizeCSSSourceMap) as the value,
then return an array of CSS.CSSSourceMap values; update parseCSSSourceMaps to
accept CSS.CSSSourceMap[] (or remove it and use the collected array directly)
and keep the existing isCSSSourceMap validation (referencing
collectCSSSourceMapContents, parseCSSSourceMaps, normalizeCSSSourceMap, and
isCSSSourceMap).
In `@packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts`:
- Around line 240-245: The return in the encode hook is casting css_diagnostics
to a non-optional string which hides that cssDiagnostics can be undefined;
update the object returned by the encode hook to return cssDiagnostics:
css_diagnostics (no "as string" cast) so the optional type is preserved, and
verify callers such as processTasmCSSDiagnostics handle the undefined case
appropriately (adjust callers if they currently assume a non-optional string).
- Around line 247-290: The beforeEmit hook currently always calls
processTasmCSSDiagnostics and collectCSSSourceMapContents even when there are no
diagnostics; modify the templateHooks.beforeEmit tapPromise handler to first
check args.cssDiagnostics (e.g., if falsy or length === 0) and early return args
before calling collectCSSSourceMapContents/processTasmCSSDiagnostics or
iterating cssChunks, so you only perform source-map collection and diagnostic
resolution when diagnostics exist; keep existing behavior for building
WebpackError and pushing to compilation.warnings unchanged when diagnostics are
present.
In `@packages/webpack/template-webpack-plugin/test/basic.test.ts`:
- Around line 360-367: The createCSSDiagnosticsJSON function builds JSON by
string interpolation which breaks for quotes/backslashes/newlines; update
createCSSDiagnosticsJSON to construct the diagnostic object/array and return
JSON.stringify(...) instead of manual concatenation (use the diagnostic fields
to create an object and stringify it), referencing the createCSSDiagnosticsJSON
function name to locate the change.
In `@packages/webpack/template-webpack-plugin/test/cssDiagnostics.test.ts`:
- Around line 285-303: The test currently uses an unsafe `as never` cast for the
chunk passed to collectCSSSourceMapContents; replace it with a minimal typed
object that documents the expected Asset contract (e.g. a Partial or Pick of the
Asset type that includes name, info and a source with map(): any) and cast that
to Asset only if necessary so the test keeps type-safety and documents the
required shape for collectCSSSourceMapContents.
🪄 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: 17c03864-a2b3-462c-978f-757c58b04a5e
📒 Files selected for processing (11)
.changeset/fix-template-css-diagnostics-sourcemap.mdpackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/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/basic.test.tspackages/webpack/template-webpack-plugin/test/cssDiagnostics.test.tspackages/webpack/template-webpack-plugin/test/fixtures/css-diagnostics-mpa/a.csspackages/webpack/template-webpack-plugin/test/fixtures/css-diagnostics-mpa/a.jspackages/webpack/template-webpack-plugin/test/fixtures/css-diagnostics-mpa/b.csspackages/webpack/template-webpack-plugin/test/fixtures/css-diagnostics-mpa/b.js
React External#698 Bundle Size — 679.93KiB (0%).d544afc(current) vs 8352530 main#686(baseline) Bundle metrics
|
| Current #698 |
Baseline #686 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch fix/template-css-diagnostics-sou... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9152 Bundle Size — 900.04KiB (0%).d544afc(current) vs 8352530 main#9140(baseline) Bundle metrics
|
| Current #9152 |
Baseline #9140 |
|
|---|---|---|
| Initial JS |
44.46KiB |
44.46KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
 Cache Invalidation |
0% |
8.1% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
228 |
228 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.29%(+0.04%) |
27.28% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9152 |
Baseline #9140 |
|
|---|---|---|
| JS |
495.9KiB |
495.9KiB |
| Other |
401.92KiB |
401.92KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch fix/template-css-diagnostics-sou... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7580 Bundle Size — 225.23KiB (0%).d544afc(current) vs 8352530 main#7568(baseline) Bundle metrics
|
| Current #7580 |
Baseline #7568 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
179 |
179 |
| Duplicate Modules |
69 |
69 |
| Duplicate Code |
44.57% |
44.57% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #7580 |
Baseline #7568 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
79.47KiB |
79.47KiB |
Bundle analysis report Branch fix/template-css-diagnostics-sou... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#712 Bundle Size — 196.39KiB (0%).d544afc(current) vs 8352530 main#700(baseline) Bundle metrics
|
| Current #712 |
Baseline #700 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
173 |
173 |
| Duplicate Modules |
66 |
66 |
| Duplicate Code |
44.07% |
44.07% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #712 |
Baseline #700 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
85.15KiB |
85.15KiB |
Bundle analysis report Branch fix/template-css-diagnostics-sou... Project dashboard
Generated by RelativeCI Documentation Report issue
Merging this PR will not alter performance
Comparing 
Footnotes
|
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Checklist