Skip to content

docs(rules): correct what the unit suite can see, and name the dead-shorthand trap #888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
isaque-bock-azion merged 3 commits into from
Aug 12, 2026
Merged

docs(rules): correct what the unit suite can see, and name the dead-shorthand trap #888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef9d6cf
docs(rules): correct what the unit suite can see, and name the dead-s…
gabriel-lisboa-azion Aug 12, 2026
1339f9e
Merge branch 'main' into docs/token-shorthand-and-test-css
herbert-julio-azion Aug 12, 2026
49e5f2f
Merge branch 'main' into docs/token-shorthand-and-test-css
isaque-bock-azion Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .claude/rules/styling.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,25 @@ Three shapes:

The guardrail token-checks (`typography-raw-length`, `leading-raw`, `tracking-raw`, `font-family-raw`, `animate-arbitrary`, `motion-hardcoded`) match **both** spellings, so accepting the IntelliSense suggestion cannot walk a raw value through the typography / motion / animation gates.

### A malformed shorthand is dead, and looks alive

The shorthand has no tolerance: the parens hold **exactly** `--token` or `type:--token`, nothing else. Get it slightly wrong and the style never reaches the element, while the class keeps reading as correct — in a 1000-character root `class` string, forever.

| Written | What actually happens |
| --- | --- |
| `bg-(--bg-surface )` | The space **terminates the candidate**. Tailwind sees the unterminated `bg-(--bg-surface` and emits **nothing**. |
| `bg-( --bg-surface)` | Same — whitespace anywhere inside the parens. |
| `min-h-[--(size-4)]` | Bracket/paren inverted. This one *does* emit a rule — `min-height: --(size-4)`, an invalid value the **browser discards at parse time**. |

Both shipped: chip's `filled` kind was fully transparent in both themes from the day it shipped (the first form), and `popover-header`'s min-height was inert from the v4 sweep until 2026-08-12 (the third). Neither is a lint, type, or build error on its own, and **a visual baseline generated from the broken render encodes the bug as correct** — so nothing downstream fails either.

Two consequences worth internalizing:

- **A dead class is invisible to the unit suite.** Vitest browser mode runs without Tailwind, so a computed-style assertion returns the same value whether the class works or does not exist (see [`testing.md`](./testing.md) § "What a real browser does NOT give this suite"). Verify a fill by sampling the rendered pixel, or probe the class through the v4 compile API — not through `getComputedStyle` in a test.
- **When a style "does nothing", suspect the class before the cascade.** Probe it (`compile(...).build(['the-class'])`) with a deliberately bogus class in the list as a control; if the bogus one also "emits", the probe is measuring nothing.

The **`dead-token-shorthand`** token-check blocks both spellings at write time and in the CI ratchet. It scans raw file text, so a comment that *quotes* a malformed class trips it too — describe such a class in prose instead of spelling it out.

## A zero length carries no unit

A zero is the one value that is identical in every unit, so the unit is pure noise — and it makes the same zero read three different ways across the codebase (`0px` here, `0rem` there, `0em` in a token). **Write `0`.**
Expand Down Expand Up @@ -219,6 +238,7 @@ padding: 0 var(--spacing-md);
## Hard prohibitions

- No zero with a length unit — `0`, never `0px` / `0rem` / `0em` (in tokens, arbitrary Tailwind values, inline `style`, or authored CSS). The single exception is inside `calc()`/`min()`/`max()`/`clamp()`, where CSS requires a unit and that unit is **`rem`**.
- No malformed token shorthand — no whitespace inside the parens (`bg-(--token )`, `bg-( --token)`) and never the inverted `[--(token)]`. The style silently never applies; blocked by `dead-token-shorthand`.
- No `const sharedClasses = [...]`, `const kindClasses = {...}`, `const sizeClasses = {...}`, `const rootClasses = computed(...)`. The whole "class map" pattern goes away.
- No `<style>` blocks (scoped or unscoped).
- No `.css` / `.scss` files inside a component directory.
Expand Down Expand Up @@ -308,6 +328,7 @@ Use a `data-*` attribute + a Tailwind variant. The decision lives in HTML, not i

