rusackas opened a new pull request, #41871:
URL: https://github.com/apache/superset/pull/41871
### SUMMARY
Emotion template-literal styles — ``css`...` `` and ``styled.x`...` `` — are
opaque strings as far as TypeScript is concerned, so invalid CSS inside them is
silently dropped by the browser at runtime with nothing to catch it at build
time. That's exactly how the `verticalalign: middle` typo fixed in #41052
slipped in: a valid property mangled into an unknown one, ignored by every
browser and every tool we run.
Emotion's own ESLint plugin (`@emotion/eslint-plugin`) doesn't help here —
it only covers import and syntax-preference hygiene, not CSS property validity.
Object styles (`css({ verticalAlign })`) get `csstype` checking for free, but
template-literal styles get none, and we use template literals everywhere.
This PR adds **Stylelint** with the
[`postcss-styled-syntax`](https://github.com/hudochenkov/postcss-styled-syntax)
custom syntax so the CSS *inside* those template literals is actually parsed
and linted. The rule set is deliberately scoped to high-signal, low-noise
checks rather than a full `stylelint-config-standard` (which would bury us in
style-opinion noise):
- `property-no-unknown` — the rule that would have caught #41052
- `unit-no-unknown`, `function-no-unknown`, `color-no-invalid-hex`,
`named-grid-areas-no-invalid`, `string-no-newline`
- `no-invalid-double-slash-comments` — `//` isn't valid CSS; it only "works"
because stylis strips it
- `declaration-block-no-duplicate-properties` (allowing consecutive
fallbacks) — catches declarations silently overriding each other
It's wired into the frontend lint pipeline the same way
`oxlint`/custom-rules are: a `stylelint`/`stylelint-fix` npm script, inclusion
in `lint:full`, and a `stylelint-frontend` pre-commit hook.
**Baseline cleanup** (to make the gate green — no functional/visual change):
- Converted `//` line comments inside CSS template literals to `/* */`
across ~23 files.
- Fixed 6 duplicate declarations that were silently overriding each other
(`white-space`, `color`, `padding`, `cursor`, `vertical-align`,
`background-repeat`). The overridden (dead) declaration was removed, preserving
rendered output.
- `.stylelintignore`s 4 files whose template literals nest a `css` tag
inside an interpolation (`styled(X)\`${p => css\`...\`}\``), a pattern
`postcss-styled-syntax` can't parse yet.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no visual change. The baseline fixes preserve rendered output
(dead/overridden declarations and inert comments only).
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
npm run stylelint # passes clean on the whole tree
```
To see it catch a bug, drop `verticalalign: middle;` into any ``css`...` ``
block and re-run — it reports `Unexpected unknown property "verticalalign"`.
The `stylelint-frontend` pre-commit hook runs the same check on staged files.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
- [x] Other: adds a frontend linter (Stylelint for CSS-in-JS) + tooling
Follow-up worth considering: the 4 ignored files point at a real
`postcss-styled-syntax` limitation with `css`-in-interpolation; if that pattern
gets refactored (or the syntax gains support) they can come off the ignore
list. A couple of the unparseable files also have genuinely stray `}` braces
that stylis tolerates — left alone here to keep this PR about the tooling.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
