From 69d2df2a691c41e7d2bfdab4d0538fb6021301c6 Mon Sep 17 00:00:00 2001 From: Ali Waseem Date: Thu, 19 Feb 2026 09:18:15 -0700 Subject: [PATCH 1/7] chore: added skills for testing + compisition of components (#43024) ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? - Added Vercel composition rules - Added custom logic for components --- .claude/skills/studio-testing/SKILL.md | 103 ++ .../rules/testing-component-tests-ui-only.md | 86 ++ .../rules/testing-e2e-shared-features.md | 98 ++ .../rules/testing-exhaustive-permutations.md | 84 ++ .../rules/testing-extract-logic.md | 94 ++ .../vercel-composition-patterns/AGENTS.md | 946 ++++++++++++++++++ .../vercel-composition-patterns/SKILL.md | 89 ++ .../rules/architecture-avoid-boolean-props.md | 100 ++ .../rules/architecture-compound-components.md | 112 +++ .../patterns-children-over-render-props.md | 87 ++ .../rules/patterns-explicit-variants.md | 100 ++ .../rules/react19-no-forwardref.md | 42 + .../rules/state-context-interface.md | 191 ++++ .../rules/state-decouple-implementation.md | 113 +++ .../rules/state-lift-state.md | 125 +++ 15 files changed, 2370 insertions(+) create mode 100644 .claude/skills/studio-testing/SKILL.md create mode 100644 .claude/skills/studio-testing/rules/testing-component-tests-ui-only.md create mode 100644 .claude/skills/studio-testing/rules/testing-e2e-shared-features.md create mode 100644 .claude/skills/studio-testing/rules/testing-exhaustive-permutations.md create mode 100644 .claude/skills/studio-testing/rules/testing-extract-logic.md create mode 100644 .claude/skills/vercel-composition-patterns/AGENTS.md create mode 100644 .claude/skills/vercel-composition-patterns/SKILL.md create mode 100644 .claude/skills/vercel-composition-patterns/rules/architecture-avoid-boolean-props.md create mode 100644 .claude/skills/vercel-composition-patterns/rules/architecture-compound-components.md create mode 100644 .claude/skills/vercel-composition-patterns/rules/patterns-children-over-render-props.md create mode 100644 .claude/skills/vercel-composition-patterns/rules/patterns-explicit-variants.md create mode 100644 .claude/skills/vercel-composition-patterns/rules/react19-no-forwardref.md create mode 100644 .claude/skills/vercel-composition-patterns/rules/state-context-interface.md create mode 100644 .claude/skills/vercel-composition-patterns/rules/state-decouple-implementation.md create mode 100644 .claude/skills/vercel-composition-patterns/rules/state-lift-state.md diff --git a/.claude/skills/studio-testing/SKILL.md b/.claude/skills/studio-testing/SKILL.md new file mode 100644 index 0000000000000..3e7d16b8f5a1d --- /dev/null +++ b/.claude/skills/studio-testing/SKILL.md @@ -0,0 +1,103 @@ +--- +name: studio-testing +description: Testing strategy for Supabase Studio. Use when writing tests, deciding what + type of test to write, extracting logic from components into testable utility + functions, or reviewing test coverage. Covers unit tests, component tests, + and E2E test selection criteria. +--- + +# Studio Testing Strategy + +How to write and structure tests for `apps/studio/`. The core principle: push +logic out of React components into pure utility functions, then test those +functions exhaustively. Only use component tests for complex UI interactions. +Use E2E tests for features shared between self-hosted and platform. + +## When to Apply + +Reference these guidelines when: + +- Writing new tests for Studio code +- Deciding which type of test to write (unit, component, E2E) +- Extracting logic from a component to make it testable +- Reviewing whether test coverage is sufficient +- Adding a new feature that needs tests + +## Rule Categories by Priority + +| Priority | Category | Impact | Prefix | +| -------- | ---------------- | -------- | ---------- | +| 1 | Logic Extraction | CRITICAL | `testing-` | +| 2 | Test Coverage | CRITICAL | `testing-` | +| 3 | Component Tests | HIGH | `testing-` | +| 4 | E2E Tests | HIGH | `testing-` | + +## Quick Reference + +### 1. Logic Extraction (CRITICAL) + +- `testing-extract-logic` - Remove logic from components into `.utils.ts` files + as pure functions: args in, return out + +### 2. Test Coverage (CRITICAL) + +- `testing-exhaustive-permutations` - Test every permutation of utility functions: + happy path, malformed input, empty values, edge cases + +### 3. Component Tests (HIGH) + +- `testing-component-tests-ui-only` - Only write component tests for complex UI + interaction logic, not business logic + +### 4. E2E Tests (HIGH) + +- `testing-e2e-shared-features` - Write E2E tests for features used in both + self-hosted and platform; cover clicks AND keyboard shortcuts + +## Decision Tree: Which Test Type? + +``` +Is the logic a pure transformation (parse, format, validate, compute)? + YES -> Extract to .utils.ts, write unit test with vitest + NO -> Does the feature involve complex UI interactions? + YES -> Is it used in both self-hosted and platform? + YES -> Write E2E test in e2e/studio/features/ + NO -> Write component test with customRender + NO -> Can you extract the logic to make it pure? + YES -> Do that, then unit test it + NO -> Write a component test +``` + +## How to Use + +Read individual rule files for detailed explanations and code examples: + +``` +rules/testing-extract-logic.md +rules/testing-exhaustive-permutations.md +``` + +Each rule file contains: + +- Brief explanation of why it matters +- Incorrect code example with explanation +- Correct code example with explanation +- Real codebase references + +## Full Compiled Document + +For the complete guide with all rules expanded: `AGENTS.md` + +## Codebase References + +| What | Where | +| ----------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | +| Util test examples | `tests/components/Grid/Grid.utils.test.ts`, `tests/components/Billing/TaxID.utils.test.ts`, `tests/components/Editor/SpreadsheetImport.utils.test.ts` | +| Component test examples | `tests/features/logs/LogsFilterPopover.test.tsx`, `tests/components/CopyButton.test.tsx` | +| E2E test example | `e2e/studio/features/filter-bar.spec.ts` | +| E2E helpers pattern | `e2e/studio/utils/filter-bar-helpers.ts` | +| Custom render | `tests/lib/custom-render.tsx` | +| MSW mock setup | `tests/lib/msw.ts` (`addAPIMock`) | +| Test README | `tests/README.md` | +| Vitest config | `vitest.config.ts` | +| Related skills | `e2e-studio-tests` (running E2E), `vitest` (API reference), `vercel-composition-patterns` (component architecture) | diff --git a/.claude/skills/studio-testing/rules/testing-component-tests-ui-only.md b/.claude/skills/studio-testing/rules/testing-component-tests-ui-only.md new file mode 100644 index 0000000000000..4ba7b3e6cb644 --- /dev/null +++ b/.claude/skills/studio-testing/rules/testing-component-tests-ui-only.md @@ -0,0 +1,86 @@ +--- +title: Component Tests Are for Complex UI Logic Only +impact: HIGH +impactDescription: prevents slow, brittle tests that should be unit tests +tags: testing, components, ui, react +--- + +## Component Tests Are for Complex UI Logic Only + +Only write component tests (`.test.tsx`) when there is complex UI interaction +logic that cannot be captured by testing utility functions alone. + +**Valid reasons for a component test:** + +- Conditional rendering based on user interaction sequences +- Popover/dropdown open/close behavior with keyboard and mouse +- Form state transitions across multiple steps +- Components that coordinate multiple async operations visually + +**Not a valid reason:** testing a calculation, transformation, parsing, or +validation that happens to live inside a component. Extract that logic into a +`.utils.ts` file and unit test it instead. + +**Incorrect (rendering a component just to test logic):** + +```tsx +test('formats the display value correctly', () => { + render() + expect(screen.getByText('$12.34')).toBeInTheDocument() +}) +``` + +This is really testing a formatting function. Extract it: + +```ts +// PriceDisplay.utils.ts +export function formatPrice(amount: number, currency: string): string { ... } + +// PriceDisplay.utils.test.ts +test('formats USD cents to dollars', () => { + expect(formatPrice(1234, 'USD')).toBe('$12.34') +}) +``` + +**Correct (component test for real UI interaction logic):** + +```tsx +// Testing popover open/close, filter application, keyboard dismiss +describe('LogsFilterPopover', () => { + test('opens popover and shows filter options', async () => { + customRender() + await userEvent.click(screen.getByRole('button')) + expect(screen.getByText('Apply')).toBeVisible() + }) + + test('applies selected filters on submit', async () => { + const onChange = vi.fn() + customRender() + // ... interact with UI ... + await userEvent.click(screen.getByText('Apply')) + expect(onChange).toHaveBeenCalledWith(expectedFilters) + }) + + test('closes on Escape key', async () => { + customRender() + await userEvent.click(screen.getByRole('button')) + await userEvent.keyboard('{Escape}') + expect(screen.queryByText('Apply')).not.toBeInTheDocument() + }) +}) +``` + +**Studio component test conventions:** + +```tsx +// Always use customRender, not raw render +import { fireEvent } from '@testing-library/react' +// Use userEvent for popovers, fireEvent for dropdowns +import userEvent from '@testing-library/user-event' +import { customRender } from 'tests/lib/custom-render' +// Use addAPIMock for API mocking in beforeEach +import { addAPIMock } from 'tests/lib/msw' +``` + +See `tests/README.md` for full conventions on custom render, MSW mocking, +and nuqs URL parameter testing. diff --git a/.claude/skills/studio-testing/rules/testing-e2e-shared-features.md b/.claude/skills/studio-testing/rules/testing-e2e-shared-features.md new file mode 100644 index 0000000000000..31945a4533f22 --- /dev/null +++ b/.claude/skills/studio-testing/rules/testing-e2e-shared-features.md @@ -0,0 +1,98 @@ +--- +title: E2E Tests for Self-Hosted and Platform Features +impact: HIGH +impactDescription: ensures critical shared features work across deployment targets +tags: testing, e2e, playwright, self-hosted, platform +--- + +## E2E Tests for Self-Hosted and Platform Features + +If a feature exists in both self-hosted and the Supabase platform, create an +E2E test to cover it. E2E tests live in `e2e/studio/features/*.spec.ts`. + +**What to cover in E2E tests:** + +- Mouse/click interactions AND keyboard shortcuts (Tab, Enter, Escape, Arrow keys) +- Full user flows end-to-end +- Both adding and removing/clearing state +- Setup and teardown (create resources in `try`, clean up in `finally`) + +**Incorrect (only tests mouse clicks):** + +```ts +test('can add a filter', async ({ page }) => { + await page.getByRole('button', { name: 'Add filter' }).click() + await page.getByRole('option', { name: 'id' }).click() + // ... only click-based interactions +}) +``` + +**Correct (covers clicks AND keyboard shortcuts):** + +```ts +test.describe('Basic Filter Operations', () => { + test('can add a filter by clicking', async ({ page }) => { + await addFilter(page, ref, 'id', 'equals', '1') + await expect(page.getByTestId('filter-condition')).toBeVisible() + }) +}) + +test.describe('Keyboard Navigation - Freeform Input', () => { + test('Enter selects column from suggestions', async ({ page }) => { + await getFilterBarInput(page).press('Enter') + await expect(page.getByTestId('operator-input')).toBeFocused() + }) + + test('Backspace on empty input highlights last condition', async ({ page }) => { + await addFilter(page, ref, 'id', 'equals', '1') + await getFilterBarInput(page).press('Backspace') + await expect(page.getByTestId('filter-condition')).toHaveAttribute('data-highlighted', 'true') + }) + + test('Escape clears highlight', async ({ page }) => { + // ... + await getFilterBarInput(page).press('Escape') + await expect(page.getByTestId('filter-condition')).toHaveAttribute('data-highlighted', 'false') + }) +}) +``` + +**E2E helper pattern:** Extract reusable interactions into helper files at +`e2e/studio/utils/*-helpers.ts`: + +```ts +// e2e/studio/utils/filter-bar-helpers.ts +export async function addFilter(page, ref, column, operator, value) { + await selectColumnFilter(page, column) + await selectOperator(page, column, operator) + // ... fill value, wait for API response +} + +export async function setupFilterBarPage(page, ref, editorUrl) { + await page.goto(editorUrl) + await enableFilterBar(page) + await page.reload() +} +``` + +This keeps spec files focused on assertions while helpers handle the +interaction mechanics. + +**Always use try/finally for resource cleanup:** + +```ts +test('filters the table', async ({ page, ref }) => { + const tableName = await createTable(page, ref) + try { + await setupFilterBarPage(page, ref, editorUrl) + await navigateToTable(page, ref, tableName) + await addFilter(page, ref, 'id', 'equals', '1') + // assertions... + } finally { + await dropTable(page, ref, tableName) + } +}) +``` + +For E2E execution details (running tests, selectors, debugging), use the +`e2e-studio-tests` skill. diff --git a/.claude/skills/studio-testing/rules/testing-exhaustive-permutations.md b/.claude/skills/studio-testing/rules/testing-exhaustive-permutations.md new file mode 100644 index 0000000000000..afa808d7ad7c6 --- /dev/null +++ b/.claude/skills/studio-testing/rules/testing-exhaustive-permutations.md @@ -0,0 +1,84 @@ +--- +title: Test Every Permutation of Utility Functions +impact: CRITICAL +impactDescription: catches edge cases and regressions in business logic +tags: testing, utils, coverage, permutations +--- + +## Test Every Permutation of Utility Functions + +Once logic is extracted into a pure function, test it exhaustively. Every code +path should have a test. Don't just test the happy path. + +**What to cover:** + +- Valid inputs (happy path for each branch) +- Invalid / malformed inputs +- Empty values, null values, missing fields +- Edge cases (timestamps with colons, special characters, boundary values) +- Security-sensitive inputs (XSS payloads, external URLs) where relevant + +**Incorrect (only tests the happy path):** + +```ts +describe('formatFilterURLParams', () => { + test('parses a filter', () => { + const result = formatFilterURLParams('id:gte:20') + expect(result).toStrictEqual({ column: 'id', operator: 'gte', value: '20' }) + }) +}) +``` + +**Correct (tests every permutation):** + +```ts +describe('formatFilterURLParams', () => { + test('parses valid filter', () => { + const result = formatFilterURLParams('id:gte:20') + expect(result).toStrictEqual({ column: 'id', operator: 'gte', value: '20' }) + }) + + test('handles timestamp with colons in value', () => { + const result = formatFilterURLParams('created:gte:2024-01-01T00:00:00') + expect(result).toStrictEqual({ + column: 'created', + operator: 'gte', + value: '2024-01-01T00:00:00', + }) + }) + + test('rejects malformed filter with missing parts', () => { + const result = formatFilterURLParams('id') + expect(result).toBeUndefined() + }) + + test('rejects unrecognized operator', () => { + const result = formatFilterURLParams('id:nope:20') + expect(result).toBeUndefined() + }) + + test('allows empty filter value', () => { + const result = formatFilterURLParams('name:eq:') + expect(result).toStrictEqual({ column: 'name', operator: 'eq', value: '' }) + }) +}) +``` + +**Another real example -- `inferColumnType` tests every data type:** + +```ts +describe('inferColumnType', () => { + test('defaults to text for empty data', () => { ... }) + test('defaults to text for missing column', () => { ... }) + test('defaults to text for null values', () => { ... }) + test('detects integer', () => { ... }) // "42" -> int8 + test('detects float', () => { ... }) // "161.72" -> float8 + test('detects boolean', () => { ... }) // "true"/"false" -> bool + test('detects boolean with nulls', () => { ... }) + test('detects JSON object', () => { ... }) // "{}" -> jsonb + test('detects timestamp', () => { ... }) // multiple formats -> timestamptz +}) +``` + +The goal: if someone changes the function, at least one test should break for +any behavioral change. diff --git a/.claude/skills/studio-testing/rules/testing-extract-logic.md b/.claude/skills/studio-testing/rules/testing-extract-logic.md new file mode 100644 index 0000000000000..6c7d087fb2b90 --- /dev/null +++ b/.claude/skills/studio-testing/rules/testing-extract-logic.md @@ -0,0 +1,94 @@ +--- +title: Extract Logic Into Utility Files +impact: CRITICAL +impactDescription: makes business logic trivially testable without rendering components +tags: testing, utils, extraction, pure-functions +--- + +## Extract Logic Into Utility Files + +Remove as much logic from components as possible. Put it in co-located +`.utils.ts` files as pure functions: arguments in, return value out. No React +hooks, no context, no side effects. + +**File naming convention:** + +- Utility file: `ComponentName.utils.ts` next to the component +- Test file: `tests/components/.../ComponentName.utils.test.ts` mirroring the source path +- Or under `tests/unit/` for non-component utilities + +**Incorrect (logic buried inside a component):** + +```tsx +// components/Billing/TaxIdForm.tsx +function TaxIdForm({ taxIdValue, taxIdName }: Props) { + const handleSubmit = () => { + // Logic buried in the component -- hard to test without rendering + const taxId = TAX_IDS.find((t) => t.name === taxIdName) + let sanitized = taxIdValue + if (taxId?.vatPrefix && !taxIdValue.startsWith(taxId.vatPrefix)) { + sanitized = taxId.vatPrefix + taxIdValue + } + submitToApi(sanitized) + } + + return
...
+} +``` + +**Correct (logic extracted to a utility file):** + +```ts +// components/Billing/TaxID.utils.ts +import { TAX_IDS } from './TaxID.constants' + +// Pure function: args in, return out +export function sanitizeTaxIdValue({ value, name }: { value: string; name: string }): string { + const taxId = TAX_IDS.find((t) => t.name === name) + if (taxId?.vatPrefix && !value.startsWith(taxId.vatPrefix)) { + return taxId.vatPrefix + value + } + return value +} +``` + +```tsx +// components/Billing/TaxIdForm.tsx +import { sanitizeTaxIdValue } from './TaxID.utils' + +function TaxIdForm({ taxIdValue, taxIdName }: Props) { + const handleSubmit = () => { + const sanitized = sanitizeTaxIdValue({ value: taxIdValue, name: taxIdName }) + submitToApi(sanitized) + } + return
...
+} +``` + +```ts +// tests/components/Billing/TaxID.utils.test.ts +import { sanitizeTaxIdValue } from 'components/.../TaxID.utils' + +describe('sanitizeTaxIdValue', () => { + test('prefixes unprefixed EU tax ID', () => { + expect(sanitizeTaxIdValue({ value: '12345678', name: 'AT VAT' })).toBe('ATU12345678') + }) + + test('passes through already-prefixed EU tax ID', () => { + expect(sanitizeTaxIdValue({ value: 'ATU12345678', name: 'AT VAT' })).toBe('ATU12345678') + }) + + test('passes through non-EU tax ID unchanged', () => { + expect(sanitizeTaxIdValue({ value: '12-3456789', name: 'US EIN' })).toBe('12-3456789') + }) +}) +``` + +The component becomes a thin shell that calls the utility. All business logic +is testable without rendering anything. + +**Real codebase examples:** + +- `components/grid/SupabaseGrid.utils.ts` -- URL param parsing, used by 15+ components +- `components/.../SpreadsheetImport/SpreadsheetImport.utils.tsx` -- CSV parsing, column type inference +- `components/.../BillingCustomerData/TaxID.utils.ts` -- tax ID sanitization and comparison diff --git a/.claude/skills/vercel-composition-patterns/AGENTS.md b/.claude/skills/vercel-composition-patterns/AGENTS.md new file mode 100644 index 0000000000000..558bf9aa1e36d --- /dev/null +++ b/.claude/skills/vercel-composition-patterns/AGENTS.md @@ -0,0 +1,946 @@ +# React Composition Patterns + +**Version 1.0.0** +Engineering +January 2026 + +> **Note:** +> This document is mainly for agents and LLMs to follow when maintaining, +> generating, or refactoring React codebases using composition. Humans +> may also find it useful, but guidance here is optimized for automation +> and consistency by AI-assisted workflows. + +--- + +## Abstract + +Composition patterns for building flexible, maintainable React components. Avoid boolean prop proliferation by using compound components, lifting state, and composing internals. These patterns make codebases easier for both humans and AI agents to work with as they scale. + +--- + +## Table of Contents + +1. [Component Architecture](#1-component-architecture) — **HIGH** + - 1.1 [Avoid Boolean Prop Proliferation](#11-avoid-boolean-prop-proliferation) + - 1.2 [Use Compound Components](#12-use-compound-components) +2. [State Management](#2-state-management) — **MEDIUM** + - 2.1 [Decouple State Management from UI](#21-decouple-state-management-from-ui) + - 2.2 [Define Generic Context Interfaces for Dependency Injection](#22-define-generic-context-interfaces-for-dependency-injection) + - 2.3 [Lift State into Provider Components](#23-lift-state-into-provider-components) +3. [Implementation Patterns](#3-implementation-patterns) — **MEDIUM** + - 3.1 [Create Explicit Component Variants](#31-create-explicit-component-variants) + - 3.2 [Prefer Composing Children Over Render Props](#32-prefer-composing-children-over-render-props) +4. [React 19 APIs](#4-react-19-apis) — **MEDIUM** + - 4.1 [React 19 API Changes](#41-react-19-api-changes) + +--- + +## 1. Component Architecture + +**Impact: HIGH** + +Fundamental patterns for structuring components to avoid prop +proliferation and enable flexible composition. + +### 1.1 Avoid Boolean Prop Proliferation + +**Impact: CRITICAL (prevents unmaintainable component variants)** + +Don't add boolean props like `isThread`, `isEditing`, `isDMThread` to customize + +component behavior. Each boolean doubles possible states and creates + +unmaintainable conditional logic. Use composition instead. + +**Incorrect: boolean props create exponential complexity** + +```tsx +function Composer({ + onSubmit, + isThread, + channelId, + isDMThread, + dmId, + isEditing, + isForwarding, +}: Props) { + return ( +
+
+ + {isDMThread ? ( + + ) : isThread ? ( + + ) : null} + {isEditing ? ( + + ) : isForwarding ? ( + + ) : ( + + )} +