- `scaffolder` (agent) refuses to emit the `kindClasses`/`sizeClasses`/`sharedClasses`/`rootClasses` pattern. The skeleton in [`.claude/skills/component-scaffold/SKILL.md`](../skills/component-scaffold/SKILL.md) uses inline classes + `data-*` variants.
- `validate-tokens.mjs` (PreToolUse hook) already blocks HEX/palette/raw typography regardless of where they appear.
- **`dead-token-shorthand`** (same shared token-checks engine, so write-time hook **and** the `check-authoring` CI ratchet) blocks a malformed shorthand — whitespace inside the parens, or the inverted `[--(token)]`. It is the only gate that sees this class of defect: the utility never exists, so nothing else in the pipeline has anything to complain about, and the unit suite runs unstyled. Pinned both directions (fires / stays silent on ordinary subtraction and on a nested `var()` fallback) in [`token-checks.test.mjs`](../../packages/webkit/test/eslint-plugin/token-checks.test.mjs).
- **Zero-unit** is gated on four surfaces, so no authoring path escapes it: the `zero-with-unit` check in the shared token-checks engine (write-time hook **and** the `check-authoring` CI ratchet, over component sources); `length-zero-no-unit` in [`.stylelintrc.json`](../../.stylelintrc.json) for authored CSS/SCSS/Vue `<style>` — set to plain `true` so the preset's `ignore: ['custom-properties']` does **not** apply, since a design system is authored almost entirely as custom properties; the same rule in the shipped [`stylelint-config.js`](../../packages/webkit/src/stylelint-config.js) so consumers inherit it; and a build-time assertion in the theme's `build:tokens`, which is the only gate that sees token values (they are authored in JS and compiled, so no linter reads them). A `length-zero-no-unit` canary fixture keeps the stylelint side from being relaxed.
- **The `rem`-in-math-function carve-out** is gated by the two engines we own — the `zero-unit-in-calc` token check and the same assertion in `build:tokens`. Stylelint's `length-zero-no-unit` deliberately skips math functions (a unit is required there), so it accepts `calc(100% - 0px)`; the token check is what makes that `0rem`.
- A future PostToolUse hook may grep `.vue` files for `const \w+Classes\s*=\s*[\{[]` and emit `BLOCKED: forbidden class preset` — until then, `echo-reporter` flags the pattern.
Expand Down
42 changes: 26 additions & 16 deletions .claude/rules/testing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,26 @@ Every component ships a co-located `*.test.ts` that proves it **works**, exercis

## Why browser mode, never jsdom

jsdom returns no-ops for `focus`, `document.activeElement`, layout/`getBoundingClientRect`, and does not surface `<Teleport>`d content — so a test that "passes" there is a false positive for exactly the behaviors that break in production (keyboard, focus trap, overlays, positioning, contrast). We run in real Chromium so those are real.
jsdom returns no-ops for `focus`, `document.activeElement`, layout/`getBoundingClientRect`, and does not surface `<Teleport>`d content — so a test that "passes" there is a false positive for exactly the behaviors that break in production (keyboard, focus trap, overlays, positioning). We run in real Chromium so those are real.

- **No mocks for layout / positioning / focus / `<Teleport>`.** If a test "needs" one of those mocks, the test is wrong. Real browser makes them real.
- Teleported overlay content escapes the render container — query it from `document.body`, not the `render()` result.

### What a real browser does NOT give this suite: CSS

**This env runs no Tailwind, and [`setup.ts`](../../packages/webkit/src/test/setup.ts) deliberately loads no theme CSS.** The DOM is real but _unstyled_: a component's utility classes emit nothing here, so every computed style is the UA default. That is a boundary, not a gap to fix — and it decides what a test may assert:

- **Never assert a computed style, a color, or a utility-derived dimension.** `getComputedStyle(el).backgroundColor` reads `rgba(0, 0, 0, 0)` whether the class is correct, misspelled, or missing entirely, so such an assertion passes in the broken _and_ fixed states — the exact false positive browser mode exists to kill. Same for a height that comes from `h-8`.
- **`expectNoA11yViolations` therefore checks semantics, not pixels** — role, name, ARIA relationships, focus order. It does **not** validate color contrast here, because there are no colors. Contrast is real only where the stylesheet is: Storybook + the visual-regression gate.
- **Pixels, contrast and token correctness belong to visual regression.** A dead utility class (a token that emits no CSS) is invisible to this suite by construction; it is caught by [`styling.md`](./styling.md)'s token checks at write time and by the visual gate at review time.

Do not re-add `@aziontech/theme/globals.css` to `setup.ts` without also wiring the Tailwind pipeline — the tokens alone would load and nothing would read them, which buys a slower suite and no new signal.

## The stack (already wired — do not reinvent)

