test(app-shell): ratchet the component-props-to-any cast that hid #3025's dead prop, and pin the schema.direction boundary - #3036
Merged
Conversation
…3025's dead prop, and pin the schema.direction boundary Two follow-ups to #3025, both about stopping that bug from recurring. **The ratchet.** #3025 removed two `const PanelGroup = ResizablePanelGroup as React.FC<any>` 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/<pkg>/src` and `apps/<app>/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<string, any>`, `as ComponentProps<typeof Button>`, `const C: React.FC<Props>`). **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 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-ups to #3025, both about stopping that bug from recurring rather than fixing anything new.
1. The ratchet
#3025 removed two
const PanelGroup = ResizablePanelGroup as React.FC<any>casts. Those casts weren't incidental to the bug — they caused it. Erasing a component's props toanymeant that when react-resizable-panels v4 renameddirection→orientation, the stale prop type-checked against nothing and survived an entire major version as a dead prop React forwarded to the DOM.What makes this worth a gate rather than review vigilance is how both casts were justified:
Neither was true — v4's
GroupPropshas nodirectionkey at all. The cast reads as a workaround for a toolchain quirk while actually disabling the check that catches a breaking upstream rename. That's precisely the kind of thing that gets re-added in good faith, by someone reasoning exactly as those two comments did.Repo-wide over
packages/<pkg>/srcandapps/<app>/src, following the existing*.ratchet.test.tsshape (#2269, ADR-0054) so it runs in the gatingpnpm testjob. CoversFC/FunctionComponent/ComponentTypeparameterised withany,React.-qualified or not.I chose a ratchet test over an ESLint rule deliberately: it matches existing house precedent, runs in a job that already gates, and needs no per-package ESLint config changes while the #2713 lint-gate sweep is still mid-flight.
Current count is zero — which is exactly why now is the moment to set the gate.
It's tested against its own failure modes
A ratchet has two silent ways to become worthless: a scan path that finds nothing, and a regex that quietly stops matching. Both are pinned. The regex test asserts it fires on the exact casts #3025 removed plus spelling variants, and that it does not fire on legitimate neighbours:
Proof it bites
Reintroducing the exact cast into
custom/navigation-overlay.tsx:Reverted after testing.
2. The
schema.directionboundary commentrenderers/complex/resizable.tsx:24mapsschema.directiononto the library'sorientation. It looks exactly like a site #3025 missed — same prop name, same file family. It isn't:directionis ObjectUI's own public authoring vocabulary and appears in stored view metadata, so renaming it to match the library would break every savedresizableschema.Documented in place, because the next person to grep for
directionwill land there and reach for the wrong fix.Verification
actions-envelope,flow-envelope,no-refresh-key-remount,adr0054): 8 tests passresizable-orientation+form-field-panes: 15 tests passpnpm --filter @object-ui/components type-check: cleaneslint: exit 0 (2 pre-existingno-explicit-anywarnings incomplex/resizable.tsxon lines I didn't touch —[key: string]: anyin the props type andpanel: anyin the map; a props index signature is a different thing from a component cast, so the ratchet correctly ignores them)No changeset — a test and a comment.
🤖 Generated with Claude Code