chore(eslint-plugin-internal): add no-style-prop-css-overrides rule#768
Open
cb-ekuersch wants to merge 3 commits into
Open
chore(eslint-plugin-internal): add no-style-prop-css-overrides rule#768cb-ekuersch wants to merge 3 commits into
cb-ekuersch wants to merge 3 commits into
Conversation
Flags top-level CSS declarations inside Linaria `css` blocks (scoped to cds-web) for properties already owned by a CDS style prop (height, width, padding-*, background-color, box-shadow, etc.). Such a declaration sits at the same single-class specificity as the base style-prop class but is emitted later in the stylesheet, so it wins the CSS source-order tiebreaker and silently overrides values consumers pass via the matching style prop. This is the class of bug fixed in CDS-2118 (Button height/width props not applying). Only top-level declarations are flagged; nested selectors/pseudo-states/ at-rules and multi-value padding/margin shorthands are allowed. Scoped in eslint.config.mjs to packages/web/src (excluding the style system, stories, tests, mocks, fixtures, and Figma files) at the `warn` level. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
🟡 Heimdall Review Status
🟡
|
| Code Owner | Status | Calculation | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| ui-systems-eng-team |
🟡
0/1
|
Denominator calculation
|
…-location aware
Reduce false positives by only flagging a CSS property hard-coded in a Linaria
`css` class when the element wearing that class is *also* explicitly passed the
matching cds-web style prop. Previously the rule flagged every top-level owned
property in any `css` block, which incorrectly caught properties components
legitimately hard-code but never expose as props on that element (e.g. Button's
baseCss `display: inline-flex`).
The element's `className` is resolved through identifiers, inline `css`,
`cx`/`cn`/`clsx`/`classnames`, and logical/conditional/array/template
expressions; padding/margin shorthands and longhands are compared per physical
side. Implemented via AST (explicit JSX attributes) rather than the type
checker: the repo doesn't enable type-aware linting for web, and matching a
`{...props}` spread's type would re-surface style props a component extends but
never intends to expose, reinstating the false positives. typescript-eslint
also discourages rules whose behavior depends on type-info availability.
Validated against packages/web/src: catches real latent overrides (e.g. Button
`minWidth` vs `min-width: unset`, Text `fontFamily`/`display` via conditional
css classes) with no false positives on hard-coded, non-exposed properties.
Co-authored-by: Cursor <cursoragent@cursor.com>
… in no-style-prop-css-overrides
Extend the rule to catch style props that reach an element through a
`{...spread}` rather than an explicit attribute. The rule now resolves the
TypeScript type of each spread argument and treats any property whose name is a
cds-web style prop as able to land on the element, then flags it when a
co-located Linaria `css` class hard-codes the same CSS property.
This catches the spread-forwarding footgun (e.g. Button hard-coding
`height: fit-content` in baseCss while `height` flows to Pressable via
`{...props}`), which the explicit-attribute check missed. Explicit attributes
take precedence over spreads for reporting, and a prop destructured out of the
spread is correctly not flagged.
Because the spread analysis requires type information, the rule's config block
in eslint.config.mjs now enables parserOptions.projectService for the in-scope
packages/web/src TS/TSX files (per typescript-eslint guidance, the rule requires
type info rather than degrading when it is absent). Tests are converted to the
type-aware RuleTester (projectService) and cover spread, destructure-out, and
explicit-precedence cases.
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed? Why?
Adds a new ESLint rule
internal/no-style-prop-css-overridestolibs/eslint-plugin-internal, scoped to cds-web (packages/web/src).The rule flags top-level CSS declarations inside Linaria
cssblocks for properties that are already owned by a CDS style prop (height,width,min/max-*, flex/grid sizing,top/left/...,opacity,z-index,color,background-color,box-shadow→elevation,border-*,font-*,line-height,padding-*,margin-*,gap,display,position,overflow,align-*,justify-content, etc.).Why: cds-web style props compile to single-class Linaria rules (the value is injected as an inline CSS variable, but the declaration consuming it lives in a class), which keeps style props consumer-overridable. A component's own
cssblock sets the same property at the same specificity but is emitted later in the stylesheet, so it wins the CSS source-order tiebreaker and silently overrides values passed via the style prop. The rule turns this implicit footgun into a lint warning.What it intentionally allows (no false positives):
cursor,transition, …).&:hover,@media, descendant selectors) — these can't be expressed via static style props.padding/marginshorthands (e.g.padding: 4px 8px).cssimported from anywhere other than@linaria/core.Scoped in
eslint.config.mjstopackages/web/src/**, excluding the style system itself (packages/web/src/styles/**), stories, tests, mocks, fixtures, and Figma Code Connect files. Severity iswarn.Includes a dedicated rule README (
src/no-style-prop-css-overrides/README.md) with the full cascade rationale and remediation guidance, plus a summary entry in the plugin README.Root cause (required for bugfixes)
Not a bugfix — this is a preventative lint rule. It guards against the class of bug fixed in CDS-2118 (
Buttonheight/widthprops not applying), where a component'scssblock declaredheight/widththat out-ordered the style-prop class in the cascade.UI changes
N/A — lint/tooling-only change with no runtime or visual impact.
Testing
How has it been tested?
Testing instructions
yarn nx run eslint-plugin-internal:test --testPathPattern=no-style-prop-css-overridesheight: 40px) to acssblock inpackages/web/src/**and confirminternal/no-style-prop-css-overrideswarns; confirm the same declaration inside&:hover { ... }does not warn.Illustrations/Icons Checklist
N/A — no changes under
packages/illustrations/**orpackages/icons/**.Change management
type=routine
risk=low
impact=sev5
automerge=false
Made with Cursor