- `packages/webkit/vitest.config.ts` — `@vitejs/plugin-vue`, `browser: { provider: playwright(), instances: [{ browser: 'chromium' }], headless: true }`, `define: { 'process.env.NODE_ENV': ... }` (so `@testing-library/vue`'s `fireEvent` runs in the browser), `retry: process.env.CI ? 2 : 0`. Story imports of `@aziontech/webkit/*` resolve through the workspace package itself (no alias needed).
- `packages/webkit/src/test/setup.ts` — imports `@aziontech/theme/globals.css` (styled DOM ⇒ axe contrast is real) + `cleanup()`.
- `packages/webkit/src/test/axe.ts` — `expectNoA11yViolations(container)`.
- `packages/webkit/src/test/setup.ts` — `cleanup()` + an anchor-navigation guard (a real click on an `<a href>` would navigate and tear down the test iframe). It loads **no** CSS, on purpose — see the CSS boundary above.
- `packages/webkit/src/test/axe.ts` — `expectNoA11yViolations(container)`; semantics only, no contrast (unstyled DOM).
- `.github/workflows/governance.yml` — the `tests` job runs Vitest browser mode sharded (×4) + retry, only when webkit/storybook changes; the `toolkit` job runs `test:gate` (existence).
- Publish-safety: `packages/webkit/package.json#files` negates `*.test.ts` and `src/test/**` (verified with `pnpm --filter webkit pack:dry`). Test files never ship to npm.

Expand All @@ -34,25 +44,25 @@ jsdom returns no-ops for `focus`, `document.activeElement`, layout/`getBoundingC

## What every `<name>.test.ts` must cover

| # | Surface | Assertion |
| --- | ---------------- | ------------------------------------------------------------------------------------------------------------------------- |
| 1 | Render | mounts without throwing; the `data-testid` fallback is present; consumer `data-testid` override wins |
| 2 | Props / variants | each variant prop (`kind`, `size`, …) maps to its `data-*` / attribute / rendered state |
| 3 | Events | every event in the spec's Events table fires with the right payload on the real user action |
| 4 | Suppression | when `disabled` / `loading` / `readonly`, the action is **not** emitted |
| 5 | v-model | drive the input, assert `update:modelValue` (and `update:open` / `update:*`) with the exact value |
| 6 | ARIA | `role`, `aria-expanded`, `aria-busy`, `aria-disabled`, `aria-selected`… as the template declares |
| 7 | a11y | `expectNoA11yViolations(container)` on the default render + any variant whose semantics differ |
| 8 | Composition | a context-aware sub-component reflects/drives the root's `provide`/`inject` state with no manual wiring |
| 9 | Overlay | open/close (trigger + second click), `Escape` closes and returns focus, panel Teleports to `body`, scroll-lock while open |
| 10 | Recursive | nested instances ≥2 levels deep render and propagate context (active item, open submenu, orientation) |
| # | Surface | Assertion |
| --- | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| 1 | Render | mounts without throwing; the `data-testid` fallback is present; consumer `data-testid` override wins |
| 2 | Props / variants | each variant prop (`kind`, `size`, …) maps to its `data-*` / attribute / rendered state |
| 3 | Events | every event in the spec's Events table fires with the right payload on the real user action |
| 4 | Suppression | when `disabled` / `loading` / `readonly`, the action is **not** emitted |
| 5 | v-model | drive the input, assert `update:modelValue` (and `update:open` / `update:*`) with the exact value |
| 6 | ARIA | `role`, `aria-expanded`, `aria-busy`, `aria-disabled`, `aria-selected`… as the template declares |
| 7 | a11y | `expectNoA11yViolations(container)` on the default render + any variant whose semantics differ (semantics only — contrast needs CSS, which this env has none of) |
| 8 | Composition | a context-aware sub-component reflects/drives the root's `provide`/`inject` state with no manual wiring |
| 9 | Overlay | open/close (trigger + second click), `Escape` closes and returns focus, panel Teleports to `body`, scroll-lock while open |
| 10 | Recursive | nested instances ≥2 levels deep render and propagate context (active item, open submenu, orientation) |

A tiny `it.each` smoke over enum variants ("mounts without throwing") is a **floor**, never the substance.

## The functional bar — no false positives, no filler

- Assert **only what you read** in the source. Never invent props/events/testids/aria/sub-components.
- **Forbidden:** assertions on Tailwind/class strings, pixel positions, animation timing, or internal component state.
- **Forbidden:** assertions on Tailwind/class strings, computed styles/colors, pixel positions, animation timing, or internal component state. A computed-style assertion is not a stricter version of a class assertion — it is a _weaker_ one here, because the unstyled DOM returns the same value whether the style is right or absent.
- **If a test only passes when the implementation is written one specific way, delete it.** It traps refactors and adds no signal.
- If a test reveals a real component defect you cannot satisfy without changing the `.vue`, **`it.skip` it with a one-line reason** — never fake a pass or weaken an assertion into meaninglessness. Record the gap in the PR.

Expand Down
Loading