refactor(desktop): remove dead shell CSS recipes and close check-dead-css blind spots - #2070
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Approve. Two scanner issues to address before merge:
- Comma-group selector liveness (check-dead-css.mjs:282).
.a, .b { ... var(--t) }with only.blive drops the rule wholesale because of.every, so--tis misattributed as dead. No comma-group class rule exists in the token sheet today, but the fix is small: split each selector, run.everyper branch, combine with.some. - The class scan misses packages/ui/src/styles.css (check-dead-css.mjs:323), which is imported into the product CSS. It currently holds four classes with no consumers:
.maka-daily-review-append/copy/quick-run/save(the daily-review panel was simplified in July). The three listed blind spots are closed; this one is not. Add the file to the scan and delete the four, or document why the component-library stylesheet is exempt.
Also on record, non-blocking: the fixpoint tests lack self-reference, cycle, and diamond cases; the reservation comment for--ease-in-out-strongand the motion-governance comment in maka-tokens.css disagree about the shimmer curve.
… tests Review round on apache#2070: - A comma group applies when ANY branch matches, so rule liveness now splits the selector and asks .every per branch, .some across branches — one live branch keeps the group's token reads, all-dead branches do not. - packages/ui/src/styles.css joins the class scan (it is imported into the product CSS); its four consumer-less daily-review button classes are deleted, and the runtime-composed maka-markdown-code-{default, compact} density variants the scan then surfaced join DYNAMIC_STYLE_HOOKS with the composing call sites named. - Fixpoint tests gain self-reference, cycle, and diamond cases, plus the comma-group case. - The --ease-in-out-strong reservation comment no longer claims the shimmer sweep uses --ease-linear (the live text shimmer animates with bare `linear`); it now matches the motion governance comment.
|
All addressed in b0cfb7f, including the non-blocking notes:
|
|
The Everything this PR owns is green on the same run: typecheck (biome/knip), storybook, e2e, test_workspaces, test_runtime_host. |
|
Rebase to the latest main would fix it. |
…-css blind spots The scanner had three blind spots that made `--check` report a clean sheet while ~13 dead legacy shell recipes sat in maka-tokens.css: - maka-tokens.css was scanned for tokens only, so its class rules were invisible to the class sweep. It now joins the scan. - The consumer check was a substring match, so `maka-shell-astryx` kept `.maka-shell` alive and `maka-chat-message-*` kept `.maka-chat` alive. Consumers now require an exact class-name match (hyphen-aware boundaries). - Token reads inside the token sheet unconditionally seeded the live set, so `--w-rail`/`--w-sidebar` were kept alive by the very dead rules that read them. Sheet-internal reads now only count when they can fire: reads in class rules whose classes have consumers, and token-to-token derivations resolved as a fixpoint from external reads. With the scanner fixed, delete what it now sees, all verified zero-consumer: the .maka-shell/.maka-sidebar/.maka-titlebar/.maka-chat/ .maka-composer-toolbar recipe family, .maka-shimmer plus its keyframes (folded into functional-motion.stories.tsx, its only consumer), --w-rail/--w-sidebar, and two prefix-collision finds the original audit missed: .maka-plan-card (plan-reminders.css) and .maka-skill-tab (theme-glass.css). --ease-in-out-strong joins RESERVED_SCALE_TOKENS as the named movement curve of the easing vocabulary rather than being deleted with its last consumer. While in the theme pipeline: @astryxdesign/cli 0.2.0 stopped emitting maka.variants.d.ts but its maka.d.ts still opens with a reference to it; the build now strips the dangling line instead of leaning on skipLibCheck (maka.d.ts regenerated). New scripts/check-dead-css.test.mjs and build-astryx-theme.test.mjs cover the prefix-collision and token-read-only-by-dead-rules fixtures from the issue's acceptance criteria, wired into test:scripts. Closes apache#1980. Verification: check-dead-css --check clean at baseline 0/0 with the stricter matching; astryx:theme --check current; typecheck clean; script tests 19/19.
… tests Review round on apache#2070: - A comma group applies when ANY branch matches, so rule liveness now splits the selector and asks .every per branch, .some across branches — one live branch keeps the group's token reads, all-dead branches do not. - packages/ui/src/styles.css joins the class scan (it is imported into the product CSS); its four consumer-less daily-review button classes are deleted, and the runtime-composed maka-markdown-code-{default, compact} density variants the scan then surfaced join DYNAMIC_STYLE_HOOKS with the composing call sites named. - Fixpoint tests gain self-reference, cycle, and diamond cases, plus the comma-group case. - The --ease-in-out-strong reservation comment no longer claims the shimmer sweep uses --ease-linear (the live text shimmer animates with bare `linear`); it now matches the motion governance comment.
b0cfb7f to
686f682
Compare
Summary
Implements #1980 end to end: fixes the three
check-dead-cssblind spots, then deletes everything the fixed scanner reveals — the audit's 13 dead rules were still present onmain(the gate printed "no dead classes or tokens found ✓" over them the whole time).Scanner fixes (
scripts/check-dead-css.mjs)maka-tokens.css— previously it was scanned for tokens only, so.maka-composer-toolbarand.maka-shimmerwere invisible outright.maka-shell-astryxkeep.maka-shellalive,maka-chat-message-*keep.maka-chatalive, etc. Consumers now match with hyphen-aware boundaries (\balone treats-as a boundary, so plain word-boundary matching would not have fixed the collision).var()read in a class rule counts only when the rule's classes have consumers; a read inside another token's value counts only once that token is live (fixpoint over derivation edges). This is what let the dead shell rules keep--w-rail/--w-sidebaralive.The script also gains an
isMainguard and exports its pure pieces for the newscripts/check-dead-css.test.mjs(fixtures cover both acceptance scenarios: prefix-collision and token-read-only-by-dead-rules). Wired intotest:scripts.Deletions (all zero-consumer under the fixed scanner, re-verified individually)
maka-tokens.css:.maka-shell,.maka-shell-rail-right, the.maka-sidebarheader/section/row/button family,.maka-titlebar(+strong),.maka-chat,.maka-composer-toolbar,.maka-shimmer+ its@keyframes(folded intofunctional-motion.stories.tsx, its only consumer), and--w-rail/--w-sidebar..maka-main,.maka-turn,.maka-codestay — they are live..maka-plan-card(plan-reminders.css; its "consumer" wasmaka-plan-card-menu) and.maka-skill-tab(theme-glass.css; its "consumer" wasmaka-skill-tabs-bar).--ease-in-out-stronglost its last consumer with.maka-shimmer, but it is the named movement curve of the motion governance vocabulary (out-strong = feedback, in-out-strong = movement), so it joinsRESERVED_SCALE_TOKENSwith that rationale instead of being deleted.Theme build (
scripts/build-astryx-theme.mjs)@astryxdesign/cli0.2.0 stopped emittingmaka.variants.d.tsbut itsmaka.d.tsstill opens with a/// <reference>to it (masked byskipLibCheck). Post-processing now strips the dangling line;maka.d.tsregenerated (css/js byte-identical),astryx:theme --checkcurrent. Covered byscripts/build-astryx-theme.test.mjs.Closes #1980.
Verification
check-dead-cssreports the 13 rules + 3 tokens before the deletions, and is clean at baseline 0/0 after;--checkpasses.npm run astryx:theme -- --check: artifacts current.npm run typecheck: clean across all workspaces.check-story-annotationsclean.Note:
dev-app-runtime.test.mjs › ignores an unreadable or wrong-schema environment filefails on my machine both with and without this change (it picks up the developer's real local env file — a pre-existing test-isolation issue, unrelated).