mirror of
https://github.com/supabase/supabase.git
synced 2026-10-16 22:59:11 +03:00
Adds a Claude skill encoding correct React Hook Form usage, so
AI-written form code follows best practices instead of copying the
anti-patterns common in older Studio code (prop-form
`form.watch()`/`formState` subscriptions, subscription-only watches,
unguarded `valueAsNumber`, `?? undefined` controlled values, defaults
computed from unloaded queries).
**Added:**
- `.claude/skills/react-hook-form/SKILL.md` — subscription model
(`useWatch`/`useFormState` with `control`), canonical zod + `FormField`
composition (layout deferred to `studio-ui-patterns`), `values:` option
for async data, null normalization for controlled inputs, number-input
handling, dirty-state and gating rules, plus a fix-what-you-touch policy
aligned with the `no-use-watch` lint ratchet
**Changed:**
- `.claude/CLAUDE.md` and `apps/studio/CLAUDE.md` — register the skill
in the skill lists/table
- `.coderabbit.yaml` — add the skill to the existing Studio
code-guidelines entry so CodeRabbit applies it when reviewing Studio
code
Benchmarked on three real form tasks (adding a live-updating field to
`ThroughputField`, a new sheet form with async + nullable data, a
review-changes step in `EditBucketModal`), each run with and without the
skill: 13/13 assertions with the skill vs 8/13 baseline. The baseline
shipped a genuine bug in one task — a `null` server default flowed into
a `''` its own schema rejected, making Save unreachable — which the
skill run avoided.
## To test
- Ask Claude Code to add a field to any Studio form and check it loads
the skill (it's in the studio CLAUDE.md skill table) and uses
`useWatch({ control, name })` rather than `form.watch`
- Skim `SKILL.md` for anything that contradicts current form conventions
— `apps/design-system` demos remain the layout source of truth
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Documentation**
* Added a new monorepo “react-hook-form” skill guide with recommended
patterns for safe form subscriptions, wiring, default values,
reset/submission flows, and common anti-patterns.
* Updated Studio skills/load guidance to expand and reorder the skills
matrix, including form logic and copywriting guidance.
* Updated required skill coverage so `react-hook-form` is included for
any form-related work.
* **Chores**
* Expanded automated review enforcement so Studio form code is checked
against the new “react-hook-form” skill guidance.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com>
87 lines
9.7 KiB
Markdown
87 lines
9.7 KiB
Markdown
# Supabase Studio
|
||
|
||
Next.js pages router + TanStack Start (mid-migration, see below), React 19. Dev server: `pnpm dev:studio` → http://localhost:8082.
|
||
|
||
## Skills — load before working
|
||
|
||
Load the skills matching the task; stack them when a task spans areas:
|
||
|
||
| Task | Additional skills |
|
||
| ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- |
|
||
| Query/mutation hooks, query keys (`data/**`) | `studio-queries` |
|
||
| UI: pages, forms, tables, charts, sheets, empty states | `studio-ui-patterns` |
|
||
| Form logic: react-hook-form fields, watch/formState, reset, number inputs | `react-hook-form` |
|
||
| Displaying API errors | `studio-error-handling` |
|
||
| Tests (deciding, writing, reviewing) | `studio-testing`, then `studio-mock-api-tests` (component/MSW) or `studio-e2e-tests` (Playwright) |
|
||
| PostHog event tracking | `telemetry-standards` |
|
||
| SQL against user databases | `safe-sql-execution` |
|
||
| Logs Explorer SQL, `data/logs` | `clickhouse-logs-queries` |
|
||
| Component API design, boolean-prop refactors | `vercel-composition-patterns` |
|
||
| User-facing copy | `copywriting` |
|
||
|
||
## TanStack Start migration
|
||
|
||
Studio is migrating from the Next.js pages router (`pages/**`) to TanStack Start (`routes/**`). Both runtimes ship side-by-side; the `STUDIO_FRAMEWORK` env var selects which one `pnpm dev`/`build` runs (default: `next`, resolved in `scripts/dispatch.js`). Full route map and strategy: `TANSTACK_MIGRATION.md`.
|
||
|
||
- **Never delete a page file.** Most `routes/**` files are thin wrappers re-exporting the default export of their `pages/**` counterpart, so the Next file is load-bearing for both runtimes until the final cleanup pass.
|
||
- Pure page-body edits propagate to the route automatically. Mirror a change by hand into the corresponding `routes/**` file only when it touches what the route duplicates: `getLayout`/layout wrapping, page titles or other `staticData` (incl. `skip*Layout` flags), `withAuth`, or redirect paths.
|
||
- A new page under `pages/**` needs a matching route under `routes/**` plus a checklist entry in `TANSTACK_MIGRATION.md`.
|
||
- New code uses native TanStack APIs — no `next/router` or `next/link`. The `compat/next/` shims exist only for legacy re-exported pages.
|
||
- `routeTree.gen.ts` is generated by the Vite plugin — never hand-edit.
|
||
|
||
## Orientation
|
||
|
||
- **Data layer** — all platform API calls go through `data/fetchers.ts` (`openapi-fetch`, typed by the generated `api-types` package) with `handleError`; never raw `fetch`. One folder per resource in `data/`, most with a `keys.ts` query-key factory.
|
||
- **State** — valtio for global state (`state/`), nuqs for URL state, react-hook-form + zod for forms.
|
||
- **Platform vs self-hosted** — `IS_PLATFORM` gates platform-only behavior; `withAuth` is a no-op when self-hosted.
|
||
- **Telemetry** — `useTrack()` from `lib/telemetry/track`; event types live in `packages/common/telemetry-constants.ts`.
|
||
- **Tests** — default to including relevant tests with any change: a couple of unit tests for extracted logic, component tests for UI behavior, E2E only when the scope demands it (`studio-testing` has the decision tree). Not every PR needs them, but "no tests" should be a considered choice, not the default. Tooling: vitest + MSW; component tests use `customRender` + `addAPIMock` from `tests/lib/`; unhandled network requests fail tests. Don't `vi.mock('@/data/...')`.
|
||
- **Shortcuts** — use the registry in `state/shortcuts/` and `components/ui/Shortcut*.tsx`; keep `G then …` chords for navigation; no one-off keyboard listeners.
|
||
- **Reuse first** — before writing a new hook or helper, search for an existing one (`hooks/`, `lib/`, `packages/common`, `packages/ui-patterns`). If you do need a new one, make it as reusable as possible: general naming, no page-specific coupling, placed where other callers can find it.
|
||
- Co-locate sub-components with their parent; avoid barrel re-export files.
|
||
|
||
## Code style
|
||
|
||
Older Studio code predates some of these conventions. For new or modified code, follow them rather than mirroring nearby legacy patterns:
|
||
|
||
- **Booleans** read as `is`/`has`/`can`/`should`. Derive them from existing state (`const isFormValid = name.length > 0 && email.includes('@')`) — mirroring a derivable value into `useState` synced by `useEffect` is a bug pattern. Give multi-condition logic a name (`const canShowAddButton = !isSchemaLocked && canUpdateColumns && …`) instead of inlining the chain in JSX.
|
||
- **Ternaries**: one is fine for a binary choice; never nest them. Anything bigger flattens — early returns in statement position, sibling `&&` blocks in JSX.
|
||
- **Fetch states** render with early returns at the top level, or a flat `&&` chain with mutually exclusive guards inline — never a nested ternary:
|
||
|
||
```tsx
|
||
// Top level: early return per state
|
||
if (isLoading) return <GenericSkeletonLoader />
|
||
if (isError) return <AlertError error={error} subject="Failed to retrieve data" />
|
||
if (isSuccess && data.length === 0) return <EmptyState />
|
||
return <DataDisplay data={data} />
|
||
|
||
// Inline: flat `&&` blocks, mutually exclusive guards
|
||
<div>
|
||
{isLoading && <ShimmeringLoader />}
|
||
{isError && <AlertError error={error} />}
|
||
{isSuccess && data.length === 0 && <EmptyState />}
|
||
{isSuccess && data.length > 0 && <DataDisplay data={data} />}
|
||
</div>
|
||
```
|
||
|
||
- **`useEffect` is for synchronizing with external systems** (subscriptions, DOM, timers) — not for deriving data (compute it in render), reacting to user actions (do it in the handler), or fetching (React Query). Older code uses effects for all of these; don't copy it.
|
||
- **State** stays as local as possible — lift it only when it's actually shared. Related form fields belong in a single react-hook-form + zod form, not parallel `useState` calls.
|
||
- **Component size**: split at ~200–300 lines — or sooner when a component grows multiple distinct UI sections, tangled conditional rendering, or clusters of unrelated `useState`. Extract repeated JSX into small components, non-trivial pure logic into `.utils.ts` functions (which get unit tests), and reusable stateful logic into custom hooks.
|
||
- **Memoization is not the default**: `useMemo`/`useCallback` only for measured expense or referential stability a memoized child depends on.
|
||
- **TypeScript**: avoid `as` casts — where external data enters, parse it with zod (`schema.parse`/`safeParse`) instead. Model multi-state values as discriminated unions (`{ status: 'success'; data: T } | { status: 'error'; error: Error }`) rather than independent boolean flags.
|
||
- **Naming**: prop callbacks are `onX`, internal handlers are `handleX`. Custom hooks return objects, not tuples.
|
||
- **Refactoring**: when you move or extract code into a new module, update every importer to point at the new location directly — do **not** leave a re-export shim in the old file "for backward compatibility." It's a one-line import change per consumer, and keeping shims around makes the codebase messy and the true source of a symbol ambiguous.
|
||
|
||
## Defaults that differ here
|
||
|
||
- **ESLint warnings are ratcheted in CI**: the per-rule occurrence count must not increase, so a new `any`, unresolved `exhaustive-deps` warning, or default export fails the build even though it's "only a warning". Check locally with `pnpm --filter studio run lint:ratchet`.
|
||
- **Clipboard**: `copyToClipboard` from `'ui'`, and never `await` anything before calling it (Safari requires the write inside the user gesture; lint-enforced) — pass a Promise as the argument instead.
|
||
- **`useParams()` comes from `'common'`**, not `next/navigation` — it camelCases keys and returns `string | undefined`.
|
||
- **Permissions**: `useAsyncCheckPermissions` from `hooks/misc/useCheckPermissions` (returns `can: true` when self-hosted).
|
||
- **Gating**: `useIsFeatureEnabled` for product features, `useFlag` from `'common'` for feature flags — two different systems.
|
||
- **Dates**: `dayjs` (plugins pre-loaded at both entries, `pages/_app.tsx` and `routes/__root.tsx`), not `date-fns`. **Toasts**: `toast` from `'sonner'`.
|
||
- **Import split**: `'ui'` = primitives, `'ui-patterns'` = composed patterns (`ConfirmationModal`, …), `@ui/*` = alias into `packages/ui/src`. Icons come from `lucide-react`.
|
||
- **New tables** use `@tanstack/react-table`; `react-data-grid` is banned for new code.
|
||
- **Ad-hoc SQL** against the user's database goes through `executeSql` / `useExecuteSqlMutation` (`data/sql/execute-sql-mutation`).
|
||
- **Confirmations**: `ConfirmationModal` / `TextConfirmModal` from `ui-patterns`, never `window.confirm`. Disabled buttons needing an explanation use `ButtonTooltip`; inline warnings use `Admonition`.
|