Consolidate @supports gating checks and fix ungated color-mix() in components (re-merge of #565) - #567
Conversation
…ity-audit-dlbw5f" This reverts commit 9445ffa.
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
✨ 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 |
PR Summary by QodoConsolidate @supports gating checks and fix ungated color-mix() usage
AI Description
Diagram
High-Level Assessment
Files changed (18)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
12 rules 1. @supports suffix false match
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // Match `@supports` only at a token boundary so it is never matched inside | ||
| // a selector or value. | ||
| if (src.startsWith('@supports', i) && (i === 0 || /[\s{}();,]/.test(src[i - 1]))) { | ||
| // Skip the prelude to the body-opening `{` at parenthesis depth 0. | ||
| let j = i + '@supports'.length; | ||
| let paren = 0; | ||
| while (j < n) { | ||
| const c = src[j]; | ||
| if (c === '"' || c === "'") { j = skipString(j); continue; } | ||
| if (c === '(') paren++; | ||
| else if (c === ')') paren--; | ||
| else if (c === '{' && paren === 0) break; | ||
| j++; |
There was a problem hiding this comment.
1. @supports suffix false match 🐞 Bug ≡ Correctness
stripSupports() treats any text starting with "@supports" as the real at-rule without verifying a
right-side token boundary, so an at-rule like "@supports-foo {…}" would be stripped and could hide
ungated modern expressions from P2/P7.
Agent Prompt
### Issue description
`tests/supports-helpers.js` `stripSupports()` matches `@supports` by prefix only. If future CSS contains an unknown/custom at-rule whose name begins with `supports` (e.g. `@supports-foo { ... }`), the scanner will incorrectly treat it as a real `@supports` block and remove it, potentially masking ungated `color-mix()`/`light-dark()`/`oklch(from ...)` usages.
### Issue Context
This helper underpins both the source-level (P2) and bundle-level (P7) gating tests, so a false match here can create false negatives across the gating suite.
### Fix Focus Areas
- tests/supports-helpers.js[51-63]
### Suggested change
Tighten the `@supports` detection to also require a **right-side boundary** (i.e., the character after `@supports` must not be a CSS ident character like `[A-Za-z0-9_-]`). For example:
- Compute `const end = i + '@supports'.length; const next = src[end] ?? ''`
- Only treat it as `@supports` if `next` is empty OR `next` is a delimiter like whitespace / `(` / `{` / etc., and specifically **not** `[-\w]`.
Optionally add a small unit test in `tests/supports-helpers.test.js` asserting that `@supports-foo { ... }` is *not* stripped.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2 participants
Re-introduces the changes from #565 as a cleanly-mergeable PR.
#565's original web merge succeeded at the git level but the request errored before GitHub could update its status, leaving it frozen as "Closed" (GitHub refused to reopen it). Since the code was already in
main, no fresh PR could show a diff. This was resolved by reverting it (#566) and re-adding it here, so the work lands in history with a proper "Merged" badge. The final tree is byte-for-byte identical to the original merge.Summary
Consolidates the fragmented @supports-gating validation logic into a shared, robust scanner (
tests/supports-helpers.js), expands P2 coverage fromtokens.cssalone to all source CSS files, and fixes an ungatedcolor-mix()declaration inoptional/components.cssthat violated the framework's modern-expression gating policy.Key changes
tests/supports-helpers.js): ImplementsstripSupports()andfindUngatedModernExpressions()helpers that are character-based, parenthesis-aware, and comment-stripped. Replaces the line-based scanner in P7 that historically miscounted braces inside@supportsprelude expressions (e.g.,@propertyfeature queries).tests/tier1-p2-coverage.test.js): Now scans allcore/*.cssandoptional/*.cssfiles (not justtokens.css), catching modern expressions used directly in component/utility rules. Uses the shared scanner so P2 and P7 can never drift.tests/tier1-p7-oldengine.test.js): Adds a second check that scans every declaration in the built bundle (custom properties AND plain properties).optional/components.css): Wrapped the.sf-btn--secondary:hoverand.sf-btn--ghost:hovercolor-mix()background declarations in an@supportsblock.tests/configurator-data-contract.test.js): Validates generated configurator data files maintain their required schema.metaFor()helper inscripts/lib/api-index/extract.jsand theFILE_TITLESpattern inscripts/gen-class-reference.jsfail loudly on missing metadata.scripts/lib/parse.js):stripStrings()andmaskStrings()now honour backslash escapes.scripts/check-artifacts.js): Declared artifact outputs must be git-tracked.scripts/artifacts.json): Addedclasses.generated.jsonand the full-api demos.CLAUDE.md; status headers on historical reports.lightningcssfrom devDependencies.Generated by Claude Code