diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 00000000000..51d7dd419ed --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,38 @@ +# Supabase Monorepo + +pnpm 10 + Turborepo monorepo. Requires Node >= 22. + +## Structure + +| Directory | Purpose | +| ----------------- | ------------------------------------------------------------ | +| `apps/studio` | Supabase Studio/Dashboard — Next.js (pages router), React 18 | +| `apps/docs` | Documentation site | +| `apps/www` | Marketing website | +| `packages/ui` | Shared UI components (shadcn/ui based) | +| `packages/common` | Shared utilities and telemetry constants | +| `e2e/studio` | Playwright E2E tests for Studio | + +## Common Commands + +```bash +pnpm install # install dependencies +pnpm dev:studio # run Studio dev server +pnpm test:studio # run Studio unit tests (vitest) +pnpm --prefix e2e/studio run e2e # run Studio E2E tests (playwright) +pnpm build --filter=studio # build Studio +pnpm lint --filter=studio # lint Studio +pnpm typecheck # typecheck all packages +``` + +## Conventions + +**UI** — import from `'ui'`, use `_Shadcn_` suffixed variants for form primitives. Check `packages/ui/index.tsx` before creating new primitives. + +**Styling** — Tailwind only, semantic tokens (`bg-muted`, `text-foreground-light`), no hardcoded colors. + +## Studio + +Pages router. Co-locate sub-components with parent. Avoid barrel re-export files. + +See studio-\* skills for detailed studio conventions. diff --git a/.claude/skills/studio-best-practices/SKILL.md b/.claude/skills/studio-best-practices/SKILL.md new file mode 100644 index 00000000000..62eca4c6d1d --- /dev/null +++ b/.claude/skills/studio-best-practices/SKILL.md @@ -0,0 +1,175 @@ +--- +name: studio-best-practices +description: React and TypeScript best practices for Supabase Studio. Use when writing + or reviewing Studio components — covers boolean naming, component structure, loading/error + states, state management, custom hooks, event handlers, conditional rendering, + performance, and TypeScript conventions. +--- + +# Studio Best Practices + +Applies to `apps/studio/**/*.{ts,tsx}`. + +## Boolean Naming + +Use descriptive prefixes — derive from existing state rather than storing separately: + +- `is` — state/identity: `isLoading`, `isPaused`, `isNewRecord` +- `has` — possession: `hasPermission`, `hasData` +- `can` — capability: `canUpdateColumns`, `canDelete` +- `should` — conditional behavior: `shouldFetch`, `shouldRender` + +Extract complex conditions into named variables: + +```tsx +// ❌ inline multi-condition +{ + !isSchemaLocked && isTableLike(selectedTable) && canUpdateColumns && !isLoading && - ) -} - -// ✅ Good - extract to descriptive variables -const isTableEntity = isTableLike(selectedTable) -const canShowAddButton = !isSchemaLocked && isTableEntity && canUpdateColumns && !isLoading - -{ - canShowAddButton && -} -``` - -### Use consistent naming conventions for booleans - -- Use `is` prefix for state/identity: `isLoading`, `isPaused`, `isNewRecord`, `isError` -- Use `has` prefix for possession: `hasPermission`, `hasShownModal`, `hasData` -- Use `can` prefix for capability/permission: `canUpdateColumns`, `canDelete`, `canEdit` -- Use `should` prefix for conditional behavior: `shouldFetch`, `shouldRender`, `shouldValidate` - -```tsx -// ✅ Good examples from codebase -const isNewRecord = column === undefined -const isPaused = project?.status === PROJECT_STATUS.INACTIVE -const isMatureProject = dayjs(project?.inserted_at).isBefore(dayjs().subtract(10, 'day')) -const { can: canUpdateColumns } = useAsyncCheckPermissions( - PermissionAction.TENANT_SQL_ADMIN_WRITE, - 'columns' -) -``` - -### Derive boolean state instead of storing it - -When a boolean can be computed from existing state, derive it rather than storing it separately. - -```tsx -// ❌ Bad - storing derived state -const [isFormValid, setIsFormValid] = useState(false) - -useEffect(() => { - setIsFormValid(name.length > 0 && email.includes('@')) -}, [name, email]) - -// ✅ Good - derive from existing state -const isFormValid = name.length > 0 && email.includes('@') -``` - -## Component Structure - -### Break down large components - -Components should ideally be under 200-300 lines. If a component grows larger, consider splitting it. - -**Signs a component should be split:** - -- Multiple distinct UI sections -- Complex conditional rendering logic -- Multiple useState hooks for unrelated state -- Difficult to understand at a glance - -```tsx -// ❌ Bad - monolithic component with everything inline -const UserDashboard = () => { - // 50 lines of hooks and state - // 100 lines of handlers - // 300 lines of JSX with nested conditions -} - -// ✅ Good - split into focused sub-components -const UserDashboard = () => { - return ( -
- - - - -
- ) -} -``` - -### Co-locate related components - -Place sub-components in the same directory as the parent component. Avoid using barrel files (files that do nothing but re-export things from other files) for imports. - -``` -components/interfaces/Auth/Users/ -├── UserPanel.tsx -├── UserOverview.tsx -├── UserLogs.tsx -├── Users.constants.ts -└── index.ts -``` - -### Extract repeated JSX patterns - -If you find yourself copying similar JSX blocks, extract them into a component. - -```tsx -// ❌ Bad - repeated pattern - - Overview - - - Logs - - -// ✅ Good - extract to component -const PanelTab = ({ value, children }: { value: string; children: ReactNode }) => ( - - {children} - -) -``` - -## Loading and Error States - -### Use consistent loading/error/success pattern - -Follow a consistent pattern for handling async states: - -```tsx -const { data, error, isLoading, isError, isSuccess } = useQuery() - -// Handle loading state first -if (isLoading) { - return -} - -// Handle error state -if (isError) { - return -} - -// Handle empty state if needed -if (isSuccess && data.length === 0) { - return -} - -// Render success state -return -``` - -### Use early returns for guard clauses - -Prefer early returns over deeply nested conditionals: - -```tsx -// ❌ Bad - deeply nested -const Component = () => { - if (data) { - if (!isError) { - if (hasPermission) { - return - } - } - } - return null -} - -// ✅ Good - early returns -const Component = () => { - if (!data) return null - if (isError) return - if (!hasPermission) return - - return -} -``` - -## State Management - -### Keep state as local as possible - -Start with local state and lift up only when needed. - -```tsx -// ✅ Good - state lives where it's used -const SearchableList = () => { - const [filterString, setFilterString] = useState('') - - const filteredItems = items.filter((item) => item.name.includes(filterString)) - - return ( -
- setFilterString(e.target.value)} /> - -
- ) -} -``` - -### Group related state with objects or reducers - -When you have multiple related pieces of state, consider grouping them: - -```tsx -// ❌ Bad - multiple related useState calls -const [name, setName] = useState('') -const [email, setEmail] = useState('') -const [phone, setPhone] = useState('') - -// ✅ Good - grouped state for forms (use react-hook-form) -const form = useForm({ - defaultValues: { name: '', email: '', phone: '' }, -}) -``` - -## Custom Hooks - -### Extract complex logic into custom hooks - -When logic becomes reusable or complex, extract it: - -```tsx -// ✅ Good - extracted to custom hook -export function useAsyncCheckPermissions(action: string, resource: string) { - const { permissions, isLoading, isSuccess } = useGetProjectPermissions() - - const can = useMemo(() => { - if (!IS_PLATFORM) return true - if (!isSuccess || !permissions) return false - return doPermissionsCheck(permissions, action, resource) - }, [isSuccess, permissions, action, resource]) - - return { isLoading, isSuccess, can } -} - -// Usage -const { can: canUpdateColumns } = useAsyncCheckPermissions( - PermissionAction.TENANT_SQL_ADMIN_WRITE, - 'columns' -) -``` - -### Return objects from hooks for better extensibility - -```tsx -// ❌ Bad - returning array (hard to extend) -const useToggle = () => { - const [value, setValue] = useState(false) - return [value, () => setValue((v) => !v)] -} - -// ✅ Good - returning object (easy to extend) -const useToggle = (initial = false) => { - const [value, setValue] = useState(initial) - return { - value, - toggle: () => setValue((v) => !v), - setTrue: () => setValue(true), - setFalse: () => setValue(false), - } -} -``` - -## Event Handlers - -### Name handlers consistently - -Use `on` prefix for prop callbacks and `handle` prefix for internal handlers: - -```tsx -interface Props { - onClose: () => void // Callback prop - onSave: (data: Data) => void -} - -const Component = ({ onClose, onSave }: Props) => { - const handleSubmit = () => { - // Internal handler - // process data - onSave(data) - } - - const handleCancel = () => { - // cleanup - onClose() - } -} -``` - -### Avoid inline arrow functions for expensive operations - -```tsx -// ❌ Bad - creates new function every render - handleItemClick(item)} /> - -// ✅ Good - stable reference with useCallback -const handleItemClick = useCallback( - (item: Item) => { - // handle click - }, - [dependencies] -) - - -``` - -## Conditional Rendering - -### Use appropriate patterns for different scenarios - -```tsx -// Simple show/hide - use && -{ - isVisible && -} - -// Binary choice - use ternary -{ - isLoading ? : -} - -// Multiple conditions - use early returns or extracted component -const StatusDisplay = ({ status }: { status: Status }) => { - if (status === 'loading') return - if (status === 'error') return - if (status === 'empty') return - return -} -``` - -### Avoid nested ternaries - -```tsx -// ❌ Bad - nested ternary -{ - isLoading ? : isError ? : -} - -// ✅ Good - separate conditions or early returns -if (isLoading) return -if (isError) return -return -``` - -## Performance - -### Use useMemo for expensive computations - -```tsx -// ✅ Good - memoize expensive filtering -const filteredItems = useMemo( - () => items.filter((item) => item.name.toLowerCase().includes(searchQuery.toLowerCase())), - [items, searchQuery] -) -``` - -### Avoid premature optimization - -Don't wrap everything in useMemo/useCallback. Only optimize when: - -- You have measured a performance problem -- The computation is genuinely expensive -- The value is passed to memoized children - -## TypeScript - -### Define prop interfaces explicitly - -```tsx -interface UserCardProps { - user: User - onEdit: (user: User) => void - onDelete: (userId: string) => void - isEditable?: boolean -} - -export const UserCard = ({ user, onEdit, onDelete, isEditable = true }: UserCardProps) => { - // ... -} -``` - -### Use discriminated unions for complex state - -```tsx -type AsyncState = - | { status: 'idle' } - | { status: 'loading' } - | { status: 'success'; data: T } - | { status: 'error'; error: Error } -``` - -### Avoid type casting, prefer validation with zod - -Never use type casting (e.g., `as any`, `as Type`). Instead, validate values at runtime using zod schemas. This ensures type safety and catches runtime errors. - -```tsx -// ❌ Bad - type casting bypasses type checking -const user = apiResponse as User -const data = unknownValue as string - -// ✅ Good - validate with zod schema -const userSchema = z.object({ - id: z.string(), - name: z.string(), - email: z.string().email(), -}) - -const user = userSchema.parse(apiResponse) -const data = z.string().parse(unknownValue) - -// ✅ Good - safe parsing with error handling -const result = userSchema.safeParse(apiResponse) -if (result.success) { - const user = result.data -} else { - // handle validation errors -} -``` diff --git a/.cursor/rules/studio/cards/RULE.md b/.cursor/rules/studio/cards/RULE.md deleted file mode 100644 index efcb36733b1..00000000000 --- a/.cursor/rules/studio/cards/RULE.md +++ /dev/null @@ -1,14 +0,0 @@ ---- -description: "Studio: Card usage for grouping related content and actions" -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio cards - -- Use cards to group related pieces of information. -- Use `CardContent` for sections and `CardFooter` for actions. -- Only use `CardHeader`/`CardTitle` when the card content is not already described by surrounding content (page title, section title, etc). -- Prefer headers/titles when multiple cards represent distinct groups (e.g. multiple settings groups). - diff --git a/.cursor/rules/studio/charts/RULE.md b/.cursor/rules/studio/charts/RULE.md deleted file mode 100644 index 558580db3e8..00000000000 --- a/.cursor/rules/studio/charts/RULE.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -description: "Studio: composable chart patterns built on Recharts and our chart presentational components" -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio charts - -Use the Design System UI pattern docs as the source of truth: - -- Documentation: `apps/design-system/content/docs/ui-patterns/charts.mdx` -- Demos: - - `apps/design-system/__registry__/default/block/chart-composed-demo.tsx` - - `apps/design-system/__registry__/default/block/chart-composed-basic.tsx` - - `apps/design-system/__registry__/default/block/chart-composed-states.tsx` - - `apps/design-system/__registry__/default/block/chart-composed-metrics.tsx` - - `apps/design-system/__registry__/default/block/chart-composed-actions.tsx` - - `apps/design-system/__registry__/default/block/chart-composed-table.tsx` - -## Best practices - -- Prefer provided chart building blocks over passing raw Recharts components to `ChartContent`. -- Use `useChart` context flags for consistent loading/disabled handling. -- Keep chart composition straightforward; avoid over-abstraction. - diff --git a/.cursor/rules/studio/component-system/RULE.md b/.cursor/rules/studio/component-system/RULE.md deleted file mode 100644 index b6433123d0d..00000000000 --- a/.cursor/rules/studio/component-system/RULE.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -description: 'Studio: UI component system (packages/ui + shadcn primitives)' -globs: - - apps/studio/**/*.{ts,tsx} - - packages/ui/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio component system - -Our primitive component system lives in `packages/ui` and is based on shadcn/ui patterns. - -- Prefer using components exported from `ui` (e.g. `import { Button } from 'ui'`). -- Prefer `_Shadcn_`-suffixed components for form components e.g. `Input_Shadcn_`. -- Avoid introducing new primitives unless explicitly requested. -- Browse available exports in `packages/ui/index.tsx` before composing new UI. diff --git a/.cursor/rules/studio/empty-states/RULE.md b/.cursor/rules/studio/empty-states/RULE.md deleted file mode 100644 index 266213fe9a9..00000000000 --- a/.cursor/rules/studio/empty-states/RULE.md +++ /dev/null @@ -1,25 +0,0 @@ ---- -description: 'Studio: empty state patterns (presentational vs informational vs zero-results vs missing route)' -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio empty states - -Use the Design System UI pattern docs as the source of truth: - -- Documentation: `apps/design-system/content/docs/ui-patterns/empty-states.mdx` -- Demos: - - `apps/design-system/registry/default/example/empty-state-presentational-icon.tsx` - - `apps/design-system/registry/default/example/empty-state-initial-state-informational.tsx` - - `apps/design-system/registry/default/example/empty-state-zero-items-table.tsx` - - `apps/design-system/registry/default/example/data-grid-empty-state.tsx` - - `apps/design-system/registry/default/example/empty-state-missing-route.tsx` - -## Quick guidance - -- Initial states: use presentational empty states when onboarding/value prop + a clear next action helps. -- Data-heavy lists: prefer informational empty states that match the list/table layout. -- Zero results: keep the UI consistent with the data state to avoid jarring transitions. -- Missing routes: prefer a centered `Admonition` pattern. diff --git a/.cursor/rules/studio/forms/RULE.md b/.cursor/rules/studio/forms/RULE.md deleted file mode 100644 index b9b729111b5..00000000000 --- a/.cursor/rules/studio/forms/RULE.md +++ /dev/null @@ -1,35 +0,0 @@ ---- -description: "Studio: form patterns (page layouts + side panels) and react-hook-form conventions" -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio forms - -Use the Design System UI pattern docs as the source of truth: - -- Documentation: `apps/design-system/content/docs/ui-patterns/forms.mdx` -- Demos: - - `apps/design-system/registry/default/example/form-patterns-pagelayout.tsx` - - `apps/design-system/registry/default/example/form-patterns-sidepanel.tsx` - -## Requirements - -- Build forms with `react-hook-form` + `zod`. -- Use `FormItemLayout` instead of manually composing `FormItem`/`FormLabel`/`FormMessage`/`FormDescription`. -- Wrap inputs with `FormControl_Shadcn_`. -- Use `_Shadcn_` imports from `ui` for form primitives where available. - -## Layout selection - -- Page layouts: `FormItemLayout layout="flex-row-reverse"` inside `Card` (`CardContent` per field; `CardFooter` for actions). -- Side panels (wide): `FormItemLayout layout="horizontal"` inside `SheetSection`. -- Side panels (narrow, `size="sm"` or below): `FormItemLayout layout="vertical"`. - -## Actions and state - -- Handle dirty state (`form.formState.isDirty`) to show Cancel and to disable Save. -- Show loading on submit buttons via `loading`. -- When submit button is outside the ``, set a stable `formId` and use the button’s `form` prop. - diff --git a/.cursor/rules/studio/layout/RULE.md b/.cursor/rules/studio/layout/RULE.md deleted file mode 100644 index b39f082b740..00000000000 --- a/.cursor/rules/studio/layout/RULE.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -description: 'Studio: page layout patterns (PageContainer/PageHeader/PageSection) and sizing guidance. Use to learn how to create or update existing pages in Studio.' -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio layout - -Use the Design System UI pattern docs as the source of truth: - -- Documentation: `apps/design-system/content/docs/ui-patterns/layout.mdx` -- Demos: - - `apps/design-system/registry/default/example/page-layout-settings.tsx` - - `apps/design-system/registry/default/example/page-layout-list.tsx` - - `apps/design-system/registry/default/example/page-layout-list-simple.tsx` - - `apps/design-system/registry/default/example/page-layout-detail.tsx` - -## Guidelines - -- Build pages using `PageContainer`, `PageHeader`, and `PageSection` for consistent spacing and max-widths. -- Choose `size` based on content: - - Settings/config: `size="default"` - - List/table-heavy: `size="large"` - - Full-screen experiences: `size="full"` -- For list pages: - - If filters/search exist, align table actions with filters (avoid `PageHeaderAside`/`PageSectionAside` for those actions). - - If no filters/search, actions can go in `PageHeaderAside` or `PageSectionAside` depending on context. diff --git a/.cursor/rules/studio/navigation/RULE.md b/.cursor/rules/studio/navigation/RULE.md deleted file mode 100644 index eb37afda3fb..00000000000 --- a/.cursor/rules/studio/navigation/RULE.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -description: "Studio: navigation patterns (page-level NavMenu + URL-driven navigation)" -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio navigation - -Use the Design System UI pattern docs as the source of truth: - -- Documentation: `apps/design-system/content/docs/ui-patterns/navigation.mdx` - -## NavMenu - -- Use `NavMenu` for a horizontal list of related views within a consistent page layout. -- Activating an item should trigger a URL change (no local-only tab state). -- See: `apps/design-system/content/docs/components/nav-menu.mdx` - diff --git a/.cursor/rules/studio/project-structure/RULE.md b/.cursor/rules/studio/project-structure/RULE.md deleted file mode 100644 index d87139dd5a3..00000000000 --- a/.cursor/rules/studio/project-structure/RULE.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -description: "Studio: project structure and where code lives" -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio project structure - -- Studio is a Next.js app using the pages router. -- Pages live in `apps/studio/pages`. - - Project pages: `apps/studio/pages/projects/[ref]` - - Org pages: `apps/studio/pages/org/[slug]` -- Studio components live in `apps/studio/components`. - - Studio UI helpers: `apps/studio/components/ui` - - Interface/page components: `apps/studio/components/interfaces` (e.g. `apps/studio/components/interfaces/Auth`) -- Shared hooks: `apps/studio/hooks` -- Shared helpers: `apps/studio/lib` - diff --git a/.cursor/rules/studio/queries/RULE.md b/.cursor/rules/studio/queries/RULE.md deleted file mode 100644 index a68bd2b8d91..00000000000 --- a/.cursor/rules/studio/queries/RULE.md +++ /dev/null @@ -1,185 +0,0 @@ ---- -description: 'Studio: data fetching conventions for queries/mutations (React Query hooks)' -globs: - - apps/studio/data/**/*.{ts,tsx} - - apps/studio/pages/**/*.{ts,tsx} - - apps/studio/components/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio queries & mutations (React Query) - -Follow the `apps/studio/data/` patterns: - -- Query options: `apps/studio/data/table-editor/table-editor-query.ts` -- Mutation hook: `apps/studio/data/edge-functions/edge-functions-update-mutation.ts` -- Keys: `apps/studio/data/edge-functions/keys.ts` -- Page usage: `apps/studio/pages/project/[ref]/database/tables/[id].tsx` - -## Organize query keys - -- Define a `keys.ts` per domain and export `*Keys` helpers (use array keys with `as const`). -- Do not inline query keys in components. - -Example: - -```ts -export const edgeFunctionsKeys = { - list: (projectRef: string | undefined) => ['projects', projectRef, 'edge-functions'] as const, - detail: (projectRef: string | undefined, slug: string | undefined) => - ['projects', projectRef, 'edge-function', slug, 'detail'] as const, -} -``` - -## Write query options (preferred pattern) - -Use `queryOptions` from `@tanstack/react-query` to define reusable query configurations. This pattern: - -- Provides type safety for query keys and data -- Can be used with `useQuery()` in components -- Can be used with `queryClient.fetchQuery()` for imperative fetching - -Guidelines: - -- Export `XVariables`, `XData`, and `XError` types from the file (prefixed with the domain name). -- Implement a private `getX(variables, signal?)` function that: - - throws if required variables are missing - - passes the `signal` through to the fetcher for cancellation - - calls `handleError(error)` on failure (which throws) — the function returns `data` on success - - this function should NOT be exported. For imperative fetching, use `queryClient.fetchQuery(xQueryOptions(...))` -- Export `xQueryOptions()` using `queryOptions` from `@tanstack/react-query`. -- Gate with `enabled` so the query doesn't run until required variables exist (and platform-only queries should include `IS_PLATFORM` from `lib/constants`). -- When migrating away from exporting `useQuery`, move all options into the `xQueryOptions` as default values. -- No extra options should be added as params, if the user wants to overwrite the options, they can do by destructuring the query options. For example, `{ ...xQueryOptions(vars), enabled: true }`. - -Template: - -```ts -import { queryOptions } from '@tanstack/react-query' - -import { xKeys } from './keys' -import { get, handleError } from '@/data/fetchers' -import { IS_PLATFORM } from '@/lib/constants' -import { ResponseError } from '@/types' - -export type XVariables = { projectRef?: string } -export type XError = ResponseError - -async function getX({ projectRef }: XVariables, signal?: AbortSignal) { - if (!projectRef) throw new Error('projectRef is required') - const { data, error } = await get('/v1/projects/{ref}/x', { - params: { path: { ref: projectRef } }, - signal, - }) - if (error) handleError(error) - return data -} - -export type XData = Awaited> - -export const xQueryOptions = ({ projectRef }: XVariables) => { - return queryOptions({ - queryKey: xKeys.list(projectRef), - queryFn: ({ signal }) => getX({ projectRef }, signal), - enabled: IS_PLATFORM && typeof projectRef !== 'undefined', - }) -} -``` - -## Using query options in components - -Use `useQuery` directly with the query options: - -```ts -import { useQuery } from '@tanstack/react-query' - -import { xQueryOptions } from '@/data/x/x-query' - -// In component: -const { data, isPending, isError } = useQuery( - xQueryOptions({ - projectRef: project?.ref, - connectionString: project?.connectionString, - }) -) -``` - -## Imperative fetching (outside React or in callbacks) - -Use `queryClient.fetchQuery()` with the query options: - -```ts -import { useQueryClient } from '@tanstack/react-query' - -import { xQueryOptions } from '@/data/x/x-query' - -// In component: -const queryClient = useQueryClient() - -const handleClick = useCallback( - async (id: number) => { - const data = await queryClient.fetchQuery( - xQueryOptions({ - id, - projectRef, - connectionString: project?.connectionString, - }) - ) - // use data... - }, - [project?.connectionString, projectRef, queryClient] -) -``` - -## Write a mutation hook - -- Export a `Variables` type that includes `projectRef`, identifiers (e.g. `slug`), and `payload`. -- Implement an `updateX(vars)` function that validates required variables and uses `handleError`. -- Prefer a `useXMutation()` wrapper that: - - accepts `UseCustomMutationOptions` (omit `mutationFn`) - - invalidates the relevant `list()` + `detail()` keys in `onSuccess` and `await`s them via `Promise.all` - - defaults to a `toast.error(...)` when `onError` isn't provided - -Template: - -```ts -import { useMutation, UseMutationOptions, useQueryClient } from '@tanstack/react-query' -import toast from 'react-hot-toast' - -import { xKeys } from './keys' -import type { UseCustomMutationOptions } from '@/data/custom-mutation' - -type XUpdateVariables = { projectRef: string; slug: string; payload: XPayload } - -export const useXUpdateMutation = ({ - onSuccess, - onError, - ...options -}: UseMutationOptions = {}) => { - const queryClient = useQueryClient() - return useMutation({ - mutationFn: updateX, - async onSuccess(data, variables, context) { - await Promise.all([ - queryClient.invalidateQueries({ - queryKey: xKeys.detail(variables.projectRef, variables.slug), - }), - queryClient.invalidateQueries({ queryKey: xKeys.list(variables.projectRef) }), - ]) - await onSuccess?.(data, variables, context) - }, - async onError(error, variables, context) { - if (onError === undefined) toast.error(`Failed to update: ${error.message}`) - else onError(error, variables, context) - }, - ...options, - }) -} -``` - -## Component usage - -- Prefer React Query's v5 flags: - - `isPending` for initial load (often aliased to `isLoading`) - - `isFetching` for background refetches -- Render states explicitly (pending → error → success), like `apps/studio/pages/project/[ref]/database/tables/[id].tsx`. diff --git a/.cursor/rules/studio/sheets/RULE.md b/.cursor/rules/studio/sheets/RULE.md deleted file mode 100644 index f55b3958ec5..00000000000 --- a/.cursor/rules/studio/sheets/RULE.md +++ /dev/null @@ -1,23 +0,0 @@ ---- -description: "Studio: side panels (Sheet) for context-preserving workflows" -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio sheets - -Use a `Sheet` when switching to a new page would be disruptive and the user should keep context (e.g. selecting an item from a list to edit details). - -## Structure - -- Prefer `SheetContent` with `size="lg"` for forms that need horizontal layout. -- Use `SheetHeader`, `SheetTitle`, `SheetSection`, and `SheetFooter` for consistent structure. -- Place submit/cancel actions in `SheetFooter`. - -## Forms in sheets - -- Prefer `FormItemLayout`: - - `layout="horizontal"` for wider sheets - - `layout="vertical"` for narrow sheets (`size="sm"` or below) -- See `@studio/forms` for the canonical patterns and demos. diff --git a/.cursor/rules/studio/styling/RULE.md b/.cursor/rules/studio/styling/RULE.md deleted file mode 100644 index f1679fbc264..00000000000 --- a/.cursor/rules/studio/styling/RULE.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -description: "Studio: styling rules (Tailwind + semantic tokens + typography/focus utilities)" -globs: - - apps/studio/**/*.{ts,tsx,scss} -alwaysApply: false ---- - -# Studio styling - -- Use Tailwind. -- Do not hardcode Tailwind color tokens; use our semantic classes: - - backgrounds: `bg`, `bg-muted`, `bg-warning`, `bg-destructive` - - text: `text-foreground`, `text-foreground-light`, `text-foreground-lighter`, `text-warning`, `text-destructive` -- Use existing typography utilities from `apps/studio/styles/typography.scss` instead of recreating styles. -- Use existing focus utilities from `apps/studio/styles/focus.scss` for consistent keyboard focus styling. - diff --git a/.cursor/rules/studio/tables/RULE.md b/.cursor/rules/studio/tables/RULE.md deleted file mode 100644 index c610fe84268..00000000000 --- a/.cursor/rules/studio/tables/RULE.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -description: "Studio: table patterns (Table vs Data Table vs Data Grid) and placement of actions/filters" -globs: - - apps/studio/**/*.{ts,tsx} -alwaysApply: false ---- - -# Studio tables - -Use the Design System UI pattern docs as the source of truth: - -- Documentation: `apps/design-system/content/docs/ui-patterns/tables.mdx` -- Demos: - - `apps/design-system/registry/default/example/table-demo.tsx` - - `apps/design-system/registry/default/example/data-table-demo.tsx` - - `apps/design-system/registry/default/example/data-grid-demo.tsx` - -## Choose the right pattern - -- `Table`: simple, static, semantic table display. -- Data Table: TanStack-powered pattern for sorting/filtering/pagination; composed per use case. -- Data Grid: only when you need virtualization, column resizing, or complex cell editing. - -## Actions and filters placement - -- Actions: above the table, aligned right. -- Search/filters: above the table, aligned left. -- If the table is the primary page content and has no filters/search, actions can live in the page’s primary/secondary actions area. - diff --git a/.cursor/rules/testing/e2e-studio/RULE.md b/.cursor/rules/testing/e2e-studio/RULE.md deleted file mode 100644 index 4d98bbfc35a..00000000000 --- a/.cursor/rules/testing/e2e-studio/RULE.md +++ /dev/null @@ -1,336 +0,0 @@ ---- -description: "Testing: Playwright E2E best practices for Studio tests (avoid flake + race conditions)" -globs: - - e2e/studio/**/*.ts - - e2e/studio/**/*.spec.ts -alwaysApply: false ---- - -# E2E Testing Best Practices - -## Getting Context - -Before writing or modifying tests, use the Playwright MCP to understand: - -- Available page elements and their roles/locators -- Current page state and network activity -- Existing test patterns in the codebase - -Avoid extensive code reading - let Playwright's inspection tools guide your understanding of the UI. - -## Avoiding Race Conditions - -### Set up API waiters BEFORE triggering actions - -The most common source of flaky tests is race conditions between UI actions and API calls. Always create response waiters before clicking buttons or navigating. - -```ts -// ❌ Bad - race condition: response might complete before waiter is set up -await page.getByRole('button', { name: 'Save' }).click() -await waitForApiResponse(page, 'pg-meta', ref, 'query?key=table-create') - -// ✅ Good - waiter is ready before action -const apiPromise = waitForApiResponse(page, 'pg-meta', ref, 'query?key=table-create') -await page.getByRole('button', { name: 'Save' }).click() -await apiPromise -``` - -### Use `createApiResponseWaiter` for pre-navigation waits - -When you need to wait for a response that happens during navigation: - -```ts -// ✅ Good - waiter created before navigation -const loadPromise = waitForTableToLoad(page, ref) -await page.goto(toUrl(`/project/${ref}/editor?schema=public`)) -await loadPromise -``` - -### Wait for multiple related API calls with Promise.all - -When an action triggers multiple API calls, wait for all of them: - -```ts -// ✅ Good - wait for all related API calls -const createTablePromise = waitForApiResponseWithTimeout(page, (response) => - response.url().includes('query?key=table-create') -) -const tablesPromise = waitForApiResponseWithTimeout(page, (response) => - response.url().includes('tables?include_columns=true') -) -const entitiesPromise = waitForApiResponseWithTimeout(page, (response) => - response.url().includes('query?key=entity-types-') -) - -await page.getByRole('button', { name: 'Save' }).click() -await Promise.all([createTablePromise, tablesPromise, entitiesPromise]) -``` - -## Waiting Strategies - -### Prefer Playwright's built-in auto-waiting - -Playwright automatically waits for elements to be actionable. Use this instead of manual timeouts: - -```ts -// ❌ Bad - arbitrary timeout -await page.waitForTimeout(2000) -await page.getByRole('button', { name: 'Submit' }).click() - -// ✅ Good - auto-waits for element to be visible and enabled -await page.getByRole('button', { name: 'Submit' }).click() -``` - -### Use `expect.poll` for dynamic assertions - -When waiting for state to change: - -```ts -// ✅ Good - polls until condition is met -await expect - .poll(async () => { - return await page.getByLabel(`View ${tableName}`).count() - }) - .toBe(0) -``` - -### Use `waitForSelector` with state for element lifecycle - -```ts -// ✅ Good - wait for panel to close -await page.waitForSelector('[data-testid="side-panel"]', { state: 'detached' }) -``` - -### Avoid `networkidle` - use specific API waits instead - -```ts -// ❌ Bad - unreliable and slow -await page.waitForLoadState('networkidle') - -// ✅ Good - wait for specific API response -await waitForApiResponse(page, 'pg-meta', ref, 'tables') -``` - -### Use timeouts sparingly and only for non-API waits - -```ts -// ✅ Acceptable - waiting for client-side debounce -await page.getByRole('textbox').fill('search term') -await page.waitForTimeout(300) // Allow debounce to complete -``` - -## Test Structure - -### Use the custom test utility - -Always import from the custom test utility for consistent fixtures: - -```ts -import { test } from '../utils/test.js' -``` - -### Use `withFileOnceSetup` for expensive setup - -When setup is expensive (cleanup, seeding), run it once per file: - -```ts -test.beforeAll(async ({ browser, ref }) => { - await withFileOnceSetup(import.meta.url, async () => { - const ctx = await browser.newContext() - const page = await ctx.newPage() - - // Expensive setup logic (e.g., cleanup old test data) - await deleteTestTables(page, ref) - }) -}) - -test.afterAll(async () => { - await releaseFileOnceCleanup(import.meta.url) -}) -``` - -### Dismiss toasts before interacting with UI - -Toasts can overlay buttons and block interactions: - -```ts -const dismissToastsIfAny = async (page: Page) => { - const closeButtons = page.getByRole('button', { name: 'Close toast' }) - const count = await closeButtons.count() - for (let i = 0; i < count; i++) { - await closeButtons.nth(i).click() - } -} - -// ✅ Good - dismiss toasts before clicking -await dismissToastsIfAny(page) -await page.getByRole('button', { name: 'New table' }).click() -``` - -## Assertions - -### Always include descriptive messages - -```ts -// ❌ Bad - no context on failure -await expect(page.getByRole('button', { name: 'Save' })).toBeVisible() - -// ✅ Good - clear message on failure -await expect( - page.getByRole('button', { name: 'Save' }), - 'Save button should be visible after form is filled' -).toBeVisible() -``` - -### Use appropriate timeouts for slow operations - -```ts -// ✅ Good - explicit timeout for slow operations -await expect( - page.getByText(`Table ${tableName} is good to go!`), - 'Success toast should be visible after table creation' -).toBeVisible({ timeout: 50000 }) -``` - -## Locators - -### Prefer role-based locators - -```ts -// ✅ Good - semantic and resilient -page.getByRole('button', { name: 'Save' }) -page.getByRole('textbox', { name: 'Username' }) -page.getByRole('menuitem', { name: 'Delete' }) - -// ❌ Avoid - brittle CSS selectors -page.locator('.btn-primary') -page.locator('#submit-button') -``` - -### Use test IDs for complex elements - -```ts -// ✅ Good - stable identifier for complex elements -page.getByTestId('table-editor-side-panel') -page.getByTestId('action-bar-save-row') -``` - -### Use `filter` for finding elements in context - -```ts -// ✅ Good - find button within specific row -const bucketRow = page.getByRole('row').filter({ hasText: bucketName }) -await bucketRow.getByRole('button').click() -``` - -## Helper Functions - -### Extract reusable operations into helpers - -Create helper functions for common operations: - -```ts -// e2e/studio/utils/storage-helpers.ts -export const createBucket = async ( - page: Page, - ref: string, - bucketName: string, - isPublic: boolean = false -) => { - await navigateToStorageFiles(page, ref) - - // Check if already exists - const bucketRow = page.getByRole('row').filter({ hasText: bucketName }) - if ((await bucketRow.count()) > 0) return - - await dismissToastsIfAny(page) - - // Create bucket with proper waits - const apiPromise = waitForApiResponse(page, 'storage', ref, 'bucket', { method: 'POST' }) - await page.getByRole('button', { name: 'New bucket' }).click() - await page.getByRole('textbox', { name: 'Bucket name' }).fill(bucketName) - await page.getByRole('button', { name: 'Create' }).click() - await apiPromise - - await expect( - page.getByRole('row').filter({ hasText: bucketName }), - `Bucket ${bucketName} should be visible` - ).toBeVisible() -} -``` - -### Use the existing wait utilities - -```ts -import { - createApiResponseWaiter, - waitForApiResponse, - waitForGridDataToLoad, - waitForTableToLoad, -} from '../utils/wait-for-response.js' -``` - -### Use the existing assertions utilities - -#### Clipboard assertions - -```ts -// ❌ Avoid - brittle hard coded timeout -await page.evaluate(() => navigator.clipboard.readText()) -await page.waitForTimeout(500) - -// ✅ Good - this utility function uses Playwright auto-retries mechanisms -await expectClipboardValue({ - page, - value: 'expectedValue' -}) -``` - -## API Mocking - -### Mock APIs for isolated testing - -```ts -// ✅ Good - mock API response -await page.route('*/**/logs.all*', async (route) => { - await route.fulfill({ body: JSON.stringify(mockAPILogs) }) -}) -``` - -### Use soft waits for optional API calls - -```ts -// ✅ Good - don't fail if API doesn't respond -await waitForApiResponse(page, 'pg-meta', ref, 'optional-endpoint', { - soft: true, - fallbackWaitMs: 1000, -}) -``` - -## Cleanup - -### Clean up test data in beforeAll/beforeEach - -```ts -test.beforeEach(async ({ page, ref }) => { - await deleteAllBuckets(page, ref) -}) -``` - -### Handle existing state gracefully - -```ts -// ✅ Good - check before trying to delete -const bucketRow = page.getByRole('row').filter({ hasText: bucketName }) -if ((await bucketRow.count()) === 0) return -// proceed with deletion -``` - -### Reset local storage when needed - -```ts -import { resetLocalStorage } from '../utils/reset-local-storage.js' - -// Clean up after tests that modify local storage -await resetLocalStorage(page, ref) -``` diff --git a/.cursor/rules/testing/unit-integration/RULE.md b/.cursor/rules/testing/unit-integration/RULE.md deleted file mode 100644 index 7248ee737a4..00000000000 --- a/.cursor/rules/testing/unit-integration/RULE.md +++ /dev/null @@ -1,10 +0,0 @@ ---- -description: "Testing: unit/integration conventions for Studio test files" -globs: - - apps/studio/**/*.test.ts - - apps/studio/**/*.test.tsx -alwaysApply: false ---- - -Follow the guidelines in `apps/studio/tests/README.md` when writing tests for Studio. - diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index fbf5c442531..75e6f46f14c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,5 +1,41 @@ # Copilot Code Review Instructions +## Review Policy — Read This First + +You are a code reviewer for a large TypeScript/Next.js/React monorepo. Your reviews must be **low-noise and high-signal**. The team acts on fewer than 20% of default Copilot suggestions, so every comment you leave must earn its place. + +### Confidence Threshold + +Only comment when you are **>85% confident** the issue is a real bug, security vulnerability, or logic error. If you are unsure, do not comment. Silence is better than noise. + +### What NOT to Comment On + +Our CI pipeline already validates the following. **Never comment on these topics:** + +- **Formatting or whitespace** — Prettier runs on every PR +- **Linting issues** — ESLint with auto-fix runs on every PR +- **Type errors** — TypeScript strict-mode typecheck runs on every PR +- **Typos or spelling** — Automated typo detection runs on every PR +- **Missing tests for trivial changes** — Handled by topic-specific test instructions +- **Import ordering or grouping** — Handled by linter +- **Naming style preferences** (camelCase vs snake_case debates) — Follow existing file conventions +- **Accessibility attributes on shadcn/Radix UI components** — See `studio-shadcn-components.instructions.md` for details + +### What TO Comment On (Priority Order) + +1. **Logic errors and bugs** — Off-by-one, null derefs, wrong conditional, unreachable code, incorrect early returns +2. **Security vulnerabilities** — XSS, SQL injection, auth bypass, secrets in code, unsafe `dangerouslySetInnerHTML` +3. **Race conditions and async bugs** — Missing `await`, unhandled promise rejections, stale closures, effect cleanup issues +4. **Data loss risks** — Destructive operations without confirmation, missing error handling on writes +5. **API contract violations** — Wrong HTTP method, missing auth headers, incorrect request/response shapes + +### Comment Style + +- **Be advisory, not prescriptive.** Use "Consider..." or "This may..." — never demand changes. +- **One comment per distinct issue.** Do not leave multiple comments about the same underlying problem. +- **No self-contradictions.** If you suggest a change, do not then flag a problem with your own suggestion. +- **Do not comment on individual commits.** Review the final state of the PR diff only. + ## Repo Context This is a TypeScript/Next.js/React monorepo: @@ -11,16 +47,13 @@ This is a TypeScript/Next.js/React monorepo: ## Topic-Specific Guidelines -Detailed review rules are in path-specific instruction files under `.github/instructions/`: +Path-specific rules in `.github/instructions/`: - **Telemetry**: `studio-telemetry.instructions.md` — event naming, property conventions, feature flag measurement - **Testing**: `studio-testing.instructions.md` — test strategy, extraction patterns, coverage expectations +- **Error Handling**: `studio-error-handling.instructions.md` — error classification, `ErrorMatcher` usage +- **E2E Tests**: `studio-e2e-tests.instructions.md` — selector priority, anti-patterns (`waitForTimeout`, `force: true`) +- **Composition Patterns**: `studio-composition-patterns.instructions.md` — avoid boolean props, use compound components +- **shadcn/Radix Components**: `studio-shadcn-components.instructions.md` — accessibility handled by primitives, do not flag -These files are scoped to `apps/studio/` and applied automatically by Copilot during reviews. - -## References - -For the full, authoritative versions of these standards: - -- Telemetry: `.claude/skills/telemetry-standards/SKILL.md` -- Testing: `.claude/skills/studio-testing/SKILL.md` +These files are scoped to `apps/studio/` and applied automatically during reviews. diff --git a/.github/instructions/studio-composition-patterns.instructions.md b/.github/instructions/studio-composition-patterns.instructions.md new file mode 100644 index 00000000000..ff62a03db49 --- /dev/null +++ b/.github/instructions/studio-composition-patterns.instructions.md @@ -0,0 +1,93 @@ +--- +applyTo: "apps/studio/**" +--- + +# React Composition Patterns Review Rules + +All comments are **advisory**. + +## Core Principle + +Avoid boolean prop proliferation. Use composition (compound components, explicit variants, children) instead of boolean flags to customize behavior. + +## When to Flag + +### 1. Boolean Prop Proliferation (HIGH) + +Flag components accumulating boolean props like `isThread`, `isEditing`, `showAttachments`. Each boolean doubles the state space. + +```tsx +// BAD — unclear intent, combinatorial explosion + + +// GOOD — self-documenting variants + + +``` + +### 2. Render Props Instead of Children (MEDIUM) + +Flag `renderX` callback props when `children` composition would work. + +```tsx +// BAD — render prop for structure + } /> + +// GOOD — compound component + + + + +``` + +### 3. UI Coupled to State Implementation (MEDIUM) + +Flag UI components calling specific state hooks like `useGlobalChannelState()` directly. The provider should own the state implementation; UI should only use a generic context interface. + +```tsx +// BAD — UI knows HOW state is managed +const state = useGlobalChannelState(channelId) + +// GOOD — provider owns implementation, UI uses context + + {/* reads from context */} + +``` + +### 4. State Trapped in Child Components (MEDIUM) + +Flag state that siblings or dialogs need but can't access without prop drilling or refs. Lift it into a provider. + +```tsx +// BAD — sibling can't access state +function ForwardComposer() { + const [state, setState] = useState(init) +} +// ForwardButton is a sibling and can't reach state + +// GOOD — provider at shared ancestor + + {/* can access state */} + {/* can also access state */} + +``` + +### 5. React 19 API Updates + +Flag `forwardRef` and `useContext` in new code — use `ref` as a regular prop and `use()` instead. + +```tsx +// BAD +const Input = forwardRef((props, ref) => ) +const value = useContext(MyContext) + +// GOOD +function Input({ ref, ...props }) { return } +const value = use(MyContext) +``` + +## Key Principle + +Lift state → Compose UI → Inject via generic context → No boolean prop proliferation. + +Canonical standard: `.claude/skills/vercel-composition-patterns/SKILL.md` diff --git a/.github/instructions/studio-e2e-tests.instructions.md b/.github/instructions/studio-e2e-tests.instructions.md new file mode 100644 index 00000000000..58488df8d18 --- /dev/null +++ b/.github/instructions/studio-e2e-tests.instructions.md @@ -0,0 +1,86 @@ +--- +applyTo: 'e2e/studio/**,apps/studio/**' +--- + +# Studio E2E Test Review Rules + +All comments are **advisory**. + +## Selector Priority (best to worst) + +1. **`getByRole` with accessible name** — most robust, tests accessibility + + ```typescript + page.getByRole('button', { name: 'Save' }) + ``` + +2. **`getByTestId`** — stable, explicit test hooks + + ```typescript + page.getByTestId('table-editor-side-panel') + ``` + +3. **`getByText` with exact match** — good for unique text + + ```typescript + page.getByText('Data API Access', { exact: true }) + ``` + +4. **`locator` with CSS** — use sparingly, more fragile + ```typescript + page.locator('[data-state="open"]') + ``` + +## Patterns to Flag + +- **XPath selectors** — fragile to DOM changes + + ```typescript + // BAD + locator('xpath=ancestor::div[contains(@class, "space-y")]') + ``` + +- **Parent traversal with `locator('..')`** — breaks when structure changes + + ```typescript + // BAD + element.locator('..').getByRole('button') + ``` + +- **`waitForTimeout`** — never use; wait for something specific instead + + ```typescript + // BAD + await page.waitForTimeout(1000) + + // GOOD — wait for UI element + await expect(page.getByText('Success')).toBeVisible() + + // GOOD — wait for API response + const apiPromise = waitForApiResponse(page, 'pg-meta', ref, 'query?key=table-create') + await saveButton.click() + await apiPromise + ``` + +- **`force: true` on clicks** — make elements visible first instead + + ```typescript + // BAD + await menuButton.click({ force: true }) + + // GOOD — hover to reveal, then click + await tableRow.hover() + await expect(menuButton).toBeVisible() + await menuButton.click() + ``` + +- **Broad `filter({ hasText })` on generic elements** — may match multiple elements; scope to specific containers instead + +## Good Practices to Encourage + +- Scope selectors to containers: `page.getByTestId('side-panel').getByRole('switch')` +- Add `aria-label` to icon-only buttons in source code for better test selectors +- Use `test.describe.configure({ mode: 'serial' })` for tests sharing database state +- Add messages to expects: `await expect(locator, 'why').toBeVisible({ timeout: 30000 })` + +Canonical standard: `.claude/skills/studio-e2e-tests/SKILL.md` diff --git a/.github/instructions/studio-error-handling.instructions.md b/.github/instructions/studio-error-handling.instructions.md new file mode 100644 index 00000000000..bb4144e46f4 --- /dev/null +++ b/.github/instructions/studio-error-handling.instructions.md @@ -0,0 +1,43 @@ +--- +applyTo: "apps/studio/**" +--- + +# Studio Error Handling Review Rules + +All comments are **advisory**. + +## Architecture + +Errors flow: `handleError()` → throws typed subclass → React Query catches → `ErrorMatcher` reads `errorType` → renders troubleshooting. The component does an O(1) lookup — it never does regex matching. + +## When to Flag + +- PR passes `error.message` instead of the full `error` object to `ErrorMatcher` — the class type is lost +- PR puts regex patterns in `error-mappings.tsx` — they belong in `data/error-patterns.ts` +- PR uses `Object.assign` to stamp `errorType` on an error — should throw a proper subclass instead +- PR passes a raw URL string for support links — should use `supportFormParams={{ projectRef }}` +- PR puts the page title inside the error mapping — it belongs on the `` caller +- PR adds callback props (`onDebugWithAI`, `onRestartProject`) to troubleshooting components — use hooks inside them instead + +## Correct Usage + +```tsx +{isError && ( + +)} +``` + +## Key Files + +| File | Purpose | +|------|---------| +| `data/error-patterns.ts` | `{ pattern, ErrorClass }` array — regex lives here | +| `types/api-errors.ts` | Error classes, `KnownErrorType` union | +| `ErrorMatcher.tsx` | Reads `errorType`, looks up mapping, renders | +| `error-mappings.tsx` | `Record` | + +Canonical standard: `.claude/skills/studio-error-handling/SKILL.md` diff --git a/.github/instructions/studio-shadcn-components.instructions.md b/.github/instructions/studio-shadcn-components.instructions.md new file mode 100644 index 00000000000..ee61129adb0 --- /dev/null +++ b/.github/instructions/studio-shadcn-components.instructions.md @@ -0,0 +1,53 @@ +--- +applyTo: "apps/studio/**" +--- + +# shadcn/Radix UI Component Review Rules + +All comments are **advisory**. + +## Core Principle + +This project uses **shadcn/ui** components built on **Radix UI** primitives (from `packages/ui/`). These components provide comprehensive accessibility out-of-the-box. **Do not flag missing accessibility attributes that are already handled by the underlying Radix primitives.** + +## Components with Built-In Accessibility — Do NOT Flag + +The following components (imported from `ui`) already handle ARIA roles, keyboard navigation, focus management, and screen reader support automatically via Radix UI primitives: + +| Component | What Radix Handles | +|-----------|-------------------| +| `Dialog`, `AlertDialog` | `role="dialog"`, `aria-modal`, focus trapping, ESC to close | +| `DropdownMenu`, `ContextMenu` | `role="menu"` / `role="menuitem"`, arrow key navigation | +| `Select` | `role="combobox"`, `aria-expanded`, keyboard selection | +| `Tabs` | `role="tablist"` / `role="tab"` / `role="tabpanel"`, `aria-selected`, arrow keys | +| `Checkbox` | `role="checkbox"`, `aria-checked`, Space to toggle | +| `RadioGroup` | `role="radio"`, `aria-checked`, arrow key navigation | +| `Switch` | `role="switch"`, `aria-checked`, keyboard toggle | +| `Tooltip` | Trigger/content association, show/hide timing | +| `Accordion`, `Collapsible` | `aria-expanded`, Enter/Space to toggle | +| `Popover`, `HoverCard` | Focus management, dismiss on ESC | +| `Slider` | `role="slider"`, `aria-valuemin/max/now`, arrow keys | +| `Toggle`, `ToggleGroup` | `aria-pressed`, keyboard support | +| `ScrollArea` | Accessible scrollbar replacement | +| `NavigationMenu` | `role="navigation"`, keyboard navigation | + +### Specifically, Never Flag These + +- Missing `role` on `Dialog`, `AlertDialog`, `DropdownMenu`, `Select`, `Tabs`, `RadioGroup`, or other Radix-based components — roles are set by the primitive +- Missing `aria-modal` on `Dialog` or `AlertDialog` — set automatically +- Missing `aria-expanded` on `Accordion`, `Collapsible`, `Select`, or `DropdownMenu` triggers — managed by Radix state +- Missing `aria-selected` on `Tabs` — managed by `TabsPrimitive` +- Missing `aria-checked` on `Checkbox`, `RadioGroup`, or `Switch` — managed by Radix state +- Missing keyboard event handlers (`onKeyDown`, `onKeyUp`) on interactive Radix components — keyboard support is built-in +- Missing focus management in `Dialog` or `AlertDialog` — focus trapping is automatic +- Missing `aria-label` on `DialogClose` or `AlertDialogCancel` — these render a visible `Close` + +## What TO Flag + +Only flag accessibility issues for: + +1. **Custom interactive elements** not using Radix primitives (e.g., a `
` that should be a `