From 10882bea739a98f11b942c20c6570eb6126dd010 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 22 Sep 2026 21:33:03 -0700 Subject: [PATCH] chore(web): no-restyle fails lint, and the ceiling gate goes (#13210) Co-authored-by: Claude Opus 5.5 (1M context) --- .github/workflows/ci.yml | 3 -- AGENTS.md | 2 +- package.json | 1 - scripts/lint-restyle-ceiling.test.ts | 24 ---------- scripts/lint-restyle-ceiling.ts | 66 ---------------------------- vite.config.ts | 6 +-- 6 files changed, 4 insertions(+), 98 deletions(-) delete mode 100644 scripts/lint-restyle-ceiling.test.ts delete mode 100644 scripts/lint-restyle-ceiling.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f65c15696d2cfca234f5ad4441ff8b32ca87b749..611d4cf44f7548a0247caa09394dbf7b242bd944 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,9 +52,6 @@ jobs: - name: Check run: vp check - - name: Check className overrides on components/ui do not grow - run: vp run lint:restyle-ceiling - - name: Typecheck run: vpr typecheck diff --git a/AGENTS.md b/AGENTS.md index 2f3265d7cafd50c95fe7b90e0577464fdf686855..00425996ccc509000ce5a7bd0d0ff99d04d66b8e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -158,7 +158,7 @@ Full glossary with file links: `docs/internals/glossary.md` ## Taste - Complexity belongs at the adapter boundary. Orchestration stays pure, UI stays dumb. -- `apps/web/src/components/ui` exports own their look. Pick a `variant` or `size`; do not restyle one with `className`. If none fits, add a variant to the component, not classes at the call site. Layout classes (width, flex, margin, position) belong on the parent. `shadcn/no-restyle` reports violations and CI caps their count. +- `apps/web/src/components/ui` exports own their look. Pick a `variant` or `size`; do not restyle one with `className`. If none fits and the look is a generic concept, add a variant to the component; a look that belongs to one feature stays in that feature's own component, not in `components/ui`. Layout classes (width, flex, margin, position) belong on the parent. `shadcn/no-restyle` fails lint on violations. - Inferred types over annotations. `any` is the enemy. - Comments describe how a thing is used, and move when the code moves. To be used mostly to describe functions, not to annotate every line of behavior. - Our users drive agents all day and notice a dropped frame, a lying spinner, and a stale label. No continuously repainting animations; they peg the GPU on high-refresh displays. diff --git a/package.json b/package.json index 4345203660d8e386467c4f0c5ded36b43bb81bfc..7caf8bb0845005a6cb9aae286cefa57ee27c9d30 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,6 @@ "knip:check": "knip --include files,dependencies --no-config-hints && knip --workspace apps/server --workspace apps/desktop --workspace apps/web --workspace packages/client-runtime --workspace packages/contracts --workspace packages/effect-acp --workspace packages/effect-codex-app-server --workspace packages/shared --workspace packages/ssh --workspace packages/tailscale --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints", "knip:production": "knip --production --preprocessor ./scripts/knip-schemas.ts", "lint:mobile": "node scripts/mobile-native-static-check.ts", - "lint:restyle-ceiling": "node scripts/lint-restyle-ceiling.ts", "test": "vp run -r test", "test:resource-monitor": "cargo test --locked --manifest-path native/resource-monitor/Cargo.toml", "test:desktop-smoke": "vp run --filter @t3tools/desktop smoke-test", diff --git a/scripts/lint-restyle-ceiling.test.ts b/scripts/lint-restyle-ceiling.test.ts deleted file mode 100644 index 24a92e10cb22552124dd344cf1948d0d9f9b57b4..0000000000000000000000000000000000000000 --- a/scripts/lint-restyle-ceiling.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { assert, describe, it } from "@effect/vitest"; - -import { countRestyleFindings, evaluateCeiling } from "./lint-restyle-ceiling.ts"; - -describe("lint-restyle-ceiling", () => { - it("counts only no-restyle diagnostics", () => { - const count = countRestyleFindings({ - diagnostics: [ - { code: "shadcn(no-restyle)" }, - { code: "react(refs)" }, - { code: "shadcn(no-restyle)" }, - ], - }); - assert.strictEqual(count, 2); - }); - - it("fails above the ceiling and passes at or below it", () => { - assert.isFalse(evaluateCeiling(11, 10).ok); - assert.isTrue(evaluateCeiling(10, 10).ok); - const below = evaluateCeiling(7, 10); - assert.isTrue(below.ok); - assert.include(below.message, "Lower RESTYLE_CEILING"); - }); -}); diff --git a/scripts/lint-restyle-ceiling.ts b/scripts/lint-restyle-ceiling.ts deleted file mode 100644 index f1e7278db46e53876d3e132dfc54d632df438405..0000000000000000000000000000000000000000 --- a/scripts/lint-restyle-ceiling.ts +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/env node -// @effect-diagnostics nodeBuiltinImport:off - one-shot CI gate over a child process. -import * as NodeChildProcess from "node:child_process"; -import * as NodePath from "node:path"; -import * as NodeURL from "node:url"; - -// `shadcn/no-restyle` stays a warning while the existing className overrides on -// components/ui exports are migrated to variants (see vite.config.ts). This gate keeps the -// count from growing: CI fails when findings exceed the ceiling. Lower the ceiling when you -// migrate a file, and delete this script when the rule becomes an error. -export const RESTYLE_CEILING = 0; - -const RULE = "shadcn(no-restyle)"; - -const repoRoot = NodePath.resolve(NodePath.dirname(NodeURL.fileURLToPath(import.meta.url)), ".."); - -interface LintReport { - readonly diagnostics: ReadonlyArray<{ readonly code: string }>; -} - -export function countRestyleFindings(report: LintReport): number { - return report.diagnostics.filter((diagnostic) => diagnostic.code === RULE).length; -} - -export function evaluateCeiling( - count: number, - ceiling: number, -): { readonly ok: boolean; readonly message: string } { - if (count > ceiling) { - return { - ok: false, - message: - `${RULE}: ${count} findings exceed the ceiling of ${ceiling}. ` + - "Use a variant or size on the components/ui export instead of a className override " + - "(run `vp lint apps/web/src` for the list).", - }; - } - const slack = ceiling - count; - return { - ok: true, - message: - slack === 0 - ? `${RULE}: ${count} findings, at the ceiling.` - : `${RULE}: ${count} findings, ${slack} below the ceiling of ${ceiling}. Lower RESTYLE_CEILING in scripts/lint-restyle-ceiling.ts to ${count}.`, - }; -} - -function main() { - const result = NodeChildProcess.spawnSync("vp", ["lint", "--format", "json", "apps/web/src"], { - cwd: repoRoot, - encoding: "utf8", - maxBuffer: 256 * 1024 * 1024, - }); - if (result.error) throw result.error; - const report = JSON.parse(result.stdout) as LintReport; - const verdict = evaluateCeiling(countRestyleFindings(report), RESTYLE_CEILING); - process.stdout.write(`${verdict.message}\n`); - process.exitCode = verdict.ok ? 0 : 1; -} - -if ( - process.argv[1] !== undefined && - NodePath.resolve(process.argv[1]) === NodeURL.fileURLToPath(import.meta.url) -) { - main(); -} diff --git a/vite.config.ts b/vite.config.ts index 87fc1fae13bf101931cc2aad0f50a454d8f9b925..0c8dec8c9f9458b9b6532fd5b70312354d013658 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -200,13 +200,13 @@ export default defineConfig({ { // components/ui exports own their look. App code picks a variant or size instead // of restyling with className; layout classes (width, flex, margin, position) stay - // allowed because placement belongs to the parent. Warn-only until the existing - // overrides are migrated to variants; the ceiling below stops the count growing. + // allowed because placement belongs to the parent. components/ui is for generic + // primitives: a look that belongs to one feature stays in that feature's component. files: ["apps/web/src/**"], excludeFiles: ["apps/web/src/components/ui/**"], rules: { "shadcn/no-restyle": [ - "warn", + "error", { allow: ["layout"], contracts: [ -- 2.54.0