From 62b71c35e95333566be767c169146baabcba6d36 Mon Sep 17 00:00:00 2001 From: Martin Schneppenheim <23424570+weeco@users.noreply.github.com> Date: Tue, 17 Feb 2026 16:56:00 +0000 Subject: [PATCH] frontend: reduce Claude context bloat and add Biome legacy-import warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trim AGENTS.md, split oversized SKILL.md files into focused entrypoints with reference files, and delete/merge redundant rules. - AGENTS.md: 86 → 47 lines (remove discoverable patterns, add verify cmd) - e2e-tester SKILL.md: 725 → 80 lines (split into 3 reference files) - form-refactorer SKILL.md: 252 → 77 lines - Delete 15 rules (basic JS, linter-enforced, merged overlaps) - Fix factual errors (wrong utility names, Next.js assumptions) - Remove inert `globs` from skill rules (not supported there) - Add .claude/rules/test-unit-vs-integration.md (paths auto-loading) - Add noRestrictedImports warnings to biome.jsonc for legacy packages --- .../.claude/rules/test-unit-vs-integration.md | 13 + frontend/.claude/skills/api-patterns/SKILL.md | 32 +- .../api-patterns/rules/use-connect-query.md | 1 + .../.claude/skills/code-standards/SKILL.md | 4 +- .../skills/code-standards/rules/no-legacy.md | 4 + .../skills/code-standards/rules/ts-no-any.md | 75 -- .../code-standards/rules/ts-use-unknown.md | 78 -- frontend/.claude/skills/e2e-tester/SKILL.md | 699 +----------------- .../e2e-tester/references/container-setup.md | 163 ++++ .../e2e-tester/references/failure-analysis.md | 156 ++++ .../e2e-tester/references/test-patterns.md | 153 ++++ .../.claude/skills/form-refactorer/SKILL.md | 222 +----- .../references/common-patterns.md | 66 +- .../references/migration-examples.md | 81 +- .../skills/react-best-practices/SKILL.md | 63 +- .../rules/async-parallel.md | 28 - .../rules/bundle-barrel-imports.md | 15 - .../rules/bundle-code-splitting.md | 78 ++ .../rules/bundle-conditional.md | 31 - .../rules/bundle-defer-third-party.md | 46 -- .../rules/bundle-dynamic-imports.md | 38 - .../rules/client-localstorage-schema.md | 71 -- .../rules/js-cache-function-results.md | 80 -- .../rules/js-cache-property-access.md | 28 - .../rules/js-cache-storage.md | 70 -- .../rules/js-caching-patterns.md | 75 ++ .../rules/js-combine-iterations.md | 32 - .../rules/js-early-exit.md | 50 -- .../rules/js-hoist-regexp.md | 45 -- .../rules/js-min-max-loop.md | 82 -- .../rules/js-set-map-lookups.md | 24 - .../rules/rerender-defer-reads.md | 14 +- .../rules/use-react-query-for-server.md | 1 + .../rules/use-zustand-persist.md | 7 +- .../ui-development/rules/use-ui-registry.md | 1 + frontend/AGENTS.md | 66 +- frontend/biome.jsonc | 12 + 37 files changed, 912 insertions(+), 1792 deletions(-) create mode 100644 frontend/.claude/rules/test-unit-vs-integration.md delete mode 100644 frontend/.claude/skills/code-standards/rules/ts-no-any.md delete mode 100644 frontend/.claude/skills/code-standards/rules/ts-use-unknown.md create mode 100644 frontend/.claude/skills/e2e-tester/references/container-setup.md create mode 100644 frontend/.claude/skills/e2e-tester/references/failure-analysis.md create mode 100644 frontend/.claude/skills/e2e-tester/references/test-patterns.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/async-parallel.md create mode 100644 frontend/.claude/skills/react-best-practices/rules/bundle-code-splitting.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/bundle-conditional.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/bundle-defer-third-party.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/bundle-dynamic-imports.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/client-localstorage-schema.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/js-cache-function-results.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/js-cache-property-access.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/js-cache-storage.md create mode 100644 frontend/.claude/skills/react-best-practices/rules/js-caching-patterns.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/js-combine-iterations.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/js-early-exit.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/js-hoist-regexp.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/js-min-max-loop.md delete mode 100644 frontend/.claude/skills/react-best-practices/rules/js-set-map-lookups.md diff --git a/frontend/.claude/rules/test-unit-vs-integration.md b/frontend/.claude/rules/test-unit-vs-integration.md new file mode 100644 index 0000000000..446a9aaddb --- /dev/null +++ b/frontend/.claude/rules/test-unit-vs-integration.md @@ -0,0 +1,13 @@ +--- +paths: + - "src/**/*.test.{ts,tsx}" +--- + +# Unit vs Integration Tests + +| Extension | Environment | Use For | +|-----------|-------------|---------| +| `.test.ts` | Node (no DOM) | Pure logic, utils, transforms | +| `.test.tsx` | JSDOM (browser) | Components, hooks, DOM interaction | + +Wrong extension = test silently passes or fails in CI. See [full rule](../skills/testing/rules/test-unit-vs-integration.md) for details. diff --git a/frontend/.claude/skills/api-patterns/SKILL.md b/frontend/.claude/skills/api-patterns/SKILL.md index 13444bdc13..bb0f6ec234 100644 --- a/frontend/.claude/skills/api-patterns/SKILL.md +++ b/frontend/.claude/skills/api-patterns/SKILL.md @@ -33,36 +33,6 @@ Make API calls with Connect Query and handle responses properly. Regenerate protos: `task proto:generate` (from repo root) -## Basic Patterns - -### Query - -```typescript -import { useQuery } from '@connectrpc/connect-query'; -import { getUser } from 'protogen/user-UserService_connectquery'; - -const { data, isLoading, error } = useQuery( - getUser, - { id: userId }, - { enabled: !!userId } -); -``` - -### Mutation - -```typescript -import { useMutation } from '@connectrpc/connect-query'; -import { createUser } from 'protogen/user-UserService_connectquery'; - -const mutation = useMutation(createUser, { - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['getUser'] }); - }, -}); - -mutation.mutate({ name, config }); -``` - ## Rules -See `rules/` directory for detailed guidance. +See `rules/` directory for detailed guidance on queries, mutations, cache invalidation, and error handling. diff --git a/frontend/.claude/skills/api-patterns/rules/use-connect-query.md b/frontend/.claude/skills/api-patterns/rules/use-connect-query.md index 88b16b76cd..625607767a 100644 --- a/frontend/.claude/skills/api-patterns/rules/use-connect-query.md +++ b/frontend/.claude/skills/api-patterns/rules/use-connect-query.md @@ -79,3 +79,4 @@ function ClusterList({ organizationId }: Props) { - [Connect Query Docs](https://connectrpc.com/docs/web/query/getting-started) - [TanStack Query Options](https://tanstack.com/query/latest/docs/framework/react/reference/useQuery) +- See also: [use-react-query-for-server](../../state-management/rules/use-react-query-for-server.md) — when to use React Query vs Zustand diff --git a/frontend/.claude/skills/code-standards/SKILL.md b/frontend/.claude/skills/code-standards/SKILL.md index 5e3a008c98..34a870368f 100644 --- a/frontend/.claude/skills/code-standards/SKILL.md +++ b/frontend/.claude/skills/code-standards/SKILL.md @@ -18,12 +18,12 @@ Write code that is accessible, performant, type-safe, and maintainable. | Action | Rule | |--------|------| -| Avoid any | `ts-no-any.md` | -| Handle unknowns | `ts-use-unknown.md` | | Write components | `react-functional-only.md` | | Async code | `async-await-promises.md` | | Avoid legacy libs | `no-legacy.md` | +`any` types and top-level regex are enforced by Biome (`noExplicitAny`, `useTopLevelRegex`). + ## Commands ```bash diff --git a/frontend/.claude/skills/code-standards/rules/no-legacy.md b/frontend/.claude/skills/code-standards/rules/no-legacy.md index 321a720ec9..08b4bc3164 100644 --- a/frontend/.claude/skills/code-standards/rules/no-legacy.md +++ b/frontend/.claude/skills/code-standards/rules/no-legacy.md @@ -101,3 +101,7 @@ When converting observables to React state, automatic behaviors must become expl - MobX stores: `src/state/` (do not add new files) - Chakra components: throughout codebase (migrate on touch) + +## See Also + +- [use-ui-registry](../../ui-development/rules/use-ui-registry.md) — Registry component usage and installation diff --git a/frontend/.claude/skills/code-standards/rules/ts-no-any.md b/frontend/.claude/skills/code-standards/rules/ts-no-any.md deleted file mode 100644 index 1a7a0e868a..0000000000 --- a/frontend/.claude/skills/code-standards/rules/ts-no-any.md +++ /dev/null @@ -1,75 +0,0 @@ ---- -title: No any Type -impact: CRITICAL -impactDescription: any bypasses type checking and hides bugs until runtime -tags: typescript, types, any, strict ---- - -# No any Type (CRITICAL) - -## Explanation - -Never use `any` type. It bypasses TypeScript's type checking and hides bugs until runtime. Use `unknown` for truly unknown types and narrow them with type guards. - -## Incorrect - -```typescript -// any allows anything -function process(data: any) { - data.foo.bar.baz(); // No error, crashes at runtime -} - -// Casting to any -const value = something as any; - -// any in generics -const items: Array = []; - -// Implicit any from missing types -function handle(event) { // Parameter 'event' implicitly has 'any' type - console.log(event.target.value); -} -``` - -## Correct - -```typescript -// Use unknown for truly unknown data -function process(data: unknown) { - if (typeof data === 'object' && data !== null && 'foo' in data) { - // Now TypeScript knows data has foo - } -} - -// Define proper types -interface EventData { - target: { value: string }; -} - -function handle(event: EventData) { - console.log(event.target.value); -} - -// Use generics for flexible but type-safe code -function identity(value: T): T { - return value; -} - -// Use type predicates for narrowing -function isString(value: unknown): value is string { - return typeof value === 'string'; -} -``` - -## When any Seems Necessary - -| Situation | Solution | -|-----------|----------| -| Third-party lib | Create type definitions or use `@types/*` | -| Complex object | Define interface incrementally | -| Generic callback | Use proper generic types | -| JSON parsing | Use `unknown` and validate | - -## Reference - -- [TypeScript unknown vs any](https://www.typescriptlang.org/docs/handbook/2/narrowing.html) diff --git a/frontend/.claude/skills/code-standards/rules/ts-use-unknown.md b/frontend/.claude/skills/code-standards/rules/ts-use-unknown.md deleted file mode 100644 index b1aad3bd74..0000000000 --- a/frontend/.claude/skills/code-standards/rules/ts-use-unknown.md +++ /dev/null @@ -1,78 +0,0 @@ ---- -title: Use unknown Instead of any -impact: HIGH -impactDescription: unknown forces type narrowing, catching errors at compile time -tags: typescript, unknown, type-narrowing, safety ---- - -# Use unknown Instead of any (HIGH) - -## Explanation - -When you truly don't know a type, use `unknown` instead of `any`. Unlike `any`, `unknown` requires you to narrow the type before using it, catching errors at compile time rather than runtime. - -## Incorrect - -```typescript -// any allows unsafe operations -function processResponse(data: any) { - return data.items.map(item => item.name); // No error, but might crash -} - -// Casting through any -const config = JSON.parse(str) as any; -console.log(config.setting); // No type safety -``` - -## Correct - -```typescript -// unknown requires narrowing -function processResponse(data: unknown) { - // Type guard - if (isResponseData(data)) { - return data.items.map(item => item.name); // Safe! - } - throw new Error('Invalid response format'); -} - -// Type predicate for narrowing -interface ResponseData { - items: Array<{ name: string }>; -} - -function isResponseData(data: unknown): data is ResponseData { - return ( - typeof data === 'object' && - data !== null && - 'items' in data && - Array.isArray(data.items) - ); -} - -// Zod for runtime validation -import { z } from 'zod'; - -const ConfigSchema = z.object({ - setting: z.string(), - enabled: z.boolean(), -}); - -const config = ConfigSchema.parse(JSON.parse(str)); -console.log(config.setting); // Type-safe! -``` - -## Narrowing Techniques - -| Technique | Use When | -|-----------|----------| -| `typeof` | Primitives (string, number, boolean) | -| `instanceof` | Class instances | -| `in` operator | Object property checks | -| Type predicates | Complex type guards | -| Zod/validation | External data (API, JSON) | - -## Reference - -- [TypeScript Narrowing](https://www.typescriptlang.org/docs/handbook/2/narrowing.html) -- [Zod Documentation](https://zod.dev/) diff --git a/frontend/.claude/skills/e2e-tester/SKILL.md b/frontend/.claude/skills/e2e-tester/SKILL.md index f560d33c17..40972f320b 100644 --- a/frontend/.claude/skills/e2e-tester/SKILL.md +++ b/frontend/.claude/skills/e2e-tester/SKILL.md @@ -10,712 +10,71 @@ Write end-to-end tests using Playwright against a full Redpanda Console stack ru ## When to Use This Skill -- Testing 2+ step user journeys (login → action → verify) +- Testing 2+ step user journeys (login -> action -> verify) - Multi-page workflows - Browser automation with Playwright -**NOT for:** Component unit tests → use [testing](../testing/SKILL.md) +**NOT for:** Component unit tests -> use [testing](../testing/SKILL.md) ## Critical Rules **ALWAYS:** - - Run `bun run build` before running E2E tests (frontend assets required) - Use `testcontainers` API for container management (never manual `docker` commands in tests) -- Test 2+ step user journeys (multi-page, multi-step scenarios) - Use `page.getByRole()` and `page.getByLabel()` selectors (avoid CSS selectors) -- Add `data-testid` attributes to components when semantic selectors aren't available -- Use Task tool with MCP Playwright agents to analyze failures and get test status -- Use Task tool with Explore agent to find missing testids in UI components +- Add `data-testid` attributes when semantic selectors aren't available +- Use Task tool with MCP Playwright agents to analyze failures - Clean up test data using `afterEach` to call cleanup API endpoints **NEVER:** - -- Test UI component rendering (that belongs in unit/integration tests) +- Test UI component rendering (use unit/integration tests) - Use brittle CSS selectors like `.class-name` or `#id` -- Use force:true when calling .click() -- Use waitForTimeout in e2e tests +- Use `force:true` on `.click()` or `waitForTimeout` - Hard-code wait times (use `waitFor` with conditions) - Leave containers running after test failures -- Commit test screenshots to git (add to `.gitignore`) -- Add testids without understanding the component's purpose and context - -## Test Architecture - -### Stack Components - -**OSS Mode (`bun run e2e-test`):** -- Redpanda container (Kafka broker + Schema Registry + Admin API) -- Backend container (Go binary serving API + embedded frontend) -- OwlShop container (test data generator) - -**Enterprise Mode (`bun run e2e-test-enterprise`):** -- Same as OSS + Enterprise features (RBAC, SSO, etc.) -- Requires `console-enterprise` repo checked out alongside `console` - -**Network Setup:** -- All containers on shared Docker network -- Internal addresses: `redpanda:9092`, `console-backend:3000` -- External access: `localhost:19092`, `localhost:3000` - -### Test Container Lifecycle - -``` -Setup (global-setup.mjs): -1. Build frontend (frontend/build/) -2. Copy frontend assets to backend/pkg/embed/frontend/ -3. Build backend Docker image with testcontainers -4. Start Redpanda container with SASL auth -5. Start backend container serving frontend -6. Wait for services to be ready - -Tests run... - -Teardown (global-teardown.mjs): -1. Stop backend container -2. Stop Redpanda container -3. Remove Docker network -4. Clean up copied frontend assets -``` - -## Workflow - -### 1. Prerequisites - -```bash -# Build frontend (REQUIRED before E2E tests) -bun run build - -# Verify Docker is running -docker ps -``` - -### 2. Write Test - -**File location:** `tests//*.spec.ts` - -**Structure:** - -```typescript -import { test, expect } from '@playwright/test'; - -test.describe('Feature Name', () => { - test('user can complete workflow', async ({ page }) => { - // Navigate to page - await page.goto('/feature'); - - // Interact with elements - await page.getByRole('button', { name: 'Create' }).click(); - await page.getByLabel('Name').fill('test-item'); - - // Submit and verify - await page.getByRole('button', { name: 'Submit' }).click(); - await expect(page.getByText('Success')).toBeVisible(); - - // Verify navigation or state change - await expect(page).toHaveURL(/\/feature\/test-item/); - }); -}); -``` - -### 3. Selectors Best Practices - -**Prefer accessibility selectors:** - -```typescript -// ✅ GOOD: Role-based (accessible) -page.getByRole('button', { name: 'Create Topic' }) -page.getByLabel('Topic Name') -page.getByText('Success message') - -// ✅ GOOD: Test IDs when role isn't clear -page.getByTestId('topic-list-item') - -// ❌ BAD: CSS selectors (brittle) -page.locator('.btn-primary') -page.locator('#topic-name-input') -``` - -**Add test IDs to components:** - -```typescript -// In React component - - -// In test -await page.getByTestId('create-topic-button').click(); -``` - -### 4. Async Operations - -```typescript -// ✅ GOOD: Wait for specific condition -await expect(page.getByRole('status')).toHaveText('Ready'); - -// ✅ GOOD: Wait for navigation -await page.waitForURL('**/topics/my-topic'); - -// ✅ GOOD: Wait for API response -await page.waitForResponse(resp => - resp.url().includes('/api/topics') && resp.status() === 200 -); - -// ❌ BAD: Fixed timeouts -await page.waitForTimeout(5000); -``` - -### 5. Authentication - -**OSS Mode:** No authentication required - -**Enterprise Mode:** Basic auth with `e2euser:very-secret` - -```typescript -test.use({ - httpCredentials: { - username: 'e2euser', - password: 'very-secret', - }, -}); -``` - -### 6. Run Tests - -```bash -# OSS tests -bun run build # Build frontend first! -bun run e2e-test # Run all OSS tests - -# Enterprise tests (requires console-enterprise repo) -bun run build -bun run e2e-test-enterprise - -# UI mode (debugging) -bun run e2e-test:ui - -# Specific test file -bun run e2e-test tests/topics/create-topic.spec.ts - -# Update snapshots -bun run e2e-test --update-snapshots -``` - -### 7. Debugging - -**Failed test debugging:** - -```bash -# Check container logs -docker ps -a | grep console-backend -docker logs - -# Check if services are accessible -curl http://localhost:3000 -curl http://localhost:19092 - -# Run with debug output -DEBUG=pw:api bun run e2e-test - -# Keep containers running on failure -TESTCONTAINERS_RYUK_DISABLED=true bun run e2e-test -``` - -**Playwright debugging tools:** - -```typescript -// Add to test for debugging -await page.pause(); // Opens Playwright Inspector - -// Screenshot on failure (automatic in config) -await page.screenshot({ path: 'debug.png' }); - -// Get page content for debugging -console.log(await page.content()); -``` - -## Common Patterns - -### Multi-Step Workflows - -```typescript -test('user creates, configures, and tests topic', async ({ page }) => { - // Step 1: Navigate and create - await page.goto('/topics'); - await page.getByRole('button', { name: 'Create Topic' }).click(); - - // Step 2: Fill form - await page.getByLabel('Topic Name').fill('test-topic'); - await page.getByLabel('Partitions').fill('3'); - await page.getByRole('button', { name: 'Create' }).click(); - - // Step 3: Verify creation - await expect(page.getByText('Topic created successfully')).toBeVisible(); - await expect(page).toHaveURL(/\/topics\/test-topic/); - - // Step 4: Configure topic - await page.getByRole('button', { name: 'Configure' }).click(); - await page.getByLabel('Retention Hours').fill('24'); - await page.getByRole('button', { name: 'Save' }).click(); - - // Step 5: Verify configuration - await expect(page.getByText('Configuration saved')).toBeVisible(); -}); -``` - -### Testing Forms - -```typescript -test('form validation works correctly', async ({ page }) => { - await page.goto('/create-topic'); - - // Submit empty form - should show errors - await page.getByRole('button', { name: 'Create' }).click(); - await expect(page.getByText('Name is required')).toBeVisible(); - - // Fill valid data - should succeed - await page.getByLabel('Topic Name').fill('valid-topic'); - await page.getByRole('button', { name: 'Create' }).click(); - await expect(page.getByText('Success')).toBeVisible(); -}); -``` - -### Testing Data Tables - -```typescript -test('user can filter and sort topics', async ({ page }) => { - await page.goto('/topics'); - - // Filter - await page.getByPlaceholder('Search topics').fill('test-'); - await expect(page.getByRole('row')).toHaveCount(3); // Header + 2 results - - // Sort - await page.getByRole('columnheader', { name: 'Name' }).click(); - const firstRow = page.getByRole('row').nth(1); - await expect(firstRow).toContainText('test-topic-a'); -}); -``` - -### API Interactions - -```typescript -test('creating topic triggers backend API', async ({ page }) => { - // Listen for API call - const apiPromise = page.waitForResponse( - resp => resp.url().includes('/api/topics') && resp.status() === 201 - ); - - // Trigger action - await page.goto('/topics'); - await page.getByRole('button', { name: 'Create Topic' }).click(); - await page.getByLabel('Name').fill('api-test-topic'); - await page.getByRole('button', { name: 'Create' }).click(); - - // Verify API was called - const response = await apiPromise; - const body = await response.json(); - expect(body.name).toBe('api-test-topic'); -}); -``` - -## Testcontainers Setup - -### Frontend Asset Copy (Required) - -The backend Docker image needs frontend assets embedded at build time: - -```typescript -// In global-setup.mjs -async function buildBackendImage(isEnterprise) { - // Copy frontend build to backend embed directory - const frontendBuildDir = resolve(__dirname, '../build'); - const embedDir = join(backendDir, 'pkg/embed/frontend'); - await execAsync(`cp -r "${frontendBuildDir}"/* "${embedDir}"/`); - - // Build Docker image using testcontainers - // Docker doesn't allow referencing files outside build context, - // so we temporarily copy the Dockerfile into the build context - const tempDockerfile = join(backendDir, '.dockerfile.e2e.tmp'); - await execAsync(`cp "${dockerfilePath}" "${tempDockerfile}"`); - - try { - await GenericContainer - .fromDockerfile(backendDir, '.dockerfile.e2e.tmp') - .build(imageTag, { deleteOnExit: false }); - } finally { - await execAsync(`rm -f "${tempDockerfile}"`).catch(() => {}); - await execAsync(`find "${embedDir}" -mindepth 1 ! -name '.gitignore' -delete`).catch(() => {}); - } -} -``` - -### Container Configuration - -**Backend container:** -```typescript -const backend = await new GenericContainer(imageTag) - .withNetwork(network) - .withNetworkAliases('console-backend') - .withExposedPorts({ container: 3000, host: 3000 }) - .withBindMounts([{ - source: configPath, - target: '/etc/console/config.yaml' - }]) - .withCommand(['--config.filepath=/etc/console/config.yaml']) - .start(); -``` - -**Redpanda container:** -```typescript -const redpanda = await new GenericContainer('redpandadata/redpanda:v25.2.1') - .withNetwork(network) - .withNetworkAliases('redpanda') - .withExposedPorts( - { container: 19_092, host: 19_092 }, // Kafka - { container: 18_081, host: 18_081 }, // Schema Registry - { container: 9644, host: 19_644 } // Admin API - ) - .withEnvironment({ RP_BOOTSTRAP_USER: 'e2euser:very-secret' }) - .withHealthCheck({ - test: ['CMD-SHELL', "rpk cluster health | grep -E 'Healthy:.+true' || exit 1"], - interval: 15_000, - retries: 5 - }) - .withWaitStrategy(Wait.forHealthCheck()) - .start(); -``` - -## CI Integration - -### GitHub Actions Setup - -```yaml -e2e-test: - runs-on: ubuntu-latest-8 - steps: - - uses: actions/checkout@v5 - - uses: oven-sh/setup-bun@v2 - - - name: Install dependencies - run: bun install --frozen-lockfile - - - name: Build frontend - run: | - REACT_APP_CONSOLE_GIT_SHA=$(echo $GITHUB_SHA | cut -c 1-6) - bun run build - - - name: Install Playwright browsers - run: bun run install:chromium +- Commit test screenshots to git - - name: Run E2E tests - run: bun run e2e-test - - - name: Upload test report - if: failure() - uses: actions/upload-artifact@v4 - with: - name: playwright-report - path: frontend/playwright-report/ -``` - -## Test ID Management - -### Finding Missing Test IDs - -Use the Task tool with Explore agent to systematically find missing testids: - -``` -Use Task tool with: -subagent_type: Explore -prompt: Search through [feature] UI components and identify all interactive - elements (buttons, inputs, links, selects) missing data-testid attributes. - List with file:line, element type, purpose, and suggested testid name. -``` - -**Example output:** -``` -schema-list.tsx:207 - button - "Edit compatibility" - schema-edit-compatibility-btn -schema-list.tsx:279 - button - "Create new schema" - schema-create-new-btn -schema-details.tsx:160 - button - "Edit compatibility" - schema-details-edit-compatibility-btn -``` - -### Adding Test IDs - -**Naming Convention:** -- Use kebab-case: `data-testid="feature-action-element"` -- Be specific: Include feature name + action + element type -- For dynamic items: Use template literals `data-testid={\`item-delete-\${id}\`}` - -**Examples:** - -```tsx -// ✅ GOOD: Specific button action - - -// ✅ GOOD: Form input with context - - -// ✅ GOOD: Table row with dynamic ID - - {schema.name} - - -// ✅ GOOD: Delete button in list -} - onClick={() => deleteSchema(schema.name)} -/> - -// ❌ BAD: Too generic - - -// ❌ BAD: Using CSS classes as identifiers - -``` - -**Where to Add:** -1. **Primary actions**: Create, Save, Delete, Edit, Submit, Cancel buttons -2. **Navigation**: Links to detail pages, breadcrumbs -3. **Forms**: All input fields, selects, checkboxes, radio buttons -4. **Lists/Tables**: Row identifiers, action buttons within rows -5. **Dialogs/Modals**: Open/close buttons, form elements inside -6. **Search/Filter**: Search inputs, filter dropdowns, clear buttons - -**Process:** -1. Use Task/Explore to find missing testids in target feature -2. Read the component file to understand context -3. Add `data-testid` following naming convention -4. Update tests to use new testids -5. Run tests to verify selectors work - -## Analyzing Test Failures - -### Using MCP Playwright Agents - -**Check Test Status:** -```typescript -// Use mcp__playwright-test__test_list to see all tests -// Use mcp__playwright-test__test_run to get detailed results -// Use mcp__playwright-test__test_debug to analyze specific failures -``` - -### Reading Playwright Logs - -**Common failure patterns and fixes:** - -#### 1. Element Not Found -``` -Error: locator.click: Target closed -Error: Timeout 30000ms exceeded waiting for locator -``` - -**Analysis steps:** -1. Check if element has correct testid/role -2. Verify element is visible (not hidden/collapsed) -3. Check for timing issues (element loads async) -4. Look for dynamic content that changes selector - -**Fix:** -```typescript -// ❌ BAD: Element might not be loaded -await page.getByRole('button', { name: 'Create' }).click(); - -// ✅ GOOD: Wait for element to be visible -await expect(page.getByRole('button', { name: 'Create' })).toBeVisible(); -await page.getByRole('button', { name: 'Create' }).click(); - -// ✅ BETTER: Add testid for stability -await page.getByTestId('create-button').click(); -``` - -#### 2. Selector Ambiguity -``` -Error: strict mode violation: locator('button') resolved to 3 elements -``` - -**Analysis:** -- Multiple elements match the selector -- Need more specific selector or testid - -**Fix:** -```typescript -// ❌ BAD: Multiple "Edit" buttons on page -await page.getByRole('button', { name: 'Edit' }).click(); - -// ✅ GOOD: More specific with testid -await page.getByTestId('schema-edit-compatibility-btn').click(); - -// ✅ GOOD: Scope within container -await page.getByRole('region', { name: 'Schema Details' }) - .getByRole('button', { name: 'Edit' }).click(); -``` - -#### 3. Timing/Race Conditions -``` -Error: expect(locator).toHaveText() -Expected string: "Success" -Received string: "Loading..." -``` - -**Analysis:** -- Test assertion ran before UI updated -- Need to wait for specific state - -**Fix:** -```typescript -// ❌ BAD: Doesn't wait for state change -await page.getByRole('button', { name: 'Save' }).click(); -expect(page.getByText('Success')).toBeVisible(); - -// ✅ GOOD: Wait for the expected state -await page.getByRole('button', { name: 'Save' }).click(); -await expect(page.getByText('Success')).toBeVisible({ timeout: 5000 }); -``` - -#### 4. Navigation Issues -``` -Error: page.goto: net::ERR_CONNECTION_REFUSED -``` - -**Analysis:** -- Backend/frontend not running -- Wrong URL or port - -**Fix:** -```bash -# Check containers are running -docker ps | grep console-backend - -# Check container logs -docker logs - -# Verify port mapping -curl http://localhost:3000 - -# Check testcontainer state file -cat .testcontainers-state.json -``` - -### Systematic Failure Analysis Workflow - -**When tests fail:** - -1. **Get Test Results** - ``` - Use mcp__playwright-test__test_run or check console output - Identify which tests failed and error messages - ``` - -2. **Analyze Error Patterns** - - Selector not found → Missing/wrong testid or element not visible - - Strict mode violation → Need more specific selector - - Timeout → Element loads async, need waitFor - - Connection refused → Container/service not running - -3. **Find Missing Test IDs** - ``` - Use Task tool with Explore agent to find missing testids in the - components related to failed tests - ``` - -4. **Add Test IDs** - - Read component file - - Add `data-testid` to problematic elements - - Follow naming convention - - Format with biome - -5. **Update Tests** - - Replace brittle selectors with stable testids - - Add proper wait conditions - - Verify with test run - -6. **Verify Fixes** - ``` - Run specific test file to verify fix - Run full suite to ensure no regressions - ``` - -## Troubleshooting - -### Container Fails to Start +## Commands -```bash -# Check if frontend build exists -ls frontend/build/ - -# Check if Docker image built successfully -docker images | grep console-backend - -# Check container logs -docker logs - -# Verify Docker network -docker network ls | grep testcontainers -``` - -### Test Timeout Issues - -```typescript -// Increase timeout for slow operations -test('slow operation', async ({ page }) => { - test.setTimeout(60000); // 60 seconds - - await page.goto('/slow-page'); - await expect(page.getByText('Loaded')).toBeVisible({ timeout: 30000 }); -}); -``` - -### Port Already in Use - -```bash -# Find and kill process using port 3000 -lsof -ti:3000 | xargs kill -9 - -# Or use different ports in test config -``` - -## Quick Reference - -**Test types:** -- E2E tests (`*.spec.ts`): Complete user workflows, browser interactions -- Integration tests (`*.test.tsx`): Component + API, no browser -- Unit tests (`*.test.ts`): Pure logic, utilities - -**Commands:** ```bash bun run build # Build frontend (REQUIRED first!) bun run e2e-test # Run OSS E2E tests bun run e2e-test-enterprise # Run Enterprise E2E tests bun run e2e-test:ui # Playwright UI mode (debugging) +bun run e2e-test tests/topics/create-topic.spec.ts # Specific file ``` -**Selector priority:** +## Test Architecture + +**OSS Mode (`bun run e2e-test`):** Redpanda + Backend + OwlShop containers +**Enterprise Mode (`bun run e2e-test-enterprise`):** Same + RBAC, SSO (requires `console-enterprise` repo) + +File location: `tests//*.spec.ts` + +## Selector Priority + 1. `getByRole()` - Best for accessibility 2. `getByLabel()` - For form inputs 3. `getByText()` - For content verification 4. `getByTestId()` - When semantic selectors aren't clear 5. CSS selectors - Avoid if possible -**Wait strategies:** -- `waitForURL()` - Navigation complete -- `waitForResponse()` - API call finished -- `waitFor()` with `expect()` - Element state changed -- Never use fixed `waitForTimeout()` unless absolutely necessary +## Test ID Naming + +- kebab-case: `data-testid="feature-action-element"` +- Specific: include feature name + action + element type +- Dynamic: `data-testid={\`item-delete-\${id}\`}` + +## References + +- [Container Setup](references/container-setup.md) — Testcontainer lifecycle, configs, CI setup +- [Test Patterns](references/test-patterns.md) — Multi-step workflows, forms, tables, API testing +- [Failure Analysis](references/failure-analysis.md) — Error patterns, debugging, MCP Playwright agents ## Output After completing work: - 1. Confirm frontend build succeeded 2. Verify all E2E tests pass 3. Note any new test IDs added to components 4. Mention cleanup of test containers -5. Report test execution time and coverage \ No newline at end of file diff --git a/frontend/.claude/skills/e2e-tester/references/container-setup.md b/frontend/.claude/skills/e2e-tester/references/container-setup.md new file mode 100644 index 0000000000..2e20627c50 --- /dev/null +++ b/frontend/.claude/skills/e2e-tester/references/container-setup.md @@ -0,0 +1,163 @@ +# Container Setup Reference + +## Test Container Lifecycle + +``` +Setup (global-setup.mjs): +1. Build frontend (frontend/build/) +2. Copy frontend assets to backend/pkg/embed/frontend/ +3. Build backend Docker image with testcontainers +4. Start Redpanda container with SASL auth +5. Start backend container serving frontend +6. Wait for services to be ready + +Tests run... + +Teardown (global-teardown.mjs): +1. Stop backend container +2. Stop Redpanda container +3. Remove Docker network +4. Clean up copied frontend assets +``` + +## Network Setup + +- All containers on shared Docker network +- Internal addresses: `redpanda:9092`, `console-backend:3000` +- External access: `localhost:19092`, `localhost:3000` + +## Frontend Asset Copy (Required) + +The backend Docker image needs frontend assets embedded at build time: + +```typescript +// In global-setup.mjs +async function buildBackendImage(isEnterprise) { + // Copy frontend build to backend embed directory + const frontendBuildDir = resolve(__dirname, '../build'); + const embedDir = join(backendDir, 'pkg/embed/frontend'); + await execAsync(`cp -r "${frontendBuildDir}"/* "${embedDir}"/`); + + // Build Docker image using testcontainers + // Docker doesn't allow referencing files outside build context, + // so we temporarily copy the Dockerfile into the build context + const tempDockerfile = join(backendDir, '.dockerfile.e2e.tmp'); + await execAsync(`cp "${dockerfilePath}" "${tempDockerfile}"`); + + try { + await GenericContainer + .fromDockerfile(backendDir, '.dockerfile.e2e.tmp') + .build(imageTag, { deleteOnExit: false }); + } finally { + await execAsync(`rm -f "${tempDockerfile}"`).catch(() => {}); + await execAsync(`find "${embedDir}" -mindepth 1 ! -name '.gitignore' -delete`).catch(() => {}); + } +} +``` + +## Container Configuration + +**Backend container:** +```typescript +const backend = await new GenericContainer(imageTag) + .withNetwork(network) + .withNetworkAliases('console-backend') + .withExposedPorts({ container: 3000, host: 3000 }) + .withBindMounts([{ + source: configPath, + target: '/etc/console/config.yaml' + }]) + .withCommand(['--config.filepath=/etc/console/config.yaml']) + .start(); +``` + +**Redpanda container:** +```typescript +const redpanda = await new GenericContainer('redpandadata/redpanda:v25.2.1') + .withNetwork(network) + .withNetworkAliases('redpanda') + .withExposedPorts( + { container: 19_092, host: 19_092 }, // Kafka + { container: 18_081, host: 18_081 }, // Schema Registry + { container: 9644, host: 19_644 } // Admin API + ) + .withEnvironment({ RP_BOOTSTRAP_USER: 'e2euser:very-secret' }) + .withHealthCheck({ + test: ['CMD-SHELL', "rpk cluster health | grep -E 'Healthy:.+true' || exit 1"], + interval: 15_000, + retries: 5 + }) + .withWaitStrategy(Wait.forHealthCheck()) + .start(); +``` + +## Authentication + +**OSS Mode:** No authentication required + +**Enterprise Mode:** Basic auth with `e2euser:very-secret` + +```typescript +test.use({ + httpCredentials: { + username: 'e2euser', + password: 'very-secret', + }, +}); +``` + +## CI Integration (GitHub Actions) + +```yaml +e2e-test: + runs-on: ubuntu-latest-8 + steps: + - uses: actions/checkout@v5 + - uses: oven-sh/setup-bun@v2 + + - name: Install dependencies + run: bun install --frozen-lockfile + + - name: Build frontend + run: | + REACT_APP_CONSOLE_GIT_SHA=$(echo $GITHUB_SHA | cut -c 1-6) + bun run build + + - name: Install Playwright browsers + run: bun run install:chromium + + - name: Run E2E tests + run: bun run e2e-test + + - name: Upload test report + if: failure() + uses: actions/upload-artifact@v4 + with: + name: playwright-report + path: frontend/playwright-report/ +``` + +## Troubleshooting + +### Container Fails to Start + +```bash +# Check if frontend build exists +ls frontend/build/ + +# Check if Docker image built successfully +docker images | grep console-backend + +# Check container logs +docker logs + +# Verify Docker network +docker network ls | grep testcontainers +``` + +### Port Already in Use + +```bash +# Find and kill process using port 3000 +lsof -ti:3000 | xargs kill -9 +``` diff --git a/frontend/.claude/skills/e2e-tester/references/failure-analysis.md b/frontend/.claude/skills/e2e-tester/references/failure-analysis.md new file mode 100644 index 0000000000..48271470af --- /dev/null +++ b/frontend/.claude/skills/e2e-tester/references/failure-analysis.md @@ -0,0 +1,156 @@ +# Failure Analysis Reference + +## Using MCP Playwright Agents + +```typescript +// Use mcp__playwright-test__test_list to see all tests +// Use mcp__playwright-test__test_run to get detailed results +// Use mcp__playwright-test__test_debug to analyze specific failures +``` + +## Finding Missing Test IDs + +Use the Task tool with Explore agent: + +``` +subagent_type: Explore +prompt: Search through [feature] UI components and identify all interactive + elements (buttons, inputs, links, selects) missing data-testid attributes. + List with file:line, element type, purpose, and suggested testid name. +``` + +## Common Failure Patterns + +### 1. Element Not Found + +``` +Error: locator.click: Target closed +Error: Timeout 30000ms exceeded waiting for locator +``` + +**Analysis steps:** +1. Check if element has correct testid/role +2. Verify element is visible (not hidden/collapsed) +3. Check for timing issues (element loads async) +4. Look for dynamic content that changes selector + +**Fix:** +```typescript +// BAD: Element might not be loaded +await page.getByRole('button', { name: 'Create' }).click(); + +// GOOD: Wait for element to be visible +await expect(page.getByRole('button', { name: 'Create' })).toBeVisible(); +await page.getByRole('button', { name: 'Create' }).click(); + +// BETTER: Add testid for stability +await page.getByTestId('create-button').click(); +``` + +### 2. Selector Ambiguity + +``` +Error: strict mode violation: locator('button') resolved to 3 elements +``` + +**Fix:** +```typescript +// BAD: Multiple "Edit" buttons on page +await page.getByRole('button', { name: 'Edit' }).click(); + +// GOOD: More specific with testid +await page.getByTestId('schema-edit-compatibility-btn').click(); + +// GOOD: Scope within container +await page.getByRole('region', { name: 'Schema Details' }) + .getByRole('button', { name: 'Edit' }).click(); +``` + +### 3. Timing/Race Conditions + +``` +Error: expect(locator).toHaveText() +Expected string: "Success" +Received string: "Loading..." +``` + +**Fix:** +```typescript +// BAD: Doesn't wait for state change +await page.getByRole('button', { name: 'Save' }).click(); +expect(page.getByText('Success')).toBeVisible(); + +// GOOD: Wait for the expected state +await page.getByRole('button', { name: 'Save' }).click(); +await expect(page.getByText('Success')).toBeVisible({ timeout: 5000 }); +``` + +### 4. Navigation Issues + +``` +Error: page.goto: net::ERR_CONNECTION_REFUSED +``` + +**Fix:** +```bash +# Check containers are running +docker ps | grep console-backend + +# Check container logs +docker logs + +# Verify port mapping +curl http://localhost:3000 + +# Check testcontainer state file +cat .testcontainers-state.json +``` + +## Systematic Failure Analysis Workflow + +1. **Get Test Results** — Use `mcp__playwright-test__test_run` or check console output +2. **Analyze Error Patterns:** + - Selector not found -> Missing/wrong testid or element not visible + - Strict mode violation -> Need more specific selector + - Timeout -> Element loads async, need waitFor + - Connection refused -> Container/service not running +3. **Find Missing Test IDs** — Use Task tool with Explore agent +4. **Add Test IDs** — Read component, add `data-testid`, follow naming convention +5. **Update Tests** — Replace brittle selectors with stable testids, add wait conditions +6. **Verify Fixes** — Run specific test file, then full suite + +## Debugging Commands + +```bash +# Check container logs +docker ps -a | grep console-backend +docker logs + +# Check if services are accessible +curl http://localhost:3000 +curl http://localhost:19092 + +# Run with debug output +DEBUG=pw:api bun run e2e-test + +# Keep containers running on failure +TESTCONTAINERS_RYUK_DISABLED=true bun run e2e-test +``` + +**In-test debugging:** +```typescript +await page.pause(); // Opens Playwright Inspector +await page.screenshot({ path: 'debug.png' }); +console.log(await page.content()); +``` + +## Test Timeout Issues + +```typescript +// Increase timeout for slow operations +test('slow operation', async ({ page }) => { + test.setTimeout(60000); // 60 seconds + await page.goto('/slow-page'); + await expect(page.getByText('Loaded')).toBeVisible({ timeout: 30000 }); +}); +``` diff --git a/frontend/.claude/skills/e2e-tester/references/test-patterns.md b/frontend/.claude/skills/e2e-tester/references/test-patterns.md new file mode 100644 index 0000000000..22b08983a6 --- /dev/null +++ b/frontend/.claude/skills/e2e-tester/references/test-patterns.md @@ -0,0 +1,153 @@ +# Test Patterns Reference + +## Basic Test Structure + +```typescript +import { test, expect } from '@playwright/test'; + +test.describe('Feature Name', () => { + test('user can complete workflow', async ({ page }) => { + await page.goto('/feature'); + await page.getByRole('button', { name: 'Create' }).click(); + await page.getByLabel('Name').fill('test-item'); + await page.getByRole('button', { name: 'Submit' }).click(); + await expect(page.getByText('Success')).toBeVisible(); + await expect(page).toHaveURL(/\/feature\/test-item/); + }); +}); +``` + +## Multi-Step Workflows + +```typescript +test('user creates, configures, and tests topic', async ({ page }) => { + // Step 1: Navigate and create + await page.goto('/topics'); + await page.getByRole('button', { name: 'Create Topic' }).click(); + + // Step 2: Fill form + await page.getByLabel('Topic Name').fill('test-topic'); + await page.getByLabel('Partitions').fill('3'); + await page.getByRole('button', { name: 'Create' }).click(); + + // Step 3: Verify creation + await expect(page.getByText('Topic created successfully')).toBeVisible(); + await expect(page).toHaveURL(/\/topics\/test-topic/); + + // Step 4: Configure topic + await page.getByRole('button', { name: 'Configure' }).click(); + await page.getByLabel('Retention Hours').fill('24'); + await page.getByRole('button', { name: 'Save' }).click(); + + // Step 5: Verify configuration + await expect(page.getByText('Configuration saved')).toBeVisible(); +}); +``` + +## Testing Forms + +```typescript +test('form validation works correctly', async ({ page }) => { + await page.goto('/create-topic'); + + // Submit empty form - should show errors + await page.getByRole('button', { name: 'Create' }).click(); + await expect(page.getByText('Name is required')).toBeVisible(); + + // Fill valid data - should succeed + await page.getByLabel('Topic Name').fill('valid-topic'); + await page.getByRole('button', { name: 'Create' }).click(); + await expect(page.getByText('Success')).toBeVisible(); +}); +``` + +## Testing Data Tables + +```typescript +test('user can filter and sort topics', async ({ page }) => { + await page.goto('/topics'); + + // Filter + await page.getByPlaceholder('Search topics').fill('test-'); + await expect(page.getByRole('row')).toHaveCount(3); // Header + 2 results + + // Sort + await page.getByRole('columnheader', { name: 'Name' }).click(); + const firstRow = page.getByRole('row').nth(1); + await expect(firstRow).toContainText('test-topic-a'); +}); +``` + +## API Interactions + +```typescript +test('creating topic triggers backend API', async ({ page }) => { + // Listen for API call + const apiPromise = page.waitForResponse( + resp => resp.url().includes('/api/topics') && resp.status() === 201 + ); + + // Trigger action + await page.goto('/topics'); + await page.getByRole('button', { name: 'Create Topic' }).click(); + await page.getByLabel('Name').fill('api-test-topic'); + await page.getByRole('button', { name: 'Create' }).click(); + + // Verify API was called + const response = await apiPromise; + const body = await response.json(); + expect(body.name).toBe('api-test-topic'); +}); +``` + +## Async Operations + +```typescript +// Wait for specific condition +await expect(page.getByRole('status')).toHaveText('Ready'); + +// Wait for navigation +await page.waitForURL('**/topics/my-topic'); + +// Wait for API response +await page.waitForResponse(resp => + resp.url().includes('/api/topics') && resp.status() === 200 +); + +// NEVER use fixed timeouts +// await page.waitForTimeout(5000); // BAD +``` + +## Selectors Best Practices + +```typescript +// GOOD: Role-based (accessible) +page.getByRole('button', { name: 'Create Topic' }) +page.getByLabel('Topic Name') +page.getByText('Success message') + +// GOOD: Test IDs when role isn't clear +page.getByTestId('topic-list-item') + +// BAD: CSS selectors (brittle) +page.locator('.btn-primary') +page.locator('#topic-name-input') +``` + +## Adding Test IDs to Components + +```tsx +// In React component + + +// In test +await page.getByTestId('create-topic-button').click(); +``` + +**Where to Add:** +1. Primary actions: Create, Save, Delete, Edit, Submit, Cancel buttons +2. Navigation: Links to detail pages, breadcrumbs +3. Forms: All input fields, selects, checkboxes, radio buttons +4. Lists/Tables: Row identifiers, action buttons within rows +5. Dialogs/Modals: Open/close buttons, form elements inside +6. Search/Filter: Search inputs, filter dropdowns, clear buttons diff --git a/frontend/.claude/skills/form-refactorer/SKILL.md b/frontend/.claude/skills/form-refactorer/SKILL.md index 171e9391a8..b3388ac44a 100644 --- a/frontend/.claude/skills/form-refactorer/SKILL.md +++ b/frontend/.claude/skills/form-refactorer/SKILL.md @@ -11,170 +11,15 @@ Refactor legacy forms to modern Redpanda UI Registry Field components with react ### MUST USE (Modern Pattern) -```tsx -import { - FieldSet, - FieldLegend, - FieldGroup, - Field, - FieldLabel, - FieldContent, - FieldDescription, - FieldError, - FieldSeparator -} from "components/redpanda-ui/components/field"; -``` - +- Field component family from `components/redpanda-ui/components/field` - react-hook-form for form state management - Zod for validation schemas via `zodResolver` -- Field component family for all form layouts ### FORBIDDEN (Legacy Patterns) -See [no-legacy](../code-standards/rules/no-legacy.md) for prohibited patterns. - -```tsx -// ❌ Also never import legacy form components -import { Form } from "components/redpanda-ui/components/form"; -``` - -## Refactoring Workflow - -### Step 1: Identify Legacy Patterns - -Scan the form file for these indicators: +See [no-legacy](../code-standards/rules/no-legacy.md) for prohibited patterns. Also never import `Form` from `components/redpanda-ui/components/form`. -**Legacy imports to remove:** -- `@chakra-ui/react` form components (FormControl, FormLabel, FormErrorMessage, FormHelperText) -- `@redpanda-data/ui` Form component -- `components/redpanda-ui/components/form` (legacy) -- Yup validation imports - -**Legacy patterns to replace:** -- `` → `` -- `` → `` -- `` → `` -- `` → `` -- Yup schemas → Zod schemas - -### Step 2: Set Up Modern Dependencies - -Add required imports: - -```tsx -import { useForm } from "react-hook-form"; -import { zodResolver } from "@hookform/resolvers/zod"; -import { z } from "zod"; -import { - FieldSet, - FieldLegend, - FieldGroup, - Field, - FieldLabel, - FieldError, - FieldDescription -} from "components/redpanda-ui/components/field"; -``` - -Create Zod schema replacing Yup: - -```tsx -// Replace Yup schema -const schema = z.object({ - name: z.string().min(1, "Name is required"), - email: z.string().email("Invalid email address"), - age: z.number().min(18, "Must be 18 or older"), -}); - -// Use with react-hook-form -const { register, handleSubmit, formState: { errors } } = useForm({ - resolver: zodResolver(schema), -}); -``` - -### Step 3: Restructure Form Layout - -Replace legacy structure with Field components: - -**Pattern: Simple vertical field** - -```tsx - - Field Label - - Optional helper text - {errors.fieldName && {errors.fieldName.message}} - -``` - -**Pattern: Horizontal field (switch/checkbox)** - -```tsx - - - - Enable Feature - This enables the feature - - -``` - -**Pattern: Multiple fields in group** - -```tsx -
- Section Title - Section description (optional) - - ... - ... - {/* Optional visual divider */} - ... - -
-``` - -### Step 4: Handle Validation Errors - -Apply error states to both Field and Input: - -```tsx - - Email - - {errors.email && {errors.email.message}} - -``` - -### Step 5: Ensure Accessibility - -Required accessibility attributes: - -1. **Label association:** Use `htmlFor` on FieldLabel matching Input `id` -2. **Error state:** Add `data-invalid` to Field when error exists -3. **ARIA invalid:** Add `aria-invalid` to Input when error exists -4. **Semantic grouping:** Use FieldSet + FieldLegend for related fields - -```tsx -// ✅ Correct accessibility - - Name - - {errors.name && {errors.name.message}} - -``` - -## Quick Reference - -### Component Hierarchy +## Component Hierarchy ``` FieldSet - Semantic fieldset container for related fields @@ -190,47 +35,27 @@ FieldSet - Semantic fieldset container for related fields └─ Field - Another field ``` -### Common Zod Patterns +## Key Patterns +**Vertical field:** ```tsx -// String validations -z.string().min(1, "Required") -z.string().email("Invalid email") -z.string().url("Invalid URL") -z.string().regex(/^[A-Z]/, "Must start with capital") - -// Number validations -z.number().min(0, "Must be positive") -z.number().max(100, "Max 100") -z.number().int("Must be integer") - -// Optional fields -z.string().optional() -z.string().nullable() - -// Arrays -z.array(z.string()).min(1, "At least one required") - -// Objects -z.object({ - nested: z.string(), -}) - -// Conditional validation -z.object({ - type: z.enum(["a", "b"]), - value: z.string(), -}).refine((data) => { - if (data.type === "a") return data.value.length > 5; - return true; -}, "Value must be longer than 5 for type A") + + Label + + {errors.fieldName && {errors.fieldName.message}} + ``` -## Advanced Scenarios - -For complex patterns (dynamic fields, field arrays, multi-section forms), see: -- [references/common-patterns.md](references/common-patterns.md) - Detailed patterns for advanced use cases -- [references/migration-examples.md](references/migration-examples.md) - Before/after examples +**Horizontal field (switch/checkbox):** +```tsx + + + + Enable Feature + Description + + +``` ## Checklist @@ -243,9 +68,10 @@ Before completing refactoring, verify: - [ ] Yup replaced with Zod + zodResolver - [ ] FieldSet + FieldLegend used for semantic grouping - [ ] FieldError conditionally rendered: `{errors.field && ...` -- [ ] No inline styling or className for layout (use Field orientation/FieldGroup) - [ ] Form submission handled via react-hook-form `handleSubmit` -## Documentation +## References -Full Field component documentation: https://redpanda-ui-registry.netlify.app/docs/field +- [Common Patterns](references/common-patterns.md) — Dynamic fields, field arrays, multi-section forms, pitfalls, Zod patterns +- [Migration Examples](references/migration-examples.md) — Before/after examples for Chakra/legacy -> modern +- [Field Component Docs](https://redpanda-ui-registry.netlify.app/docs/field) diff --git a/frontend/.claude/skills/form-refactorer/references/common-patterns.md b/frontend/.claude/skills/form-refactorer/references/common-patterns.md index 9b525436c6..28e6384525 100644 --- a/frontend/.claude/skills/form-refactorer/references/common-patterns.md +++ b/frontend/.claude/skills/form-refactorer/references/common-patterns.md @@ -340,20 +340,54 @@ export const InlineEditForm = () => { }; ``` +## Common Zod Patterns + +```tsx +// String validations +z.string().min(1, "Required") +z.string().email("Invalid email") +z.string().url("Invalid URL") +z.string().regex(/^[A-Z]/, "Must start with capital") + +// Number validations +z.number().min(0, "Must be positive") +z.number().max(100, "Max 100") +z.number().int("Must be integer") + +// Optional fields +z.string().optional() +z.string().nullable() + +// Arrays +z.array(z.string()).min(1, "At least one required") + +// Objects +z.object({ + nested: z.string(), +}) + +// Conditional validation +z.object({ + type: z.enum(["a", "b"]), + value: z.string(), +}).refine((data) => { + if (data.type === "a") return data.value.length > 5; + return true; +}, "Value must be longer than 5 for type A") +``` + ## Common Pitfalls ### Pitfall 1: Missing htmlFor/id Association -❌ **Wrong:** ```tsx +// WRONG: Email -``` -✅ **Correct:** -```tsx +// CORRECT: Email @@ -362,15 +396,13 @@ export const InlineEditForm = () => { ### Pitfall 2: Missing aria-invalid -❌ **Wrong:** ```tsx +// WRONG: -``` -✅ **Correct:** -```tsx +// CORRECT: @@ -378,42 +410,36 @@ export const InlineEditForm = () => { ### Pitfall 3: Conditional FieldError Without Check -❌ **Wrong:** ```tsx +// WRONG: {errors.email.message} -``` -✅ **Correct:** -```tsx +// CORRECT: {errors.email && {errors.email.message}} ``` ### Pitfall 4: Using Legacy Form Components -❌ **Wrong:** ```tsx +// WRONG: import { Form } from "components/redpanda-ui/components/form"; import { Form } from "@redpanda-data/ui"; -``` -✅ **Correct:** -```tsx +// CORRECT: import { Field, FieldLabel } from "components/redpanda-ui/components/field"; ``` ### Pitfall 5: Forgetting FieldContent for Horizontal Fields -❌ **Wrong:** ```tsx +// WRONG: Enable Receive updates -``` -✅ **Correct:** -```tsx +// CORRECT: diff --git a/frontend/.claude/skills/form-refactorer/references/migration-examples.md b/frontend/.claude/skills/form-refactorer/references/migration-examples.md index 65c53b85ee..3537cc08af 100644 --- a/frontend/.claude/skills/form-refactorer/references/migration-examples.md +++ b/frontend/.claude/skills/form-refactorer/references/migration-examples.md @@ -89,13 +89,13 @@ export const CreateServerForm = () => { ``` **Key Changes:** -- Replace Chakra `FormControl` → `Field` -- Replace Chakra `FormLabel` → `FieldLabel` with `htmlFor` -- Replace Chakra `FormErrorMessage` → `FieldError` +- Replace Chakra `FormControl` -> `Field` +- Replace Chakra `FormLabel` -> `FieldLabel` with `htmlFor` +- Replace Chakra `FormErrorMessage` -> `FieldError` - Add `FieldSet`, `FieldLegend`, `FieldGroup` for proper structure - Add `data-invalid` attribute to Field - Add `aria-invalid` to Input -- Replace Yup → Zod for validation +- Replace Yup -> Zod for validation - Import components from Redpanda UI Registry ## Example 2: Form with Descriptions @@ -140,10 +140,10 @@ export const SettingsForm = () => { ``` **Key Changes:** -- Replace `Form` component → native `
` -- Replace `Form.Field` → `Field` -- Replace `label` prop → `FieldLabel` component -- Replace `helperText` prop → `FieldDescription` component +- Replace `Form` component -> native `` +- Replace `Form.Field` -> `Field` +- Replace `label` prop -> `FieldLabel` component +- Replace `helperText` prop -> `FieldDescription` component - Add `id` and `htmlFor` for accessibility ## Example 3: Horizontal Field with Switch @@ -191,7 +191,7 @@ export const NotificationSettings = () => { ``` **Key Changes:** -- Replace Chakra layout props → `orientation="horizontal"` on Field +- Replace Chakra layout props -> `orientation="horizontal"` on Field - Use `FieldContent` to group label and description - Remove manual spacing/styling props - Add proper `htmlFor` association @@ -271,7 +271,7 @@ export const ProfileForm = () => { ``` **Key Changes:** -- Replace `Box` layout containers → `FieldSet` and `FieldGroup` +- Replace `Box` layout containers -> `FieldSet` and `FieldGroup` - Add `FieldLegend` for semantic grouping - Use `FieldSeparator` instead of margin/spacing props - Remove all Chakra spacing props (mb, mt, etc.) @@ -279,7 +279,7 @@ export const ProfileForm = () => { ## Common Migration Patterns -### Pattern 1: isInvalid → data-invalid + aria-invalid +### Pattern 1: isInvalid -> data-invalid + aria-invalid **Before:** ```tsx @@ -295,7 +295,7 @@ export const ProfileForm = () => { ``` -### Pattern 2: Helper Text → FieldDescription +### Pattern 2: Helper Text -> FieldDescription **Before:** ```tsx @@ -315,7 +315,7 @@ export const ProfileForm = () => { ``` -### Pattern 3: Error Messages → FieldError +### Pattern 3: Error Messages -> FieldError **Before:** ```tsx @@ -331,7 +331,7 @@ export const ProfileForm = () => { ``` -### Pattern 4: Form Sections → FieldGroup with FieldSeparator +### Pattern 4: Form Sections -> FieldGroup with FieldSeparator **Before:** ```tsx @@ -350,3 +350,56 @@ export const ProfileForm = () => { ... ``` + +## Refactoring Workflow + +### Step 1: Identify Legacy Patterns + +Scan the form file for these indicators: + +**Legacy imports to remove:** +- `@chakra-ui/react` form components (FormControl, FormLabel, FormErrorMessage, FormHelperText) +- `@redpanda-data/ui` Form component +- `components/redpanda-ui/components/form` (legacy) +- Yup validation imports + +**Legacy patterns to replace:** +- `` -> `` +- `` -> `` +- `` -> `` +- `` -> `` +- Yup schemas -> Zod schemas + +### Step 2: Set Up Modern Dependencies + +```tsx +import { useForm } from "react-hook-form"; +import { zodResolver } from "@hookform/resolvers/zod"; +import { z } from "zod"; +import { + FieldSet, FieldLegend, FieldGroup, Field, + FieldLabel, FieldError, FieldDescription +} from "components/redpanda-ui/components/field"; +``` + +### Step 3: Restructure Form Layout + +Replace legacy structure with Field components using patterns above. + +### Step 4: Handle Validation Errors + +Apply error states to both Field and Input: +```tsx + + Email + + {errors.email && {errors.email.message}} + +``` + +### Step 5: Ensure Accessibility + +1. **Label association:** Use `htmlFor` on FieldLabel matching Input `id` +2. **Error state:** Add `data-invalid` to Field when error exists +3. **ARIA invalid:** Add `aria-invalid` to Input when error exists +4. **Semantic grouping:** Use FieldSet + FieldLegend for related fields diff --git a/frontend/.claude/skills/react-best-practices/SKILL.md b/frontend/.claude/skills/react-best-practices/SKILL.md index 3f1e81052b..08932d1158 100644 --- a/frontend/.claude/skills/react-best-practices/SKILL.md +++ b/frontend/.claude/skills/react-best-practices/SKILL.md @@ -5,7 +5,7 @@ description: Client-side React performance optimization patterns. # React Best Practices -Client-side React optimization patterns for Cloud UI. +Client-side React optimization patterns for Cloud UI. 29 rules organized by category and impact. ## Activation Conditions @@ -14,6 +14,63 @@ Client-side React optimization patterns for Cloud UI. - Bundle size concerns - useEffect/useMemo patterns -## Rules +## Rules by Category -See `rules/` directory for detailed guidance. +### Bundle Optimization (CRITICAL) +| Rule | Key Point | +|------|-----------| +| `bundle-barrel-imports` | Import from source files, not barrel `index.ts` | +| `bundle-code-splitting` | `React.lazy` + `Suspense` for heavy components, conditional loads, deferred 3rd-party | +| `bundle-preload` | Preload critical resources | + +### Re-render Prevention (HIGH) +| Rule | Key Point | +|------|-----------| +| `rerender-memo` | Memoize expensive computations | +| `rerender-dependencies` | Minimize hook dependency arrays | +| `rerender-derived-state` | Compute during render, not in effects | +| `rerender-functional-setstate` | `setState(prev => ...)` to avoid stale closures | +| `rerender-lazy-state-init` | `useState(() => expensive())` not `useState(expensive())` | +| `rerender-simple-expression-in-memo` | Don't memoize trivial expressions | +| `rerender-transitions` | `useTransition` for non-urgent updates | +| `rerender-defer-reads` | Read URL params / storage inside callbacks, not at render | + +### Rendering (MEDIUM) +| Rule | Key Point | +|------|-----------| +| `rendering-conditional-render` | Short-circuit with `&&` / ternary, avoid render in effects | +| `rendering-hoist-jsx` | Move static JSX outside components | +| `rendering-content-visibility` | CSS `content-visibility: auto` for off-screen | +| `rendering-activity` | React `` for hidden UI | +| `rendering-animate-svg-wrapper` | Wrap animated SVGs to isolate re-renders | +| `rendering-svg-precision` | Limit SVG coordinate precision | + +### Async (MEDIUM) +| Rule | Key Point | +|------|-----------| +| `async-suspense-boundaries` | Granular `` boundaries | +| `async-defer-await` | Don't await non-blocking work | +| `async-dependencies` | Load deps in parallel, not sequentially | + +### JavaScript (MEDIUM) +| Rule | Key Point | +|------|-----------| +| `js-caching-patterns` | Module-level Map caches for repeated calls + storage reads | +| `js-batch-dom-css` | Batch DOM reads/writes to avoid layout thrashing | +| `js-index-maps` | Pre-index arrays into Maps for O(1) lookups | +| `js-length-check-first` | Check `.length` before expensive operations | +| `js-tosorted-immutable` | Use `.toSorted()` / `.toReversed()` for immutable transforms | + +RegExp hoisting is enforced by Biome (`useTopLevelRegex`). + +### Browser (MEDIUM) +| Rule | Key Point | +|------|-----------| +| `client-event-listeners` | Clean up event listeners in useEffect return | +| `client-passive-event-listeners` | `{ passive: true }` for scroll/touch handlers | + +### Advanced (LOW) +| Rule | Key Point | +|------|-----------| +| `advanced-event-handler-refs` | Stable callback refs to avoid child re-renders | +| `advanced-use-latest` | `useLatest` pattern for always-current values in callbacks | diff --git a/frontend/.claude/skills/react-best-practices/rules/async-parallel.md b/frontend/.claude/skills/react-best-practices/rules/async-parallel.md deleted file mode 100644 index 64133f6c31..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/async-parallel.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -title: Promise.all() for Independent Operations -impact: CRITICAL -impactDescription: 2-10× improvement -tags: async, parallelization, promises, waterfalls ---- - -## Promise.all() for Independent Operations - -When async operations have no interdependencies, execute them concurrently using `Promise.all()`. - -**Incorrect (sequential execution, 3 round trips):** - -```typescript -const user = await fetchUser() -const posts = await fetchPosts() -const comments = await fetchComments() -``` - -**Correct (parallel execution, 1 round trip):** - -```typescript -const [user, posts, comments] = await Promise.all([ - fetchUser(), - fetchPosts(), - fetchComments() -]) -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/bundle-barrel-imports.md b/frontend/.claude/skills/react-best-practices/rules/bundle-barrel-imports.md index ee48f3273c..9c989a5ca0 100644 --- a/frontend/.claude/skills/react-best-practices/rules/bundle-barrel-imports.md +++ b/frontend/.claude/skills/react-best-practices/rules/bundle-barrel-imports.md @@ -37,21 +37,6 @@ import TextField from '@mui/material/TextField' // Loads only what you use ``` -**Alternative (Next.js 13.5+):** - -```js -// next.config.js - use optimizePackageImports -module.exports = { - experimental: { - optimizePackageImports: ['lucide-react', '@mui/material'] - } -} - -// Then you can keep the ergonomic barrel imports: -import { Check, X, Menu } from 'lucide-react' -// Automatically transformed to direct imports at build time -``` - Direct imports provide 15-70% faster dev boot, 28% faster builds, 40% faster cold starts, and significantly faster HMR. Libraries commonly affected: `lucide-react`, `@mui/material`, `@mui/icons-material`, `@tabler/icons-react`, `react-icons`, `@headlessui/react`, `@radix-ui/react-*`, `lodash`, `ramda`, `date-fns`, `rxjs`, `react-use`. diff --git a/frontend/.claude/skills/react-best-practices/rules/bundle-code-splitting.md b/frontend/.claude/skills/react-best-practices/rules/bundle-code-splitting.md new file mode 100644 index 0000000000..640450a6e0 --- /dev/null +++ b/frontend/.claude/skills/react-best-practices/rules/bundle-code-splitting.md @@ -0,0 +1,78 @@ +--- +title: Code Splitting and Lazy Loading +impact: CRITICAL +impactDescription: directly affects TTI and LCP +tags: bundle, dynamic-import, code-splitting, react-lazy, conditional-loading +--- + +## Code Splitting and Lazy Loading + +Use `React.lazy` with `Suspense` to lazy-load large components not needed on initial render. Load large data modules conditionally. Defer non-critical third-party libraries. + +### Dynamic Imports for Heavy Components + +**Incorrect (Monaco bundles with main chunk ~300KB):** + +```tsx +import { MonacoEditor } from './monaco-editor' +``` + +**Correct (Monaco loads on demand):** + +```tsx +import { lazy, Suspense } from 'react' + +const MonacoEditor = lazy(() => + import('./monaco-editor').then(m => ({ default: m.MonacoEditor })) +) + +function CodePanel({ code }: { code: string }) { + return ( + Loading editor...}> + + + ) +} +``` + +### Conditional Module Loading + +Load large data or modules only when a feature is activated: + +```tsx +function AnimationPlayer({ enabled, setEnabled }: Props) { + const [frames, setFrames] = useState(null) + + useEffect(() => { + if (enabled && !frames) { + import('./animation-frames.js') + .then(mod => setFrames(mod.frames)) + .catch(() => setEnabled(false)) + } + }, [enabled, frames, setEnabled]) + + if (!frames) return + return +} +``` + +### Defer Non-Critical Third-Party Libraries + +Analytics, logging, and error tracking don't block user interaction: + +```tsx +const Analytics = lazy(() => + import('@some-vendor/analytics').then(m => ({ default: m.Analytics })) +) + +export function App({ children }) { + return ( +
+ {children} + + + +
+ ) +} +``` diff --git a/frontend/.claude/skills/react-best-practices/rules/bundle-conditional.md b/frontend/.claude/skills/react-best-practices/rules/bundle-conditional.md deleted file mode 100644 index 99d6fc90e3..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/bundle-conditional.md +++ /dev/null @@ -1,31 +0,0 @@ ---- -title: Conditional Module Loading -impact: HIGH -impactDescription: loads large data only when needed -tags: bundle, conditional-loading, lazy-loading ---- - -## Conditional Module Loading - -Load large data or modules only when a feature is activated. - -**Example (lazy-load animation frames):** - -```tsx -function AnimationPlayer({ enabled, setEnabled }: { enabled: boolean; setEnabled: React.Dispatch> }) { - const [frames, setFrames] = useState(null) - - useEffect(() => { - if (enabled && !frames && typeof window !== 'undefined') { - import('./animation-frames.js') - .then(mod => setFrames(mod.frames)) - .catch(() => setEnabled(false)) - } - }, [enabled, frames, setEnabled]) - - if (!frames) return - return -} -``` - -The `typeof window !== 'undefined'` check prevents bundling this module for SSR, optimizing server bundle size and build speed. diff --git a/frontend/.claude/skills/react-best-practices/rules/bundle-defer-third-party.md b/frontend/.claude/skills/react-best-practices/rules/bundle-defer-third-party.md deleted file mode 100644 index 5b4fa2e49f..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/bundle-defer-third-party.md +++ /dev/null @@ -1,46 +0,0 @@ ---- -title: Defer Non-Critical Third-Party Libraries -impact: MEDIUM -impactDescription: loads after initial render -tags: bundle, third-party, analytics, defer ---- - -## Defer Non-Critical Third-Party Libraries - -Analytics, logging, and error tracking don't block user interaction. Load them after initial render. - -**Incorrect (blocks initial bundle):** - -```tsx -import { Analytics } from '@some-vendor/analytics' - -export function App({ children }) { - return ( -
- {children} - -
- ) -} -``` - -**Correct (loads after initial render):** - -```tsx -import { lazy, Suspense } from 'react' - -const Analytics = lazy(() => - import('@some-vendor/analytics').then(m => ({ default: m.Analytics })) -) - -export function App({ children }) { - return ( -
- {children} - - - -
- ) -} -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/bundle-dynamic-imports.md b/frontend/.claude/skills/react-best-practices/rules/bundle-dynamic-imports.md deleted file mode 100644 index b3d5395c86..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/bundle-dynamic-imports.md +++ /dev/null @@ -1,38 +0,0 @@ ---- -title: Dynamic Imports for Heavy Components -impact: CRITICAL -impactDescription: directly affects TTI and LCP -tags: bundle, dynamic-import, code-splitting, react-lazy ---- - -## Dynamic Imports for Heavy Components - -Use `React.lazy` with `Suspense` to lazy-load large components not needed on initial render. - -**Incorrect (Monaco bundles with main chunk ~300KB):** - -```tsx -import { MonacoEditor } from './monaco-editor' - -function CodePanel({ code }: { code: string }) { - return -} -``` - -**Correct (Monaco loads on demand):** - -```tsx -import { lazy, Suspense } from 'react' - -const MonacoEditor = lazy(() => - import('./monaco-editor').then(m => ({ default: m.MonacoEditor })) -) - -function CodePanel({ code }: { code: string }) { - return ( - Loading editor...}> - - - ) -} -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/client-localstorage-schema.md b/frontend/.claude/skills/react-best-practices/rules/client-localstorage-schema.md deleted file mode 100644 index d30a1a7d4f..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/client-localstorage-schema.md +++ /dev/null @@ -1,71 +0,0 @@ ---- -title: Version and Minimize localStorage Data -impact: MEDIUM -impactDescription: prevents schema conflicts, reduces storage size -tags: client, localStorage, storage, versioning, data-minimization ---- - -## Version and Minimize localStorage Data - -Add version prefix to keys and store only needed fields. Prevents schema conflicts and accidental storage of sensitive data. - -**Incorrect:** - -```typescript -// No version, stores everything, no error handling -localStorage.setItem('userConfig', JSON.stringify(fullUserObject)) -const data = localStorage.getItem('userConfig') -``` - -**Correct:** - -```typescript -const VERSION = 'v2' - -function saveConfig(config: { theme: string; language: string }) { - try { - localStorage.setItem(`userConfig:${VERSION}`, JSON.stringify(config)) - } catch { - // Throws in incognito/private browsing, quota exceeded, or disabled - } -} - -function loadConfig() { - try { - const data = localStorage.getItem(`userConfig:${VERSION}`) - return data ? JSON.parse(data) : null - } catch { - return null - } -} - -// Migration from v1 to v2 -function migrate() { - try { - const v1 = localStorage.getItem('userConfig:v1') - if (v1) { - const old = JSON.parse(v1) - saveConfig({ theme: old.darkMode ? 'dark' : 'light', language: old.lang }) - localStorage.removeItem('userConfig:v1') - } - } catch {} -} -``` - -**Store minimal fields from server responses:** - -```typescript -// User object has 20+ fields, only store what UI needs -function cachePrefs(user: FullUser) { - try { - localStorage.setItem('prefs:v1', JSON.stringify({ - theme: user.preferences.theme, - notifications: user.preferences.notifications - })) - } catch {} -} -``` - -**Always wrap in try-catch:** `getItem()` and `setItem()` throw in incognito/private browsing (Safari, Firefox), when quota exceeded, or when disabled. - -**Benefits:** Schema evolution via versioning, reduced storage size, prevents storing tokens/PII/internal flags. diff --git a/frontend/.claude/skills/react-best-practices/rules/js-cache-function-results.md b/frontend/.claude/skills/react-best-practices/rules/js-cache-function-results.md deleted file mode 100644 index 180f8ac8ff..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/js-cache-function-results.md +++ /dev/null @@ -1,80 +0,0 @@ ---- -title: Cache Repeated Function Calls -impact: MEDIUM -impactDescription: avoid redundant computation -tags: javascript, cache, memoization, performance ---- - -## Cache Repeated Function Calls - -Use a module-level Map to cache function results when the same function is called repeatedly with the same inputs during render. - -**Incorrect (redundant computation):** - -```typescript -function ProjectList({ projects }: { projects: Project[] }) { - return ( -
- {projects.map(project => { - // slugify() called 100+ times for same project names - const slug = slugify(project.name) - - return - })} -
- ) -} -``` - -**Correct (cached results):** - -```typescript -// Module-level cache -const slugifyCache = new Map() - -function cachedSlugify(text: string): string { - if (slugifyCache.has(text)) { - return slugifyCache.get(text)! - } - const result = slugify(text) - slugifyCache.set(text, result) - return result -} - -function ProjectList({ projects }: { projects: Project[] }) { - return ( -
- {projects.map(project => { - // Computed only once per unique project name - const slug = cachedSlugify(project.name) - - return - })} -
- ) -} -``` - -**Simpler pattern for single-value functions:** - -```typescript -let isLoggedInCache: boolean | null = null - -function isLoggedIn(): boolean { - if (isLoggedInCache !== null) { - return isLoggedInCache - } - - isLoggedInCache = document.cookie.includes('auth=') - return isLoggedInCache -} - -// Clear cache when auth changes -function onAuthChange() { - isLoggedInCache = null -} -``` - -Use a Map (not a hook) so it works everywhere: utilities, event handlers, not just React components. - -Reference: [How we made the Vercel Dashboard twice as fast](https://vercel.com/blog/how-we-made-the-vercel-dashboard-twice-as-fast) diff --git a/frontend/.claude/skills/react-best-practices/rules/js-cache-property-access.md b/frontend/.claude/skills/react-best-practices/rules/js-cache-property-access.md deleted file mode 100644 index 39eec9061b..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/js-cache-property-access.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -title: Cache Property Access in Loops -impact: LOW-MEDIUM -impactDescription: reduces lookups -tags: javascript, loops, optimization, caching ---- - -## Cache Property Access in Loops - -Cache object property lookups in hot paths. - -**Incorrect (3 lookups × N iterations):** - -```typescript -for (let i = 0; i < arr.length; i++) { - process(obj.config.settings.value) -} -``` - -**Correct (1 lookup total):** - -```typescript -const value = obj.config.settings.value -const len = arr.length -for (let i = 0; i < len; i++) { - process(value) -} -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/js-cache-storage.md b/frontend/.claude/skills/react-best-practices/rules/js-cache-storage.md deleted file mode 100644 index aa4a30c081..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/js-cache-storage.md +++ /dev/null @@ -1,70 +0,0 @@ ---- -title: Cache Storage API Calls -impact: LOW-MEDIUM -impactDescription: reduces expensive I/O -tags: javascript, localStorage, storage, caching, performance ---- - -## Cache Storage API Calls - -`localStorage`, `sessionStorage`, and `document.cookie` are synchronous and expensive. Cache reads in memory. - -**Incorrect (reads storage on every call):** - -```typescript -function getTheme() { - return localStorage.getItem('theme') ?? 'light' -} -// Called 10 times = 10 storage reads -``` - -**Correct (Map cache):** - -```typescript -const storageCache = new Map() - -function getLocalStorage(key: string) { - if (!storageCache.has(key)) { - storageCache.set(key, localStorage.getItem(key)) - } - return storageCache.get(key) -} - -function setLocalStorage(key: string, value: string) { - localStorage.setItem(key, value) - storageCache.set(key, value) // keep cache in sync -} -``` - -Use a Map (not a hook) so it works everywhere: utilities, event handlers, not just React components. - -**Cookie caching:** - -```typescript -let cookieCache: Record | null = null - -function getCookie(name: string) { - if (!cookieCache) { - cookieCache = Object.fromEntries( - document.cookie.split('; ').map(c => c.split('=')) - ) - } - return cookieCache[name] -} -``` - -**Important (invalidate on external changes):** - -If storage can change externally (another tab, server-set cookies), invalidate cache: - -```typescript -window.addEventListener('storage', (e) => { - if (e.key) storageCache.delete(e.key) -}) - -document.addEventListener('visibilitychange', () => { - if (document.visibilityState === 'visible') { - storageCache.clear() - } -}) -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/js-caching-patterns.md b/frontend/.claude/skills/react-best-practices/rules/js-caching-patterns.md new file mode 100644 index 0000000000..204bc9bb55 --- /dev/null +++ b/frontend/.claude/skills/react-best-practices/rules/js-caching-patterns.md @@ -0,0 +1,75 @@ +--- +title: Client-Side Caching Patterns +impact: MEDIUM +impactDescription: reduces redundant computation and expensive I/O +tags: javascript, cache, memoization, localStorage, storage, performance +--- + +## Client-Side Caching Patterns + +### Cache Repeated Function Calls + +Use a module-level Map to cache function results when the same function is called repeatedly with the same inputs. + +```typescript +const slugifyCache = new Map() + +function cachedSlugify(text: string): string { + if (slugifyCache.has(text)) return slugifyCache.get(text)! + const result = slugify(text) + slugifyCache.set(text, result) + return result +} +``` + +Use a Map (not a hook) so it works everywhere: utilities, event handlers, not just React components. + +### Cache Storage API Calls + +`localStorage`, `sessionStorage`, and `document.cookie` are synchronous and expensive. Cache reads in memory: + +```typescript +const storageCache = new Map() + +function getLocalStorage(key: string) { + if (!storageCache.has(key)) { + storageCache.set(key, localStorage.getItem(key)) + } + return storageCache.get(key) +} + +function setLocalStorage(key: string, value: string) { + localStorage.setItem(key, value) + storageCache.set(key, value) +} +``` + +Invalidate on external changes: + +```typescript +window.addEventListener('storage', (e) => { + if (e.key) storageCache.delete(e.key) +}) +``` + +### Version localStorage Data + +Add version prefix to keys and store only needed fields: + +```typescript +const VERSION = 'v2' + +function saveConfig(config: { theme: string; language: string }) { + try { + localStorage.setItem(`userConfig:${VERSION}`, JSON.stringify(config)) + } catch { + // Throws in incognito, quota exceeded, or disabled + } +} +``` + +Always wrap `getItem()`/`setItem()` in try-catch — they throw in incognito/private browsing (Safari, Firefox). + +## Reference + +- [How we made the Vercel Dashboard twice as fast](https://vercel.com/blog/how-we-made-the-vercel-dashboard-twice-as-fast) diff --git a/frontend/.claude/skills/react-best-practices/rules/js-combine-iterations.md b/frontend/.claude/skills/react-best-practices/rules/js-combine-iterations.md deleted file mode 100644 index 044d017ec3..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/js-combine-iterations.md +++ /dev/null @@ -1,32 +0,0 @@ ---- -title: Combine Multiple Array Iterations -impact: LOW-MEDIUM -impactDescription: reduces iterations -tags: javascript, arrays, loops, performance ---- - -## Combine Multiple Array Iterations - -Multiple `.filter()` or `.map()` calls iterate the array multiple times. Combine into one loop. - -**Incorrect (3 iterations):** - -```typescript -const admins = users.filter(u => u.isAdmin) -const testers = users.filter(u => u.isTester) -const inactive = users.filter(u => !u.isActive) -``` - -**Correct (1 iteration):** - -```typescript -const admins: User[] = [] -const testers: User[] = [] -const inactive: User[] = [] - -for (const user of users) { - if (user.isAdmin) admins.push(user) - if (user.isTester) testers.push(user) - if (!user.isActive) inactive.push(user) -} -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/js-early-exit.md b/frontend/.claude/skills/react-best-practices/rules/js-early-exit.md deleted file mode 100644 index f46cb89c6c..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/js-early-exit.md +++ /dev/null @@ -1,50 +0,0 @@ ---- -title: Early Return from Functions -impact: LOW-MEDIUM -impactDescription: avoids unnecessary computation -tags: javascript, functions, optimization, early-return ---- - -## Early Return from Functions - -Return early when result is determined to skip unnecessary processing. - -**Incorrect (processes all items even after finding answer):** - -```typescript -function validateUsers(users: User[]) { - let hasError = false - let errorMessage = '' - - for (const user of users) { - if (!user.email) { - hasError = true - errorMessage = 'Email required' - } - if (!user.name) { - hasError = true - errorMessage = 'Name required' - } - // Continues checking all users even after error found - } - - return hasError ? { valid: false, error: errorMessage } : { valid: true } -} -``` - -**Correct (returns immediately on first error):** - -```typescript -function validateUsers(users: User[]) { - for (const user of users) { - if (!user.email) { - return { valid: false, error: 'Email required' } - } - if (!user.name) { - return { valid: false, error: 'Name required' } - } - } - - return { valid: true } -} -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/js-hoist-regexp.md b/frontend/.claude/skills/react-best-practices/rules/js-hoist-regexp.md deleted file mode 100644 index dae3fefdc0..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/js-hoist-regexp.md +++ /dev/null @@ -1,45 +0,0 @@ ---- -title: Hoist RegExp Creation -impact: LOW-MEDIUM -impactDescription: avoids recreation -tags: javascript, regexp, optimization, memoization ---- - -## Hoist RegExp Creation - -Don't create RegExp inside render. Hoist to module scope or memoize with `useMemo()`. - -**Incorrect (new RegExp every render):** - -```tsx -function Highlighter({ text, query }: Props) { - const regex = new RegExp(`(${query})`, 'gi') - const parts = text.split(regex) - return <>{parts.map((part, i) => ...)} -} -``` - -**Correct (memoize or hoist):** - -```tsx -const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ - -function Highlighter({ text, query }: Props) { - const regex = useMemo( - () => new RegExp(`(${escapeRegex(query)})`, 'gi'), - [query] - ) - const parts = text.split(regex) - return <>{parts.map((part, i) => ...)} -} -``` - -**Warning (global regex has mutable state):** - -Global regex (`/g`) has mutable `lastIndex` state: - -```typescript -const regex = /foo/g -regex.test('foo') // true, lastIndex = 3 -regex.test('foo') // false, lastIndex = 0 -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/js-min-max-loop.md b/frontend/.claude/skills/react-best-practices/rules/js-min-max-loop.md deleted file mode 100644 index 4b6656e96a..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/js-min-max-loop.md +++ /dev/null @@ -1,82 +0,0 @@ ---- -title: Use Loop for Min/Max Instead of Sort -impact: LOW -impactDescription: O(n) instead of O(n log n) -tags: javascript, arrays, performance, sorting, algorithms ---- - -## Use Loop for Min/Max Instead of Sort - -Finding the smallest or largest element only requires a single pass through the array. Sorting is wasteful and slower. - -**Incorrect (O(n log n) - sort to find latest):** - -```typescript -interface Project { - id: string - name: string - updatedAt: number -} - -function getLatestProject(projects: Project[]) { - const sorted = [...projects].sort((a, b) => b.updatedAt - a.updatedAt) - return sorted[0] -} -``` - -Sorts the entire array just to find the maximum value. - -**Incorrect (O(n log n) - sort for oldest and newest):** - -```typescript -function getOldestAndNewest(projects: Project[]) { - const sorted = [...projects].sort((a, b) => a.updatedAt - b.updatedAt) - return { oldest: sorted[0], newest: sorted[sorted.length - 1] } -} -``` - -Still sorts unnecessarily when only min/max are needed. - -**Correct (O(n) - single loop):** - -```typescript -function getLatestProject(projects: Project[]) { - if (projects.length === 0) return null - - let latest = projects[0] - - for (let i = 1; i < projects.length; i++) { - if (projects[i].updatedAt > latest.updatedAt) { - latest = projects[i] - } - } - - return latest -} - -function getOldestAndNewest(projects: Project[]) { - if (projects.length === 0) return { oldest: null, newest: null } - - let oldest = projects[0] - let newest = projects[0] - - for (let i = 1; i < projects.length; i++) { - if (projects[i].updatedAt < oldest.updatedAt) oldest = projects[i] - if (projects[i].updatedAt > newest.updatedAt) newest = projects[i] - } - - return { oldest, newest } -} -``` - -Single pass through the array, no copying, no sorting. - -**Alternative (Math.min/Math.max for small arrays):** - -```typescript -const numbers = [5, 2, 8, 1, 9] -const min = Math.min(...numbers) -const max = Math.max(...numbers) -``` - -This works for small arrays, but can be slower or just throw an error for very large arrays due to spread operator limitations. Maximal array length is approximately 124000 in Chrome 143 and 638000 in Safari 18; exact numbers may vary - see [the fiddle](https://jsfiddle.net/qw1jabsx/4/). Use the loop approach for reliability. diff --git a/frontend/.claude/skills/react-best-practices/rules/js-set-map-lookups.md b/frontend/.claude/skills/react-best-practices/rules/js-set-map-lookups.md deleted file mode 100644 index 680a4892ee..0000000000 --- a/frontend/.claude/skills/react-best-practices/rules/js-set-map-lookups.md +++ /dev/null @@ -1,24 +0,0 @@ ---- -title: Use Set/Map for O(1) Lookups -impact: LOW-MEDIUM -impactDescription: O(n) to O(1) -tags: javascript, set, map, data-structures, performance ---- - -## Use Set/Map for O(1) Lookups - -Convert arrays to Set/Map for repeated membership checks. - -**Incorrect (O(n) per check):** - -```typescript -const allowedIds = ['a', 'b', 'c', ...] -items.filter(item => allowedIds.includes(item.id)) -``` - -**Correct (O(1) per check):** - -```typescript -const allowedIds = new Set(['a', 'b', 'c', ...]) -items.filter(item => allowedIds.has(item.id)) -``` diff --git a/frontend/.claude/skills/react-best-practices/rules/rerender-defer-reads.md b/frontend/.claude/skills/react-best-practices/rules/rerender-defer-reads.md index e867c95f02..acba5d6f35 100644 --- a/frontend/.claude/skills/react-best-practices/rules/rerender-defer-reads.md +++ b/frontend/.claude/skills/react-best-practices/rules/rerender-defer-reads.md @@ -2,21 +2,21 @@ title: Defer State Reads to Usage Point impact: MEDIUM impactDescription: avoids unnecessary subscriptions -tags: rerender, searchParams, localStorage, optimization +tags: rerender, URLSearchParams, localStorage, optimization --- ## Defer State Reads to Usage Point -Don't subscribe to dynamic state (searchParams, localStorage) if you only read it inside callbacks. +Don't subscribe to dynamic state (URL params, localStorage) if you only read it inside callbacks. -**Incorrect (subscribes to all searchParams changes):** +**Incorrect (subscribes to URL param changes, re-renders on every change):** ```tsx function ShareButton({ chatId }: { chatId: string }) { - const searchParams = useSearchParams() + // Reading URL params at render time creates a subscription + const ref = new URLSearchParams(window.location.search).get('ref') const handleShare = () => { - const ref = searchParams.get('ref') shareChat(chatId, { ref }) } @@ -24,7 +24,7 @@ function ShareButton({ chatId }: { chatId: string }) { } ``` -**Correct (reads on demand, no subscription):** +**Correct (reads on demand inside callback, no subscription):** ```tsx function ShareButton({ chatId }: { chatId: string }) { @@ -37,3 +37,5 @@ function ShareButton({ chatId }: { chatId: string }) { return } ``` + +Same principle applies to localStorage, sessionStorage, and cookies — read inside callbacks rather than at render time. diff --git a/frontend/.claude/skills/state-management/rules/use-react-query-for-server.md b/frontend/.claude/skills/state-management/rules/use-react-query-for-server.md index 54d25bfe35..a4d8bcd3f3 100644 --- a/frontend/.claude/skills/state-management/rules/use-react-query-for-server.md +++ b/frontend/.claude/skills/state-management/rules/use-react-query-for-server.md @@ -81,3 +81,4 @@ function ClusterList() { - [TanStack Query Overview](https://tanstack.com/query/latest/docs/framework/react/overview) - [Connect Query Docs](https://connectrpc.com/docs/web/query/getting-started) +- See also: [use-connect-query](../../api-patterns/rules/use-connect-query.md) — Connect Query hook patterns diff --git a/frontend/.claude/skills/state-management/rules/use-zustand-persist.md b/frontend/.claude/skills/state-management/rules/use-zustand-persist.md index 078c76658d..187e8677d0 100644 --- a/frontend/.claude/skills/state-management/rules/use-zustand-persist.md +++ b/frontend/.claude/skills/state-management/rules/use-zustand-persist.md @@ -9,7 +9,7 @@ tags: zustand, persist, sessionStorage, middleware ## Explanation -Use Zustand's persist middleware with sessionStorage for state that should survive page navigation but not browser sessions. This is useful for wizard data, user preferences within a session, and cross-component state. +Use Zustand's persist middleware with sessionStorage for state that should survive page navigation but not browser sessions. Use `createFlatStorage` from `src/utils/store.ts` as the storage adapter. ## Incorrect @@ -38,7 +38,7 @@ const useStore = create()( ```tsx import { create } from 'zustand'; import { persist } from 'zustand/middleware'; -import { createPersistedZustandSessionStorage } from './utils'; +import { createFlatStorage } from 'utils/store'; type WizardData = { step: number; @@ -59,7 +59,7 @@ const useWizardStore = create()( }), { name: 'wizard-state', - storage: createPersistedZustandSessionStorage< + storage: createFlatStorage< Omit >(), } @@ -76,4 +76,5 @@ Always include a reset function for: ## Reference +- `src/utils/store.ts` — `createFlatStorage` implementation (uses sessionStorage) - [Zustand Persist Middleware](https://docs.pmnd.rs/zustand/integrations/persisting-store-data) diff --git a/frontend/.claude/skills/ui-development/rules/use-ui-registry.md b/frontend/.claude/skills/ui-development/rules/use-ui-registry.md index 2c4ad7a047..385de8de4d 100644 --- a/frontend/.claude/skills/ui-development/rules/use-ui-registry.md +++ b/frontend/.claude/skills/ui-development/rules/use-ui-registry.md @@ -56,3 +56,4 @@ yes | bunx @fumadocs/cli add --dir https://redpanda-ui-registry.netlify.app/r bu - https://redpanda-ui-registry.netlify.app - MCP tools: `mcp__redpanda-ui__search-docs`, `mcp__redpanda-ui__get_component` +- See also: [no-legacy](../../code-standards/rules/no-legacy.md) — full list of prohibited legacy patterns diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index 1cc837db28..aee61aa087 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -2,6 +2,12 @@ React 18.3 · Bun · Rsbuild +## Critical Rules + +- New code MUST use Registry components (`src/components/redpanda-ui/`), react-hook-form + Zod, Vitest, and Connect Query +- See [no-legacy](.claude/skills/code-standards/rules/no-legacy.md) for prohibited patterns +- `src/protogen/` is generated — DO NOT EDIT + ## Commands | Command | Purpose | @@ -14,18 +20,9 @@ React 18.3 · Bun · Rsbuild | `bun run lint` | Biome linter | | `bun run type:check` | TypeScript | -## Critical Rules - -### Legacy Patterns +## Verify Changes -See [no-legacy](.claude/skills/code-standards/rules/no-legacy.md) for prohibited patterns. - -### ALWAYS Use (Modern) - -- Registry: `src/components/redpanda-ui/` -- react-hook-form + Zod -- Vitest, Playwright -- Connect Query +After changes: `bun run type:check && bun run lint && bun run test` ## Directory Structure @@ -33,53 +30,18 @@ See [no-legacy](.claude/skills/code-standards/rules/no-legacy.md) for prohibited src/ ├── components/ │ ├── pages/ # Feature pages -│ └── redpanda-ui/ # Modern UI (USE THIS) +│ └── redpanda-ui/ # UI Registry (USE THIS) ├── react-query/ # Connect Query hooks ├── hooks/ # Custom hooks ├── utils/ # Utilities └── protogen/ # Generated (DO NOT EDIT) ``` -## Quick Patterns - -**Query:** -```typescript -useQuery(getResource, request, { enabled: !!id }) -``` - -**Mutation:** -```typescript -useMutation(updateResource, { onSuccess: invalidate }) -``` - -**Store:** -```typescript -const v = useStore((s) => s.value) // always use selectors -``` - -**Tests:** -- `.test.ts` = unit (node) -- `.test.tsx` = integration (jsdom) - -## On-Demand Skills - -Load skill when task matches trigger: +## Plan Mode -| Task Type | Skill | -|-----------|-------| -| Tests (unit/integration) | [testing](.claude/skills/testing/SKILL.md) | -| E2E / Playwright | [e2e-tester](.claude/skills/e2e-tester/SKILL.md) | -| UI components | [ui-development](.claude/skills/ui-development/SKILL.md) | -| Forms | [form-refactorer](.claude/skills/form-refactorer/SKILL.md) | -| API calls | [api-patterns](.claude/skills/api-patterns/SKILL.md) | -| Global state | [state-management](.claude/skills/state-management/SKILL.md) | -| Performance | [react-best-practices](.claude/skills/react-best-practices/SKILL.md) | -| Linting | [code-standards](.claude/skills/code-standards/SKILL.md) | -| Security | [security-scan](.claude/skills/security-scan/SKILL.md) | -| Router migration | [tanstack-router-migration](.claude/skills/tanstack-router-migration/SKILL.md) | -| Design review | [web-design-guidelines](.claude/skills/web-design-guidelines/SKILL.md) | +- Extremely concise plans. Sacrifice grammar for concision. +- End each plan with unresolved questions, if any. -## Plan Mode +## Compaction -- Make the plan extremely concise. Sacrifice grammar for the sake of concision. -- At the end of each plan, give me a list of unresolved questions to answer, if any. +When compacting, preserve: modified file list, test commands, and error messages. diff --git a/frontend/biome.jsonc b/frontend/biome.jsonc index e0a8fe17dd..4c4b39ecdc 100644 --- a/frontend/biome.jsonc +++ b/frontend/biome.jsonc @@ -73,6 +73,18 @@ }, "style": { "noDefaultExport": "off", + "noRestrictedImports": { + "level": "warn", + "options": { + "paths": { + "@redpanda-data/ui": "Deprecated — use components from 'components/redpanda-ui/' instead", + "@chakra-ui/react": "Deprecated — use Registry components + Tailwind instead", + "mobx": "Deprecated — use Zustand instead", + "mobx-react-lite": "Deprecated — use Zustand instead", + "yup": "Deprecated — use Zod instead" + } + } + }, "useConsistentTypeDefinitions": { "level": "error", "options": {