From 1234cea90e6ac24b84e3e98c39608a5e5f884986 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:02:37 +0800 Subject: [PATCH] test(app-shell): ratchet the component-props-to-`any` cast that hid #3025's dead prop, and pin the schema.direction boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to #3025, both about stopping that bug from recurring. **The ratchet.** #3025 removed two `const PanelGroup = ResizablePanelGroup as React.FC` casts. Those casts were not incidental to the bug — they caused it. Erasing a component's props to `any` meant that when react-resizable-panels v4 renamed `direction` to `orientation`, the stale prop type-checked against nothing and survived a whole major version as a dead prop React forwarded to the DOM. Deleting the casts is what turned it back into a hard TS2322. What makes this worth a ratchet rather than review vigilance is how both casts were justified: one blamed vite-plugin-dts for "not resolving the direction prop type correctly", the other a prop that "does not always narrow cleanly in our TS config". Neither was true — v4's `GroupProps` simply has no `direction` key. The cast reads as a workaround for a toolchain quirk while actually disabling the check that catches a breaking upstream rename, so it is the kind of thing that gets re-added in good faith. Repo-wide over `packages//src` and `apps//src`, following the existing `*.ratchet.test.ts` shape (#2269, ADR-0054) so it runs in the gating `pnpm test` job. Covers `FC` / `FunctionComponent` / `ComponentType` parameterised with `any`, qualified or not. Current count is zero, which is why now is the moment to set the gate. Three tests, because a ratchet has two silent failure modes of its own: a scan path that finds nothing, and a regex that stops matching. Both are pinned — the second against the exact casts #3025 removed, plus the legitimate neighbours it must not fire on (`as Record`, `as ComponentProps`, `const C: React.FC`). **The boundary comment.** `renderers/complex/resizable.tsx` maps `schema.direction` onto the library's `orientation`. That looks exactly like a site #3025 missed. It is not: `direction` is ObjectUI's own public authoring vocabulary and appears in stored view metadata, so renaming it to match the library would break every saved `resizable` schema. Said so in place, since the next person to grep for `direction` will land there. Verified the ratchet actually bites: reintroducing the exact cast into `custom/navigation-overlay.tsx` fails it with the file and line, and the four sibling ratchets plus the 15 resizable tests stay green. Co-Authored-By: Claude Opus 5 --- .../src/no-component-any-cast.ratchet.test.ts | 133 ++++++++++++++++++ .../src/renderers/complex/resizable.tsx | 8 +- 2 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 packages/app-shell/src/no-component-any-cast.ratchet.test.ts diff --git a/packages/app-shell/src/no-component-any-cast.ratchet.test.ts b/packages/app-shell/src/no-component-any-cast.ratchet.test.ts new file mode 100644 index 000000000..0a9435eb8 --- /dev/null +++ b/packages/app-shell/src/no-component-any-cast.ratchet.test.ts @@ -0,0 +1,133 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#3025 ratchet (AGENTS.md Commandment #6 — "No `any`"). Same shape as + * the ADR-0054 and #2269 ratchets: runs in the gating `pnpm test` job and fails + * if the anti-pattern reappears. + * + * The bug this guards. Two call sites carried + * `const PanelGroup = ResizablePanelGroup as React.FC`. The cast erased + * the component's props to `any`, so when react-resizable-panels v4 renamed + * `PanelGroup`'s `direction` prop to `orientation`, the stale `direction` kept + * type-checking against nothing — for a whole major version. It silently became + * a dead prop that React forwarded to the DOM as a stray attribute. Removing + * the casts is what turned it back into a hard `TS2322`. + * + * Both comments blamed the toolchain (vite-plugin-dts "not resolving the prop + * type correctly"; a prop that "does not always narrow cleanly in our TS + * config"). Neither was true. That is the real hazard here: a cast like this + * reads as a workaround for a tooling quirk while actually disabling the check + * that would have caught a breaking upstream rename. + * + * If this fails: do not widen a component's props to `any` to make an error go + * away. Read the dependency's current prop types — the prop was probably + * renamed or removed. If a genuinely-invalid prop must be passed, type the + * escape hatch narrowly (`as ComponentProps & { extra?: T }`), never + * with a blanket `any`. + * + * SCOPE — repo-wide over `packages//src` and `apps//src`, production + * sources only. Test files are excluded, both to match the existing ratchets + * and because this file necessarily contains the pattern in its own regex. + */ + +import { describe, it, expect } from 'vitest'; +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +// packages/app-shell/src -> repo root +const repoRoot = path.resolve(here, '../../..'); + +/** + * Banned: casting a value to a component type parameterised with `any`. + * + * Covers the three spellings that erase props identically — `FC`, + * `FunctionComponent`, `ComponentType` — with or without the `React.` + * qualifier, and tolerant of whitespace inside the type arguments. + */ +const COMPONENT_ANY_CAST = + /\bas\s+(?:React\.)?(?:FC|FunctionComponent|ComponentType)\s*<\s*any\s*>/; + +function collectSourceFiles(): string[] { + const out: string[] = []; + const walk = (dir: string) => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const name = entry.name; + if (name === 'node_modules' || name === 'dist' || name.startsWith('.wt-') || name === '.next') { + continue; + } + const full = path.join(dir, name); + if (entry.isDirectory()) { + walk(full); + } else if (/\.tsx?$/.test(name) && !/\.(test|spec)\.tsx?$/.test(name)) { + out.push(full); + } + } + }; + for (const group of ['packages', 'apps']) { + const groupDir = path.join(repoRoot, group); + let pkgs: string[]; + try { + pkgs = readdirSync(groupDir); + } catch { + continue; + } + for (const pkg of pkgs) { + const srcDir = path.join(groupDir, pkg, 'src'); + try { + if (statSync(srcDir).isDirectory()) walk(srcDir); + } catch { + /* package has no src/ */ + } + } + } + return out; +} + +describe('objectui#3025 — no component-props-to-any cast ratchet', () => { + it('finds the repo sources (guards against a broken scan path)', () => { + // The repo has thousands of source files; if this collapses, the scan + // globs have gone stale and the ratchet would silently pass on nothing. + expect(collectSourceFiles().length).toBeGreaterThan(500); + }); + + it('detects the pattern it is meant to ban (guards against a dead regex)', () => { + // A ratchet whose regex silently stops matching is worse than no ratchet, + // so pin it against the exact casts #3025 removed, plus spelling variants. + for (const sample of [ + 'const PanelGroup = ResizablePanelGroup as React.FC;', + 'const X = Y as FC', + 'const X = Y as React.ComponentType< any >', + 'const X = Y as FunctionComponent', + ]) { + expect(COMPONENT_ANY_CAST.test(sample), sample).toBe(true); + } + // ...and must not fire on legitimate neighbours. + for (const ok of [ + 'const x = y as Record', + 'const x = y as ComponentProps', + 'const C: React.FC = (p) => null', + ]) { + expect(COMPONENT_ANY_CAST.test(ok), ok).toBe(false); + } + }); + + it('has zero component-props-to-any casts in production sources', () => { + const offenders: string[] = []; + for (const file of collectSourceFiles()) { + const src = readFileSync(file, 'utf8'); + for (const line of src.split('\n')) { + if (COMPONENT_ANY_CAST.test(line)) { + offenders.push(`${path.relative(repoRoot, file)} :: ${line.trim()}`); + } + } + } + // If this fails: you widened a component's props to `any`. That disables + // the check that catches renamed/removed props across a dependency major — + // exactly how #3025's dead `direction` prop survived. Read the current + // prop types instead, or type the escape hatch narrowly. Do not allowlist. + expect(offenders).toEqual([]); + }); +}); diff --git a/packages/components/src/renderers/complex/resizable.tsx b/packages/components/src/renderers/complex/resizable.tsx index ab16d8024..af2e9ca1b 100644 --- a/packages/components/src/renderers/complex/resizable.tsx +++ b/packages/components/src/renderers/complex/resizable.tsx @@ -20,7 +20,13 @@ ComponentRegistry.register('resizable', ({ schema, className, ...props }: { schema: ResizableSchema; className?: string; [key: string]: any }) => { const panels = Array.isArray(schema.panels) ? schema.panels : []; return ( -