feat(external-bundle): emit css diagnostics#2563
Conversation
📝 WalkthroughWalkthroughAdds CSS diagnostics propagation and processing to ExternalBundleWebpackPlugin: the encode callback may return ChangesCSS Diagnostics Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 encode diagnostic emission to the @lynx-js/lynx-bundle-rslib-config external bundle build flow by consuming css_diagnostics returned from TASM and converting them into webpack warnings (optionally source-mapped back to original files).
Changes:
- Bump
@lynx-js/tasmto0.0.38and wire in@lynx-js/template-webpack-pluginto reuseprocessTasmCSSDiagnostics. - Enhance
ExternalBundleWebpackPluginto collect CSS source maps, resolve/dedupe TASM CSS diagnostics, and push them intocompilation.warningswithfile/locwhen available. - Update default external bundle output settings to enable CSS sourcemaps and disable CSS minification (with a note about sourcemap correctness), plus add a focused unit test for diagnostics emission.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates lockfile for @lynx-js/tasm@0.0.38 and workspace link for @lynx-js/template-webpack-plugin. |
| packages/webpack/template-webpack-plugin/package.json | Bumps TASM dependency to 0.0.38. |
| packages/rspeedy/lynx-bundle-rslib-config/tsconfig.json | Adds TS project reference to template webpack plugin build config. |
| packages/rspeedy/lynx-bundle-rslib-config/tsconfig.build.json | Adds TS project reference to template webpack plugin build config. |
| packages/rspeedy/lynx-bundle-rslib-config/test/external-bundle.test.ts | Adds unit coverage asserting CSS diagnostics become webpack warnings with mapped locations. |
| packages/rspeedy/lynx-bundle-rslib-config/src/webpack/ExternalBundleWebpackPlugin.ts | Emits resolved CSS diagnostics via compilation.warnings and collects CSS source maps from extracted CSS assets. |
| packages/rspeedy/lynx-bundle-rslib-config/src/externalBundleRslibConfig.ts | Enables CSS sourcemaps for external bundles and disables CSS minification to avoid sourcemap source path issues. |
| packages/rspeedy/lynx-bundle-rslib-config/package.json | Updates dependencies (@lynx-js/tasm@0.0.38, adds @lynx-js/template-webpack-plugin) and aligns test script pattern with repo conventions. |
| packages/rspeedy/lynx-bundle-rslib-config/etc/lynx-bundle-rslib-config.api.md | Updates public API surface for css_diagnostics on the encode return type. |
| .changeset/update-template-tasm.md | Changeset for template plugin patch release due to TASM bump. |
| .changeset/external-bundle-css-diagnostics.md | Changeset for external bundle config patch release to emit CSS encode diagnostics. |
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.
🧹 Nitpick comments (2)
packages/rspeedy/lynx-bundle-rslib-config/src/webpack/ExternalBundleWebpackPlugin.ts (2)
110-117: 💤 Low valueOptional: short-circuit diagnostic processing when there are no diagnostics.
processTasmCSSDiagnosticsis invoked on every compilation, even whencssDiagnosticsisundefined(the common path when no CSS issues are emitted). It's safe —extractTasmCSSDiagnosticsreturns[]early — butcollectCSSSourceMapContentsstill walks all assets andJSON.stringifys every CSS source map. Skipping the call whencssDiagnosticsis nullish avoids that work on the hot path.♻️ Proposed early-return
- const { buffer, encodeOptions, cssDiagnostics } = await this.#encode(assets) - - const resolvedDiagnostics = processTasmCSSDiagnostics({ - cssDiagnostics, - cssSourceMaps: collectCSSSourceMapContents(assets), - context: compiler.context, - emittedWarnings: emittedCSSDiagnosticWarnings, - }) - resolvedDiagnostics.forEach((diagnostic) => { + const { buffer, encodeOptions, cssDiagnostics } = await this.#encode(assets) + + const resolvedDiagnostics = cssDiagnostics + ? processTasmCSSDiagnostics({ + cssDiagnostics, + cssSourceMaps: collectCSSSourceMapContents(assets), + context: compiler.context, + emittedWarnings: emittedCSSDiagnosticWarnings, + }) + : [] + resolvedDiagnostics.forEach((diagnostic) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rspeedy/lynx-bundle-rslib-config/src/webpack/ExternalBundleWebpackPlugin.ts` around lines 110 - 117, The code currently calls processTasmCSSDiagnostics and collectCSSSourceMapContents on every compilation; add a nullish check for cssDiagnostics (the value returned from this.#encode) and short-circuit the diagnostic work when cssDiagnostics is null or undefined: if cssDiagnostics is nullish, skip calling collectCSSSourceMapContents and processTasmCSSDiagnostics and set resolvedDiagnostics to an empty array (or the expected empty shape), otherwise call collectCSSSourceMapContents(assets) and processTasmCSSDiagnostics({ cssDiagnostics, cssSourceMaps: ..., context: compiler.context, emittedWarnings: emittedCSSDiagnosticWarnings }); this change touches the block using const { buffer, encodeOptions, cssDiagnostics } = await this.#encode(assets) and the subsequent processTasmCSSDiagnostics call.
234-252: 💤 Low valueConsider deduping by reference rather than serialized JSON.
Source maps for CSS chunks can be large;
JSON.stringifying each one purely to use it as aSetkey, then reparsing it insideparseCSSSourceMaps, doubles the work. Since each asset emits a uniqueSourceinstance, deduping by the asset name (or by theRawSourceMapobject identity /filefield) would avoid the round-trip. Low priority.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rspeedy/lynx-bundle-rslib-config/src/webpack/ExternalBundleWebpackPlugin.ts` around lines 234 - 252, collectCSSSourceMapContents currently JSON.stringifys each source map to dedupe which forces double work in parseCSSSourceMaps; instead gather the actual source map objects (or use the asset name) and dedupe by object identity or a stable key like sourceMap.file or asset.name. Update collectCSSSourceMapContents to push the RawSourceMap (not JSON) into an array and use a Set of seen keys (sourceMap.file || asset.name) to skip duplicates, then return the unique collection and adjust parseCSSSourceMaps callers to accept the RawSourceMap objects rather than re-parsing JSON.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/rspeedy/lynx-bundle-rslib-config/src/webpack/ExternalBundleWebpackPlugin.ts`:
- Around line 110-117: The code currently calls processTasmCSSDiagnostics and
collectCSSSourceMapContents on every compilation; add a nullish check for
cssDiagnostics (the value returned from this.#encode) and short-circuit the
diagnostic work when cssDiagnostics is null or undefined: if cssDiagnostics is
nullish, skip calling collectCSSSourceMapContents and processTasmCSSDiagnostics
and set resolvedDiagnostics to an empty array (or the expected empty shape),
otherwise call collectCSSSourceMapContents(assets) and
processTasmCSSDiagnostics({ cssDiagnostics, cssSourceMaps: ..., context:
compiler.context, emittedWarnings: emittedCSSDiagnosticWarnings }); this change
touches the block using const { buffer, encodeOptions, cssDiagnostics } = await
this.#encode(assets) and the subsequent processTasmCSSDiagnostics call.
- Around line 234-252: collectCSSSourceMapContents currently JSON.stringifys
each source map to dedupe which forces double work in parseCSSSourceMaps;
instead gather the actual source map objects (or use the asset name) and dedupe
by object identity or a stable key like sourceMap.file or asset.name. Update
collectCSSSourceMapContents to push the RawSourceMap (not JSON) into an array
and use a Set of seen keys (sourceMap.file || asset.name) to skip duplicates,
then return the unique collection and adjust parseCSSSourceMaps callers to
accept the RawSourceMap objects rather than re-parsing JSON.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 360fb8e7-dd2a-4ad8-96cc-9f1238b2f589
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.changeset/external-bundle-css-diagnostics.md.changeset/update-template-tasm.mdpackages/rspeedy/lynx-bundle-rslib-config/etc/lynx-bundle-rslib-config.api.mdpackages/rspeedy/lynx-bundle-rslib-config/package.jsonpackages/rspeedy/lynx-bundle-rslib-config/src/externalBundleRslibConfig.tspackages/rspeedy/lynx-bundle-rslib-config/src/webpack/ExternalBundleWebpackPlugin.tspackages/rspeedy/lynx-bundle-rslib-config/test/external-bundle.test.tspackages/rspeedy/lynx-bundle-rslib-config/tsconfig.build.jsonpackages/rspeedy/lynx-bundle-rslib-config/tsconfig.jsonpackages/webpack/template-webpack-plugin/package.json
7f2d1b3 to
2d0afe0
Compare
🦋 Changeset detectedLatest commit: 2d0afe0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
React External#927 Bundle Size — 680.83KiB (~+0.01%).2d0afe0(current) vs 5f3b6eb main#913(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch fix/external-bundle-css-diagnost... Project dashboard Generated by RelativeCI Documentation Report issue |
React MTF Example#944 Bundle Size — 196.68KiB (0%).2d0afe0(current) vs 5f3b6eb main#930(baseline) Bundle metrics
|
| Current #944 |
Baseline #930 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
174 |
174 |
|
66 |
66 |
|
44.05% |
44.05% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #944 |
Baseline #930 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
85.45KiB |
85.45KiB |
Bundle analysis report Branch fix/external-bundle-css-diagnost... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7812 Bundle Size — 225.52KiB (0%).2d0afe0(current) vs 5f3b6eb main#7798(baseline) Bundle metrics
|
| Current #7812 |
Baseline #7798 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
180 |
180 |
|
69 |
69 |
|
44.54% |
44.54% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7812 |
Baseline #7798 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
79.77KiB |
79.77KiB |
Bundle analysis report Branch fix/external-bundle-css-diagnost... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9385 Bundle Size — 900.03KiB (0%).2d0afe0(current) vs 5f3b6eb main#9371(baseline) Bundle metrics
|
| Current #9385 |
Baseline #9371 |
|
|---|---|---|
44.46KiB |
44.46KiB |
|
2.22KiB |
2.22KiB |
|
0% |
0% |
|
9 |
9 |
|
11 |
11 |
|
228 |
228 |
|
11 |
11 |
|
27.29% |
27.29% |
|
10 |
10 |
|
0 |
0 |
Bundle size by type no changes
| Current #9385 |
Baseline #9371 |
|
|---|---|---|
495.9KiB |
495.9KiB |
|
401.92KiB |
401.92KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch fix/external-bundle-css-diagnost... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#78 Bundle Size — 198.12KiB (0%).2d0afe0(current) vs 5f3b6eb main#64(baseline) Bundle metrics
Bundle size by type
|
| Current #78 |
Baseline #64 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
52.36KiB |
52.36KiB |
Bundle analysis report Branch fix/external-bundle-css-diagnost... Project dashboard
Generated by RelativeCI Documentation Report issue
Merging this PR will not alter performance
Comparing Footnotes
|
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
Checklist