From 914a859b9494a829e66f6a03bf24af5bbb3f7c53 Mon Sep 17 00:00:00 2001 From: notque Date: Sun, 12 Apr 2026 00:11:28 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat(refs):=20react-native-engineer=20?= =?UTF-8?q?=E2=80=94=20testing=20and=20error-handling=20references=20(Leve?= =?UTF-8?q?l=202=E2=86=923)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two reference files covering gaps identified by audit: - testing.md: RNTL 12+ patterns, jest-expo config, native module mocking, async assertion patterns, anti-patterns (testID queries, snapshots, per-file navigation mocks), error-fix mappings with detection commands - error-handling.md: ErrorBoundary setup, Sentry init sequence, unhandled promise rejection capture, typed fetch error handling, anti-patterns (empty catch, console.error-only, missing boundaries), detection commands Update agent loading table and error handling section to route to new files. Detection commands in both files push aggregate level from 2 to 3. --- agents/react-native-engineer.md | 8 + .../references/error-handling.md | 332 ++++++++++++++++++ .../references/testing.md | 271 ++++++++++++++ 3 files changed, 611 insertions(+) create mode 100644 agents/react-native-engineer/references/error-handling.md create mode 100644 agents/react-native-engineer/references/testing.md diff --git a/agents/react-native-engineer.md b/agents/react-native-engineer.md index 55a41845..249a17ce 100644 --- a/agents/react-native-engineer.md +++ b/agents/react-native-engineer.md @@ -80,6 +80,8 @@ Do not load references for domains not relevant to the task — context is a sca | useState, derived state, Zustand, state structure, dispatchers, ground truth | `state-management.md` | | Conditional rendering, &&, Text components, React Compiler, memoization | `rendering-patterns.md` | | Monorepo, fonts, imports, design system, dependency versions, autolinking | `monorepo-config.md` | +| Tests, RNTL, jest, Maestro, Detox, native module mocking, waitFor, snapshot | `testing.md` | +| Error boundaries, Sentry, crash recovery, unhandled rejection, try/catch, fetch errors | `error-handling.md` | ## Error Handling @@ -93,6 +95,10 @@ Do not load references for domains not relevant to the task — context is a sca **State sync issues**: Load `state-management.md` — stale closure or redundant derived state. +**Production crashes, Error Boundaries, Sentry, unhandled rejections**: Load `error-handling.md` — error boundary setup, crash reporting patterns, fetch error handling. + +**Test setup, RNTL queries, native module mocks, async assertions**: Load `testing.md` — RNTL patterns, jest config, native mock setup, anti-patterns. + ## References - [list-performance.md](react-native-engineer/references/list-performance.md) — FlashList/LegendList, memoization, virtualization, stable references @@ -102,3 +108,5 @@ Do not load references for domains not relevant to the task — context is a sca - [state-management.md](react-native-engineer/references/state-management.md) — Minimal state, dispatch updaters, fallback patterns, ground truth - [rendering-patterns.md](react-native-engineer/references/rendering-patterns.md) — Falsy && crash prevention, Text components, React Compiler - [monorepo-config.md](react-native-engineer/references/monorepo-config.md) — Fonts, imports, native dep autolinking, dependency versions +- [testing.md](react-native-engineer/references/testing.md) — RNTL patterns, jest config, native module mocking, async assertions, anti-patterns +- [error-handling.md](react-native-engineer/references/error-handling.md) — Error boundaries, Sentry init, unhandled rejections, fetch error handling, crash recovery diff --git a/agents/react-native-engineer/references/error-handling.md b/agents/react-native-engineer/references/error-handling.md new file mode 100644 index 00000000..5c2bdc29 --- /dev/null +++ b/agents/react-native-engineer/references/error-handling.md @@ -0,0 +1,332 @@ +# Error Handling Reference + + +> **Scope**: Production error handling in React Native: Error Boundaries, Sentry integration, promise rejection capture, and crash-safe rendering patterns. +> **Version range**: React 18+, React Native 0.72+, @sentry/react-native 5+ +> **Generated**: 2026-04-12 — verify Sentry DSN config patterns against current @sentry/react-native docs + +--- + +## Overview + +React Native apps crash hard on unhandled errors — there's no browser error overlay to recover from. A production crash closes the app. The three failure modes are: (1) synchronous render errors without an `ErrorBoundary`, (2) unhandled promise rejections that swallow failures silently, and (3) native module errors that surface as cryptic red boxes during development but silent crashes in production builds. + +--- + +## Pattern Table + +| Pattern | Version | Use When | Avoid When | +|---------|---------|----------|------------| +| `ErrorBoundary` (class component) | React 16+ | catching render-phase errors | async errors inside event handlers | +| `react-native-error-boundary` | any | quick ErrorBoundary with fallback UI | you need custom recovery logic | +| `Sentry.init()` in app entry | `@sentry/react-native 5+` | production crash reporting | local dev — noise ratio is high | +| `unhandledRejection` global handler | RN 0.68+ | catching all unhandled promise rejections | replacing proper `try/catch` per call | +| `InteractionManager.runAfterInteractions` | any | deferring error-prone work past animation frames | time-sensitive data fetching | + +--- + +## Correct Patterns + +### Wrap Screen Roots in ErrorBoundary + +Every screen-level component should be wrapped in an `ErrorBoundary`. A crash in one screen should not bring down the entire app. + +```tsx +import { ErrorBoundary } from 'react-error-boundary' + +function FeedScreen() { + return ( + ( + + Something went wrong. + + Try again + + + )} + onError={(error, info) => { + // report to Sentry or your crash service + captureException(error, { extra: { componentStack: info.componentStack } }) + }} + > + + + ) +} +``` + +**Why**: Without a boundary, any render-phase throw (null dereference, bad prop type, failed deserialization) crashes the entire React tree. The boundary catches it, renders fallback UI, and lets the user recover without restarting the app. + +--- + +### Initialize Sentry Before the React Tree + +Sentry must be initialized before `AppRegistry.registerComponent` — before any React component mounts. Errors during app startup are otherwise invisible. + +```ts +// index.js (app entry — before importing App) +import * as Sentry from '@sentry/react-native' + +Sentry.init({ + dsn: process.env.EXPO_PUBLIC_SENTRY_DSN, + environment: process.env.EXPO_PUBLIC_ENV ?? 'development', + // Sample at 10% in production to control event volume + tracesSampleRate: process.env.EXPO_PUBLIC_ENV === 'production' ? 0.1 : 1.0, + enabled: process.env.EXPO_PUBLIC_ENV !== 'development', +}) + +// Then import and register App +import { registerRootComponent } from 'expo' +import App from './App' +registerRootComponent(App) +``` + +**Why**: Errors that occur during app initialization (config load, font loading, initial navigation mount) are lost if Sentry isn't set up first. `EXPO_PUBLIC_*` variables are safe to embed in the bundle — do not use secret keys here. + +--- + +### Capture Unhandled Promise Rejections + +React Native 0.68+ surfaces unhandled rejections as yellow warnings in dev, but in production they silently swallow errors. Install a global handler. + +```ts +// App.tsx or index.js setup +const handleUnhandledRejection = (event: PromiseRejectionEvent) => { + console.error('Unhandled promise rejection:', event.reason) + captureException(event.reason, { tags: { type: 'unhandled_rejection' } }) + // Don't call event.preventDefault() — let RN's default handling also run +} + +// Node-style (Hermes engine, RN 0.64+) +if (global.HermesInternal) { + const tracking = require('promise/setimmediate/rejection-tracking') + tracking.enable({ + allRejections: true, + onUnhandled: (id: number, rejection: unknown) => { + captureException(rejection, { tags: { rejection_id: id } }) + }, + }) +} +``` + +**Why**: Fire-and-forget async calls (`fetchUser()` without `await` or `.catch()`) fail silently in production. The global handler catches them for visibility without requiring every call site to add error handling. + +--- + +### Type Fetch Errors — Never Assume the Shape + +Network errors in React Native come in three shapes: `Error` instances from `fetch` throwing on network failure, JSON parse errors when the server returns HTML (503 page), and valid JSON with an error status code. + +```ts +async function fetchUser(id: string): Promise { + let res: Response + + try { + res = await fetch(`${API_URL}/users/${id}`) + } catch (err) { + // Network failure — no response at all + throw new Error(`Network error fetching user ${id}: ${String(err)}`) + } + + if (!res.ok) { + // Server returned 4xx/5xx — body may not be JSON + const text = await res.text().catch(() => '') + throw new Error(`HTTP ${res.status} fetching user ${id}: ${text.slice(0, 200)}`) + } + + try { + return res.json() as Promise + } catch (err) { + throw new Error(`Invalid JSON for user ${id}: ${String(err)}`) + } +} +``` + +**Why**: `fetch` does NOT throw on 4xx/5xx status codes. Calling `res.json()` on a 503 HTML error page throws a parse error with a misleading message. Wrapping each phase separately gives actionable error messages in Sentry. + +--- + +## Anti-Pattern Catalog + +### ❌ Using `console.error` as the Only Error Reporting + +**Detection**: +```bash +grep -rn 'console\.error' --include="*.tsx" --include="*.ts" | grep -v "\.test\." | grep -v "\.spec\." +rg 'console\.error' --type ts --type tsx | grep -v test +``` + +**What it looks like**: +```tsx +try { + await syncData() +} catch (err) { + console.error('Sync failed', err) // invisible in production +} +``` + +**Why wrong**: `console.error` is stripped or suppressed in production builds. Errors logged this way are invisible to on-call and never trigger alerts. Crashes go undetected until users report them. + +**Fix**: +```tsx +import { captureException } from '@sentry/react-native' + +try { + await syncData() +} catch (err) { + captureException(err, { tags: { operation: 'sync' } }) + // optionally also console.error in dev + if (__DEV__) console.error('Sync failed', err) +} +``` + +--- + +### ❌ Empty Catch Blocks + +**Detection**: +```bash +grep -rn 'catch\s*(.*)\s*{\s*}' --include="*.ts" --include="*.tsx" +rg 'catch\s*\(.*\)\s*\{\s*\}' --type ts +``` + +**What it looks like**: +```ts +try { + await loadUserPreferences() +} catch (err) { + // TODO: handle this +} +``` + +**Why wrong**: Silent swallow. The error is gone. The app is now in an inconsistent state — preferences were not loaded, but no error boundary fired, no fallback rendered, no alert triggered. These are the hardest bugs to diagnose because there's no stack trace. + +**Fix**: At minimum, report and reset to a safe default: +```ts +try { + await loadUserPreferences() +} catch (err) { + captureException(err) + // explicit fallback state + await setDefaultPreferences() +} +``` + +--- + +### ❌ Missing Error Boundary at Navigation Root + +**Detection**: +```bash +grep -rn 'Stack.Screen\|Tabs.Screen' --include="*.tsx" | grep -v ErrorBoundary +rg 'NavigationContainer' --type tsx | grep -B5 -A10 'NavigationContainer' +``` + +**What it looks like**: +```tsx +export default function RootLayout() { + return ( + + + + + ) +} +``` + +**Why wrong**: If `ProfileScreen` throws during render, it unwinds the entire navigation tree. With no boundary, the app white-screens. Users must force-quit. + +**Fix**: Wrap each screen's content component in an ErrorBoundary, or add a root-level boundary around the entire navigator: +```tsx +export default function RootLayout() { + return ( + } onError={captureException}> + + + + + + ) +} +``` + +--- + +### ❌ Accessing `.data` on an Unvalidated API Response + +**Detection**: +```bash +grep -rn '\.data\.' --include="*.ts" --include="*.tsx" | grep -v "\.test\." | grep "await fetch\|axios\|useFetch" +rg '(await\s+\w+\(.*\))\.data\.' --type ts +``` + +**What it looks like**: +```ts +const response = await fetch('/api/user') +const json = await response.json() +setUser(json.data.profile.name) // throws if data or profile is undefined +``` + +**Why wrong**: API contracts break. A server returns `{ error: "not found" }` instead of `{ data: { profile: ... } }`. The chain `.data.profile.name` throws `Cannot read properties of undefined (reading 'profile')` — a crash with a misleading error message. + +**Fix**: Validate the response shape before accessing nested paths, or use optional chaining with a fallback: +```ts +const json = await response.json() +if (!json.data?.profile) { + throw new Error(`Unexpected API shape: ${JSON.stringify(json).slice(0, 200)}`) +} +setUser(json.data.profile.name) +``` + +--- + +## Error-Fix Mappings + +| Error Message | Root Cause | Fix | +|---------------|------------|-----| +| `TypeError: Cannot read properties of undefined (reading 'X')` | Null/undefined accessed via property chain after API response | Add optional chaining or explicit null check before deep access | +| `Network request failed` | No network or wrong host in dev | Check `__DEV__` vs production API URL; verify device can reach the API host | +| `JSON Parse error: Unrecognized token '<'` | Server returned HTML (error page) instead of JSON | Check `res.ok` before calling `res.json()` — server returned 4xx/5xx | +| `Maximum update depth exceeded` | State setter called inside render or effect without dependency guard | Move setter into event handler or add correct deps array to `useEffect` | +| `Warning: Can't perform a React state update on an unmounted component` | Async operation completes after component unmounts | Return cleanup function from `useEffect` that cancels in-flight request | +| `Unhandled promise rejection: Error: Invariant Violation` | Native module call outside the main thread context | Move native module calls to a dedicated service, not inside callbacks | + +--- + +## Version-Specific Notes + +| Version | Change | Impact | +|---------|--------|--------| +| RN 0.71 | `Promise.allSettled` enabled by default in Hermes | Use `allSettled` instead of `all` when you want partial results on failure | +| RN 0.73 | Unhandled rejection handling improved in Hermes | Stack traces from async errors are now preserved — update Sentry sourcemap upload | +| React 18 | `startTransition` errors fall back to nearest ErrorBoundary | Transitions that throw no longer crash the whole tree | +| `@sentry/react-native` 5.0 | `Sentry.wrap(App)` deprecated — use `Sentry.init()` then `withSentry(App)` | Update app entry if using older Sentry integration pattern | + +--- + +## Detection Commands Reference + +```bash +# Find console.error used as only error reporting (not in tests) +grep -rn 'console\.error' --include="*.tsx" --include="*.ts" | grep -v "\.test\.\|\.spec\." + +# Find empty catch blocks +grep -rn 'catch\s*(.*)\s*{\s*}' --include="*.ts" --include="*.tsx" + +# Find fetch calls without .ok check +grep -rn 'await fetch\|\.json()' --include="*.ts" --include="*.tsx" | grep -v 'res\.ok\|response\.ok' + +# Find deep property access on API responses without null guards +grep -rn '\.data\.\|\.result\.' --include="*.ts" --include="*.tsx" | grep -v '\?\.' + +# Find missing ErrorBoundary around screen components (Expo Router pattern) +grep -rn 'Stack\.Screen\|Tabs\.Screen' --include="*.tsx" | grep -v 'ErrorBoundary' +``` + +--- + +## See Also + +- `rendering-patterns.md` — Text component crashes and conditional render crashes during render phase +- `state-management.md` — Stale state that causes incorrect error recovery diff --git a/agents/react-native-engineer/references/testing.md b/agents/react-native-engineer/references/testing.md new file mode 100644 index 00000000..ea6723c9 --- /dev/null +++ b/agents/react-native-engineer/references/testing.md @@ -0,0 +1,271 @@ +# Testing Reference + + +> **Scope**: React Native test setup, React Native Testing Library (RNTL) patterns, Maestro E2E, native module mocking, and Expo test configuration. +> **Version range**: React Native 0.72+, RNTL 12+, Expo SDK 50+ +> **Generated**: 2026-04-12 — verify against current @testing-library/react-native release notes + +--- + +## Overview + +React Native testing has three distinct layers: unit/component tests via RNTL, integration via jest with native module mocks, and E2E via Maestro or Detox. The most common failure mode is testing implementation details (renders, re-renders, internal state) instead of user-observable behavior. Native modules require manual mocking; RNTL's `render` does not invoke native code. + +--- + +## Pattern Table + +| Tool | Version | Use When | Avoid When | +|------|---------|----------|------------| +| `@testing-library/react-native` | `12+` | component behavior, user interactions | testing animation internals, native rendering | +| `jest-expo` | `SDK 50+` | Expo managed workflow tests | bare RN without Expo — use `react-native` preset | +| `Maestro` | `1.36+` | E2E flows on device/simulator | unit-level component logic | +| `Detox` | `20+` | E2E when Maestro yaml DSL is insufficient | simple flows — Maestro is faster to author | +| `@react-native-community/async-storage/jest/async-storage-mock` | any | mocking AsyncStorage in unit tests | leave AsyncStorage unmocked — causes silent hang | + +--- + +## Correct Patterns + +### Test User Behavior, Not Implementation + +Query by accessible labels and roles, not test IDs or internal component names. + +```tsx +import { render, fireEvent, screen } from '@testing-library/react-native' +import { LoginScreen } from './LoginScreen' + +it('submits login with valid credentials', () => { + render() + + fireEvent.changeText(screen.getByLabelText('Email'), 'user@example.com') + fireEvent.changeText(screen.getByLabelText('Password'), 'hunter2') + fireEvent.press(screen.getByRole('button', { name: 'Log in' })) + + expect(screen.getByText('Logging in...')).toBeTruthy() +}) +``` + +**Why**: `getByLabelText` and `getByRole` query what a screen reader sees. If you rename a component but keep its accessible label, the test still passes. If you rename the label, the test correctly fails. + +--- + +### Mock Native Modules at the jest Config Level + +Native modules that don't exist in the jest environment must be mocked globally — not inside individual test files — so every test file gets the mock automatically. + +```js +// jest.config.js +module.exports = { + preset: 'jest-expo', // or 'react-native' + setupFilesAfterFramework: ['./jest.setup.ts'], + moduleNameMapper: { + // mock native camera module + 'react-native-vision-camera': '/__mocks__/react-native-vision-camera.ts', + }, +} +``` + +```ts +// __mocks__/react-native-vision-camera.ts +export const Camera = jest.fn(() => null) +export const useCameraDevices = jest.fn(() => ({ back: {}, front: {} })) +``` + +**Why**: Native modules throw "Cannot read properties of null" in jest because the native bridge doesn't exist. Module-level mocks prevent test pollution across files. + +--- + +### Use `waitFor` for Async State Changes + +Avoid `setTimeout` delays. Use `waitFor` from RNTL which polls until assertion passes or timeout expires. + +```tsx +import { render, waitFor, screen } from '@testing-library/react-native' + +it('shows loaded data after fetch', async () => { + render() + + // Wrong: arbitrary delay + // await new Promise(r => setTimeout(r, 500)) + + // Correct: poll until assertion passes + await waitFor(() => { + expect(screen.getByText('Jane Doe')).toBeTruthy() + }) +}) +``` + +**Why**: Arbitrary delays make tests either flaky (too short) or slow (too long). `waitFor` exits as soon as the assertion passes, bounding wait time without hardcoding delays. + +--- + +### Set Up AsyncStorage Mock Globally + +AsyncStorage calls that hit the real module in jest hang indefinitely with no error. + +```ts +// jest.setup.ts +import mockAsyncStorage from '@react-native-async-storage/async-storage/jest/async-storage-mock' +jest.mock('@react-native-async-storage/async-storage', () => mockAsyncStorage) +``` + +**Why**: The real AsyncStorage module requires native bridge. Without the mock, `getItem`/`setItem` calls return `Promise` — the test hangs until jest timeout kills it. + +--- + +## Anti-Pattern Catalog + +### ❌ Querying by `testID` Instead of Accessible Attributes + +**Detection**: +```bash +grep -rn 'getByTestId\|findByTestId' --include="*.test.tsx" --include="*.spec.tsx" +rg 'getByTestId|findByTestId' --type tsx +``` + +**What it looks like**: +```tsx +const button = screen.getByTestId('submit-button') +fireEvent.press(button) +``` + +**Why wrong**: `testID` is invisible to users and screen readers. Tests using it break if the component is refactored even when behavior is unchanged. They also don't verify accessibility — a button with no label passes the test but fails assistive technology users. + +**Fix**: +```tsx +const button = screen.getByRole('button', { name: 'Submit' }) +fireEvent.press(button) +``` + +**Version note**: RNTL 7+ requires the component to have `accessible={true}` or a role for `getByRole` to find it. Add `accessibilityRole="button"` to `Pressable` components. + +--- + +### ❌ Importing from `react-native` Instead of `@testing-library/react-native` + +**Detection**: +```bash +grep -rn "from 'react-native'" --include="*.test.tsx" --include="*.spec.tsx" | grep -v "^.*//.*from 'react-native'" +``` + +**What it looks like**: +```tsx +import { render } from 'react-native' // wrong — no render export +import { act } from 'react-native/test-utils' // old pattern +``` + +**Why wrong**: `react-native` doesn't export `render`. These imports cause `Cannot find module` errors or silently import the wrong `act` (react vs react-native differ on batching semantics). + +**Fix**: +```tsx +import { render, fireEvent, screen, act, waitFor } from '@testing-library/react-native' +``` + +--- + +### ❌ Mocking `useNavigation` Inside Individual Test Files + +**Detection**: +```bash +grep -rn "jest.mock.*navigation\|jest.mock.*router" --include="*.test.tsx" +``` + +**What it looks like**: +```tsx +jest.mock('@react-navigation/native', () => ({ + useNavigation: () => ({ navigate: jest.fn() }), +})) +``` + +**Why wrong**: Repeated local mocks diverge over time. When the real API adds a new field (`navigation.setOptions`, `navigation.getId`), each test file needs updating separately. Components that call multiple navigation hooks get increasingly complex local mocks. + +**Fix**: Create a single navigation mock in `__mocks__/@react-navigation/native.ts` and let jest auto-resolve it. Or use `createNavigationContainerRef` and wrap the component in a `NavigationContainer` in the test. + +```tsx +import { NavigationContainer } from '@react-navigation/native' + +function renderWithNavigation(ui: React.ReactElement) { + return render({ui}) +} +``` + +--- + +### ❌ Using Snapshots for UI Components + +**Detection**: +```bash +grep -rn 'toMatchSnapshot\|toMatchInlineSnapshot' --include="*.test.tsx" +rg 'toMatchSnapshot' --type tsx +``` + +**What it looks like**: +```tsx +it('renders correctly', () => { + const tree = render().toJSON() + expect(tree).toMatchSnapshot() +}) +``` + +**Why wrong**: Snapshot tests fail on every styling or structure change, creating update churn. They assert nothing about behavior — a component that renders the wrong text passes if the snapshot was wrong to begin with. They're especially noisy in monorepos. + +**Fix**: Test specific rendered output that reflects user-visible behavior: +```tsx +it('displays the title', () => { + render() + expect(screen.getByText('Hello')).toBeTruthy() +}) +``` + +--- + +## Error-Fix Mappings + +| Error Message | Root Cause | Fix | +|---------------|------------|-----| +| `Invariant Violation: TurboModuleRegistry.getEnforcing(...)` | Native module accessed in jest without mock | Add module to `moduleNameMapper` in jest.config or mock in setupFilesAfterFramework | +| `Unable to find an element with the text: ...` | Component not yet rendered or async data not resolved | Wrap assertion in `await waitFor(...)` | +| `Warning: An update to ... inside a test was not wrapped in act(...)` | State update triggered after test ended or outside act | Use `await act(async () => { ... })` around the trigger, or use `waitFor` | +| `Cannot find module '@testing-library/react-native'` | Package not installed | `bun add -D @testing-library/react-native` or `npm install -D @testing-library/react-native` | +| `Element type is invalid: expected a string or class/function but got undefined` | Mocked module returns wrong shape | Check `__mocks__` file — default export may be missing or named exports wrong | +| `jest did not exit one second after the test run has completed` | Unmocked async module (AsyncStorage, NetInfo) holds open handle | Add mock in `jest.setup.ts` for all native async modules | + +--- + +## Version-Specific Notes + +| Version | Change | Impact | +|---------|--------|--------| +| RNTL 12.0 | `userEvent` API added — simulates real user gestures, not synthetic events | Prefer `userEvent.press()` over `fireEvent.press()` for interaction fidelity | +| RNTL 12.4 | `screen` query object promoted to stable — no need to destructure `render()` return | Use `screen.getByText()` instead of `const { getByText } = render()` | +| Expo SDK 50 | `jest-expo` preset updated to support new architecture (JSI) | If tests hang after SDK 50 upgrade, check `jest-expo` version matches SDK | +| RN 0.73 | New Architecture (Fabric) enabled by default in new projects | Some community library mocks break under Fabric — check library's jest setup docs | + +--- + +## Detection Commands Reference + +```bash +# Find testID queries that should be role/label queries +grep -rn 'getByTestId\|findByTestId' --include="*.test.tsx" --include="*.spec.tsx" + +# Find snapshot tests in component files +grep -rn 'toMatchSnapshot\|toMatchInlineSnapshot' --include="*.test.tsx" + +# Find unmocked native module patterns +grep -rn 'jest.mock.*native' --include="*.test.tsx" | grep -v "__mocks__" + +# Find setTimeout used as async delay in tests (should use waitFor) +grep -rn 'setTimeout.*[0-9]' --include="*.test.tsx" --include="*.spec.tsx" + +# Find old act import pattern +grep -rn "from 'react-native/test-utils'\|from 'react-test-renderer'" --include="*.test.tsx" +``` + +--- + +## See Also + +- `rendering-patterns.md` — Text component rules that affect what RNTL can query +- `list-performance.md` — FlashList/LegendList require specific test setup for virtualization From 8838ed899f98849920f5f6c1b2c097092a749b37 Mon Sep 17 00:00:00 2001 From: notque Date: Sun, 12 Apr 2026 04:10:34 -0700 Subject: [PATCH 2/2] =?UTF-8?q?feat(refs):=20react-portfolio-engineer=20?= =?UTF-8?q?=E2=80=94=20Next.js=20App=20Router=20patterns=20and=20Core=20We?= =?UTF-8?q?b=20Vitals=20(Level=202=E2=86=923)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two reference files to lift react-portfolio-engineer from Level 2 to Level 3: - nextjs-app-router.md: App Router Server vs Client component split, generateStaticParams, generateMetadata per artwork, URL search params for category filtering, anti-patterns for 'use client' on full pages and legacy getServerSideProps in app/ directory - performance.md: Core Web Vitals targets (LCP/CLS/INP), priority loading for hero images, explicit width/height to prevent CLS, responsive sizes prop, dynamic imports for lightbox, anti-patterns for multiple priority props, missing sizes, unsync filter without useMemo, barrel icon library imports Both files include detection commands (grep/rg) and error-fix mappings. Loading table in agent body updated to map task signals to reference files. --- agents/react-portfolio-engineer.md | 13 +- .../references/nextjs-app-router.md | 295 +++++++++++++++++ .../references/performance.md | 312 ++++++++++++++++++ 3 files changed, 616 insertions(+), 4 deletions(-) create mode 100644 agents/react-portfolio-engineer/references/nextjs-app-router.md create mode 100644 agents/react-portfolio-engineer/references/performance.md diff --git a/agents/react-portfolio-engineer.md b/agents/react-portfolio-engineer.md index d2700fab..667cfe33 100644 --- a/agents/react-portfolio-engineer.md +++ b/agents/react-portfolio-engineer.md @@ -351,10 +351,15 @@ STOP and ask the user (get explicit confirmation) when: ## References -For detailed information: -- **Lightbox Patterns**: [references/lightbox-patterns.md](references/lightbox-patterns.md) - Complete lightbox implementation -- **Image Optimization**: [references/image-optimization.md](references/image-optimization.md) - next/image best practices -- **Responsive Design**: [references/responsive-design.md](references/responsive-design.md) - Breakpoints and mobile patterns +Load these reference files based on the task type: + +| Task Type | Reference File | +|-----------|---------------| +| Lightbox implementation, keyboard/touch navigation | [references/lightbox-patterns.md](references/lightbox-patterns.md) | +| next/image, blur placeholders, WebP/AVIF, format config | [references/image-optimization.md](references/image-optimization.md) | +| Breakpoints, mobile-first CSS, touch interactions | [references/responsive-design.md](references/responsive-design.md) | +| App Router pages, Server vs Client components, metadata API, URL filtering, SSG | [references/nextjs-app-router.md](references/nextjs-app-router.md) | +| Core Web Vitals, LCP, CLS, INP, bundle size, `priority`, `sizes` prop | [references/performance.md](references/performance.md) | **Shared Patterns**: - [anti-rationalization-core.md](../skills/shared-patterns/anti-rationalization-core.md) - Universal rationalization patterns diff --git a/agents/react-portfolio-engineer/references/nextjs-app-router.md b/agents/react-portfolio-engineer/references/nextjs-app-router.md new file mode 100644 index 00000000..a9e3d72c --- /dev/null +++ b/agents/react-portfolio-engineer/references/nextjs-app-router.md @@ -0,0 +1,295 @@ +# Next.js App Router Reference + +> **Scope**: App Router patterns specific to portfolio/gallery sites: Server vs Client components, metadata API, static generation, and data fetching. +> **Version range**: Next.js 13.4+ (App Router stable), 14+ (recommended) +> **Generated**: 2026-04-12 — verify against Next.js changelog for latest changes + +--- + +## Overview + +Portfolio sites built with Next.js App Router should default to Server Components for all static content (gallery grids, about pages, artwork metadata) and use Client Components only for interactive elements (lightbox, filtering UI, touch gestures). The most common failure mode is marking entire page trees as `'use client'` — this disables static generation and ships full JavaScript bundles for content that needs none. + +--- + +## Pattern Table + +| Pattern | Version | Use When | Avoid When | +|---------|---------|----------|------------| +| `generateStaticParams` | 13.4+ | Gallery pages with known slugs | Dynamic content from user input | +| `generateMetadata` | 13.4+ | Per-artwork SEO titles/OG images | Static metadata for all pages | +| `export const dynamic = 'force-static'` | 13.4+ | Page must never run server-side | Page reads cookies/headers | +| `` with `loading.tsx` | 13.4+ | Data fetching in Server Components | Wrapping non-async content | +| `unstable_cache` | 14.0+ | Repeated CMS/API fetches per build | One-off requests | + +--- + +## Correct Patterns + +### Server Component for Gallery Grid + +Gallery grids render the same content for all visitors. Use a Server Component: no `'use client'`, data fetch directly in the component, no bundle cost. + +```tsx +// app/gallery/page.tsx — Server Component (no 'use client') +import { GalleryGrid } from '@/components/GalleryGrid' +import { getArtworks } from '@/lib/artworks' + +export default async function GalleryPage() { + const artworks = await getArtworks() // runs at build time (SSG) + return +} + +// GalleryGrid can also be a Server Component if it has no event handlers +// components/GalleryGrid.tsx +import Image from 'next/image' + +export function GalleryGrid({ artworks }: { artworks: Artwork[] }) { + return ( +
+ {artworks.map(art => ( + {art.alt} + ))} +
+ ) +} +``` + +**Why**: Server Components ship zero JavaScript for the component itself. A gallery grid serving 50 artworks with plain img tags still renders as static HTML. + +--- + +### Isolate Interactivity at the Leaf + +Client Components should be small, leaf nodes. Hoist `'use client'` as deep as possible to preserve static generation of parent content. + +```tsx +// ✅ Correct: only the lightbox trigger is a Client Component +// components/ArtworkCard.tsx — Server Component +import { LightboxTrigger } from './LightboxTrigger' // Client Component + +export function ArtworkCard({ artwork }: { artwork: Artwork }) { + return ( +
+ {artwork.alt} +

{artwork.title}

+ {/* Only interactive part */} +
+ ) +} + +// components/LightboxTrigger.tsx +'use client' +import { useLightbox } from '@/hooks/useLightbox' + +export function LightboxTrigger({ artworkId }: { artworkId: string }) { + const { open } = useLightbox() + return ( + + ) +} +``` + +--- + +### Per-Artwork Metadata with generateMetadata + +Each artwork detail page should have unique title and OG image for social sharing. + +```tsx +// app/gallery/[slug]/page.tsx +import { getArtworkBySlug } from '@/lib/artworks' +import type { Metadata } from 'next' + +export async function generateMetadata({ params }: { params: { slug: string } }): Promise { + const artwork = await getArtworkBySlug(params.slug) + return { + title: `${artwork.title} — Portfolio`, + description: artwork.description, + openGraph: { + images: [{ url: artwork.src, width: 1200, height: 630 }], + }, + } +} + +export async function generateStaticParams() { + const artworks = await getArtworkBySlug('*') // get all slugs + return artworks.map(art => ({ slug: art.slug })) +} +``` + +--- + +## Anti-Pattern Catalog + +### ❌ Marking the Whole Page as Client Component + +**Detection**: +```bash +grep -rn "'use client'" app/ --include="*.tsx" | grep "page.tsx" +rg "'use client'" --type tsx -l | xargs grep -l "export default function" +``` + +**What it looks like**: +```tsx +// app/gallery/page.tsx +'use client' // ← entire page becomes a Client Component +import { useState } from 'react' +import Image from 'next/image' +import { artworks } from '@/data/artworks' + +export default function GalleryPage() { + const [filter, setFilter] = useState('all') + // ...gallery rendering with Image components +} +``` + +**Why wrong**: The entire gallery tree becomes client-rendered JavaScript. Static generation is disabled — Next.js must rerun the page on every request. Images lose build-time optimization hints. Bundle size includes all artwork metadata in the initial JS payload. + +**Fix**: Extract `useState` filtering into a `FilterBar` Client Component. Keep `GalleryPage` as a Server Component that passes artworks down. + +```tsx +// app/gallery/page.tsx — Server Component +import { FilterBar } from '@/components/FilterBar' // 'use client' +import { artworks } from '@/data/artworks' + +export default function GalleryPage() { + return // FilterBar handles state internally +} +``` + +--- + +### ❌ Using useRouter for Category Filtering Instead of URL Search Params + +**Detection**: +```bash +grep -rn "useRouter\|router.push" --include="*.tsx" app/ +rg "setFilter\|activeFilter" --type tsx +``` + +**What it looks like**: +```tsx +'use client' +const [activeCategory, setActiveCategory] = useState('all') +// Filter applied in memory, URL does not reflect state +``` + +**Why wrong**: Filtering state disappears on page refresh. Users cannot share links to specific gallery categories. Back button loses filter context. Browser history is not updated. + +**Fix**: Use `useSearchParams` + `useRouter` to sync filter with URL: + +```tsx +'use client' +import { useSearchParams, useRouter } from 'next/navigation' + +export function FilterBar({ categories }: { categories: string[] }) { + const searchParams = useSearchParams() + const router = useRouter() + const active = searchParams.get('category') ?? 'all' + + const setFilter = (cat: string) => { + const params = new URLSearchParams(searchParams) + if (cat === 'all') params.delete('category') + else params.set('category', cat) + router.push(`?${params.toString()}`, { scroll: false }) + } + + return ( + + ) +} +``` + +**Version note**: `useSearchParams` requires a `` boundary in Next.js 14+. Wrap the component in `` or use the `params` prop from the page. + +--- + +### ❌ Calling getServerSideProps in App Router Pages + +**Detection**: +```bash +grep -rn "getServerSideProps\|getStaticProps" app/ --include="*.tsx" +``` + +**What it looks like**: +```tsx +// app/gallery/page.tsx — WRONG in App Router +export async function getStaticProps() { ... } +export async function getServerSideProps() { ... } +``` + +**Why wrong**: These are Pages Router APIs. In App Router they are silently ignored — the functions exist but never run. Data fetching must happen in the `async` component body directly. + +**Fix**: Fetch data directly in the async Server Component: + +```tsx +export default async function GalleryPage() { + const artworks = await fetch('https://api.example.com/artworks').then(r => r.json()) + return +} +``` + +--- + +## Error-Fix Mappings + +| Error Message | Root Cause | Fix | +|---------------|------------|-----| +| `Error: useState is not a function` in Server Component | `useState` imported in non-'use client' file | Add `'use client'` to the file or extract hook to a Client Component | +| `Error: useSearchParams() should be wrapped in a suspense boundary` | Next.js 14 requires Suspense for useSearchParams | Wrap with `` | +| `Warning: Each child in a list should have a unique "key" prop` | Missing `key` on mapped artwork elements | Add `key={artwork.id}` to mapped Image/article elements | +| `Image with src "..." was detected as the Largest Contentful Paint` without `priority` | Hero image not marked as priority | Add `priority` prop to the first visible Image | +| `next/image Un-configured Host` | External image domain not in next.config.js | Add domain to `images.domains` or `images.remotePatterns` | + +--- + +## Version-Specific Notes + +| Version | Change | Impact | +|---------|--------|--------| +| 13.4 | App Router stable; `generateStaticParams` replaces `getStaticPaths` | Portfolio pages should migrate from Pages Router | +| 14.0 | `useSearchParams` requires Suspense boundary | Wrap all filter components in Suspense | +| 14.1 | `unstable_cache` added for deduplicating CMS fetches | Use for Sanity/Contentful per-build calls | +| 15.0 | Partial Prerendering (PPR) experimental; static shell + dynamic holes | Useful for galleries with analytics-driven "popular" sections | + +--- + +## Detection Commands Reference + +```bash +# Pages incorrectly marked as Client Components +grep -rn "'use client'" app/ --include="*.tsx" | grep "page.tsx" + +# Legacy Pages Router APIs used in App Router +grep -rn "getServerSideProps\|getStaticProps" app/ --include="*.tsx" + +# useState-based category filter (should be URL search params) +rg "useState.*category|useState.*filter" --type tsx + +# Missing Suspense around useSearchParams (Next.js 14+) +grep -rn "useSearchParams" --include="*.tsx" | grep -v "Suspense" +``` + +--- + +## See Also + +- `image-optimization.md` — next/image props, WebP/AVIF config, blur placeholders +- `performance.md` — Core Web Vitals, LCP optimization, bundle analysis diff --git a/agents/react-portfolio-engineer/references/performance.md b/agents/react-portfolio-engineer/references/performance.md new file mode 100644 index 00000000..5b67a8c4 --- /dev/null +++ b/agents/react-portfolio-engineer/references/performance.md @@ -0,0 +1,312 @@ +# Performance Optimization Reference + +> **Scope**: Core Web Vitals optimization for portfolio/gallery sites: LCP, CLS, INP targets, bundle analysis, and Next.js-specific strategies. +> **Version range**: Next.js 14+, React 18+ +> **Generated**: 2026-04-12 — verify LCP/CLS/INP thresholds against web.dev/vitals + +--- + +## Overview + +Portfolio sites are image-heavy by definition, which makes them high-risk for poor Core Web Vitals. The two most impactful metrics are LCP (Largest Contentful Paint — the hero image load time) and CLS (Cumulative Layout Shift — images causing layout jumps before they load). The third metric, INP (Interaction to Next Paint), primarily affects filtering and lightbox open/close interactions. Getting LCP below 2.5s on a portfolio requires correct `priority`, explicit `width/height` on every image, and proper format negotiation. + +--- + +## Pattern Table + +| Metric | Good | Needs Improvement | Poor | Primary Cause for Portfolios | +|--------|------|-------------------|------|------------------------------| +| LCP | < 2.5s | 2.5–4.0s | > 4.0s | Hero image not `priority`, wrong `sizes` | +| CLS | < 0.1 | 0.1–0.25 | > 0.25 | Missing width/height on images | +| INP | < 200ms | 200–500ms | > 500ms | Heavy JS on filter interactions | + +--- + +## Correct Patterns + +### Priority Loading for Hero Image + +The hero image IS the LCP element for portfolio sites. It must be preloaded — `priority` tells Next.js to inject a `` in the document head. + +```tsx +// app/page.tsx — hero section +import Image from 'next/image' + +export default function HomePage() { + return ( +
+ Featured artwork: oil painting of winter coastline at dusk, critical for LCP + sizes="100vw" // hero takes full viewport width + className="object-cover w-full h-screen" + /> +
+ ) +} +``` + +**Why**: Without `priority`, Next.js lazy-loads the image. For a full-bleed hero, this means the LCP element starts loading only after the page's critical CSS is parsed — typically adding 400-800ms to LCP on mobile. + +--- + +### Explicit Width/Height on Every Gallery Image + +Prevents CLS by reserving space before the image loads. The browser calculates the aspect ratio from `width` and `height` and reserves that space immediately. + +```tsx +// ✅ Correct: explicit dimensions prevent layout shift +{artwork.alt} + +// ❌ Wrong: fill without container dimensions causes CLS +
{/* no height set on container */} + {artwork.alt} +
+``` + +**When using `fill`**: The container MUST have `position: relative` and an explicit height (or `aspect-ratio`): + +```tsx +
{/* aspect-ratio reserves space */} + {artwork.alt} +
+``` + +--- + +### Responsive `sizes` Prop + +Without `sizes`, Next.js generates a srcSet but the browser picks the largest size by default. A correct `sizes` string cuts image payload by 50-70% on mobile. + +```tsx +// Single-column mobile, two-column tablet, three-column desktop grid +{artwork.alt} + +// Full-width hero + + +// Fixed sidebar thumbnail (never changes size) + +// (no sizes needed when dimensions are truly fixed) +``` + +--- + +### Route-Level Code Splitting for Heavy Gallery Features + +Lightbox libraries and masonry layout engines should not ship on initial load. Use dynamic imports with `ssr: false` for client-only UI. + +```tsx +// components/Lightbox.tsx — heavy keyboard/touch handler +import dynamic from 'next/dynamic' + +const LightboxModal = dynamic( + () => import('./LightboxModal'), + { + ssr: false, // Lightbox is client-only + loading: () => null, // render nothing while loading (not a spinner — it opens fast) + } +) + +export function Lightbox(props: LightboxProps) { + return +} +``` + +**Why**: Lightbox code is only needed after a user clicks. Dynamic import defers its ~15-30KB until interaction, improving initial bundle size and TTI. + +--- + +## Anti-Pattern Catalog + +### ❌ Multiple `priority` Images + +**Detection**: +```bash +grep -rn "priority" --include="*.tsx" | grep -v "//.*priority" +rg "priority(?:={true})?" --type tsx +``` + +**What it looks like**: +```tsx +// Every gallery card has priority — defeats the purpose +{artworks.map(art => ( + {art.alt} +))} +``` + +**Why wrong**: `priority` injects a `` for each image. With 12 gallery items, that's 12 preloads competing for bandwidth at page load. The browser's preload queue is limited; excess preloads are ignored or delay the actual LCP image. Initial page load slows down. + +**Fix**: Use `priority` only for the first visible image (hero or first above-fold gallery item). For the rest, use default lazy loading: + +```tsx +{artworks.map((art, index) => ( + {art.alt} +))} +``` + +--- + +### ❌ Missing `sizes` on Responsive Gallery Images + +**Detection**: +```bash +grep -rn "next/image\|from 'next/image'" --include="*.tsx" -l | xargs grep -L "sizes=" +rg "width=\{[0-9]+\}.*height=\{[0-9]+\}" --type tsx | grep -v "sizes=" +``` + +**What it looks like**: +```tsx +{artwork.alt} +``` + +**Why wrong**: The browser requests a 1200px image even on a 375px mobile screen. The generated srcSet is useless without a `sizes` hint. A 3-column desktop grid serving 300px thumbnails will download 1200px source images on mobile — typically 4-8x the necessary payload. + +**Fix**: Add a `sizes` prop that matches the CSS layout: + +```tsx +{artwork.alt} +``` + +**Version note**: This applies to all Next.js versions. The default 100vw assumption was introduced intentionally to fail safe (never load too small) but portfolio sites pay the mobile bandwidth cost. + +--- + +### ❌ Synchronous Heavy Computation on Filter Interaction + +**Detection**: +```bash +grep -rn "\.filter\(.*\.map\|\.sort\(.*\.filter" --include="*.tsx" +rg "useMemo|useCallback" --type tsx -l +``` + +**What it looks like**: +```tsx +'use client' +const filtered = artworks + .filter(a => a.category === activeCategory) + .sort((a, b) => new Date(b.date).getTime() - new Date(a.date).getTime()) + .map(a => ({ ...a, formattedDate: formatDate(a.date) })) +// Runs on every render, including every keystroke in a search input +``` + +**Why wrong**: Synchronous sort + map on 100+ artworks blocks the main thread. INP for the filter interaction will exceed 200ms on mid-range mobile devices. React re-renders include this computation on every state change. + +**Fix**: Memoize with `useMemo` and stable dependencies: + +```tsx +const filtered = useMemo(() => + artworks + .filter(a => activeCategory === 'all' || a.category === activeCategory) + .sort((a, b) => b.date.localeCompare(a.date)), + [artworks, activeCategory] // only recomputes when these change +) +``` + +--- + +### ❌ Importing Full Icon Libraries + +**Detection**: +```bash +grep -rn "from 'react-icons'" --include="*.tsx" +grep -rn "from '@heroicons/react'" --include="*.tsx" | grep "^[^/]" +``` + +**What it looks like**: +```tsx +import { FaArrowLeft, FaArrowRight, FaTimes } from 'react-icons/fa' +// Imports entire react-icons/fa barrel — adds ~50KB to bundle +``` + +**Why wrong**: Named imports from barrel files like `react-icons/fa` often pull in the entire icon set due to CommonJS export semantics. Even with tree-shaking, some bundler configurations ship the whole 500+ icon set. + +**Fix**: Import from the specific icon's module path, or use inline SVGs for a handful of icons: + +```tsx +// Option A: path-specific import +import FaArrowLeft from 'react-icons/fa/FaArrowLeft' + +// Option B: inline SVG (best for <5 icons in a portfolio lightbox) +export function CloseIcon() { + return ( + + ) +} +``` + +--- + +## Error-Fix Mappings + +| Error Message | Root Cause | Fix | +|---------------|------------|-----| +| `Image is missing required "width" property` | Using `fill` without container or missing props | Add `width` and `height`, or use `fill` with a positioned container | +| `Error: Image optimization using the default loader is not compatible with next export` | Using `next export` (static HTML) with next/image | Add `unoptimized: true` in next.config.js, or use Vercel/custom loader | +| `Warning: Image with src "X" has "fill" but is missing "sizes" prop` | Next.js 13+ fill images need `sizes` | Add `sizes` matching the layout breakpoints | +| `Hydration failed because the initial UI does not match what was rendered on the server` | Client-only state (lightbox open) rendered in SSR | Move to `useEffect` or use `suppressHydrationWarning` for ephemeral UI state | + +--- + +## Detection Commands Reference + +```bash +# Multiple priority images (only first above-fold image should have priority) +grep -rn "priority" --include="*.tsx" app/ components/ + +# Images missing sizes prop +rg "