fix: buildCssSnippet self-referential var() bug + slot styling guidance#158
fix: buildCssSnippet self-referential var() bug + slot styling guidance#158
Conversation
…ce + font inheritance warnings - Fix buildCssSnippet generating circular --token: var(--token) CSS - Use CEM default values or smart property-name heuristics for placeholder values - Add slot styling section showing correct light DOM CSS patterns - Add font vs layout inheritance anti-pattern warning for slotted content Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe changes extend a styling diagnostics handler to emit warnings about CSS inheritance expectations for slotted elements, add slot styling guidance to CSS snippets with light-DOM selectors, update token customization output with default values, and introduce a helper function to generate placeholder CSS values. Tests validate these features, and a benchmark fixture is refreshed with new timing metrics. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![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
🤖 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/core/src/handlers/styling-diagnostics.ts`:
- Around line 247-250: The generated slot CSS includes an extra space when
slot.description is empty; update the formatting in the loop over namedSlots so
you conditionally render the description and its surrounding spaces rather than
always inserting desc.trim(). Specifically, in the block that builds desc and
calls lines.push (referencing namedSlots, desc, tag, slot.name, and the
lines.push call), change the push so it emits `{ ${slot.description.trim()} }`
only when a non-empty description exists and otherwise emits `{ }` (no double
space).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fbdd8743-8084-484c-8f96-ed9879817082
📒 Files selected for processing (3)
packages/core/src/handlers/styling-diagnostics.tstests/__fixtures__/benchmark-results/latest-benchmark.jsontests/handlers/styling-diagnostics.test.ts
| for (const slot of namedSlots.slice(0, 3)) { | ||
| const desc = slot.description ? ` /* ${slot.description} */` : ''; | ||
| lines.push(`${tag} [slot="${slot.name}"] { ${desc.trim()} }`); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor formatting inconsistency when slot description is empty.
When slot.description is falsy, desc is '' and desc.trim() produces an empty string, resulting in output like my-button [slot="prefix"] { } with a double space before the closing brace. This is purely cosmetic but slightly inconsistent with other generated CSS.
✨ Optional: Clean up empty description handling
for (const slot of namedSlots.slice(0, 3)) {
const desc = slot.description ? ` /* ${slot.description} */` : '';
- lines.push(`${tag} [slot="${slot.name}"] { ${desc.trim()} }`);
+ lines.push(`${tag} [slot="${slot.name}"] {${desc} }`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const slot of namedSlots.slice(0, 3)) { | |
| const desc = slot.description ? ` /* ${slot.description} */` : ''; | |
| lines.push(`${tag} [slot="${slot.name}"] { ${desc.trim()} }`); | |
| } | |
| for (const slot of namedSlots.slice(0, 3)) { | |
| const desc = slot.description ? ` /* ${slot.description} */` : ''; | |
| lines.push(`${tag} [slot="${slot.name}"] {${desc} }`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/handlers/styling-diagnostics.ts` around lines 247 - 250,
The generated slot CSS includes an extra space when slot.description is empty;
update the formatting in the loop over namedSlots so you conditionally render
the description and its surrounding spaces rather than always inserting
desc.trim(). Specifically, in the block that builds desc and calls lines.push
(referencing namedSlots, desc, tag, slot.name, and the lines.push call), change
the push so it emits `{ ${slot.description.trim()} }` only when a non-empty
description exists and otherwise emits `{ }` (no double space).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
buildCssSnippetwas generating circular CSS like--my-button-bg: var(--my-button-bg)— agents copying this would produce broken styles. Now generates correct override patterns using CEM defaults or smart property-name heuristics.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements