diff --git a/AGENTS.md b/AGENTS.md index 8a61343bc..6ef8161c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,6 +37,7 @@ Detailed guides for specific subsystems live in `contrib/claude/`: - [`contrib/claude/graphql.md`](contrib/claude/graphql.md) — Go GraphQL backend (gqlgen, @goModel, connection types, cursor pagination) - [`contrib/claude/license.md`](contrib/claude/license.md) — ISC license header (all file types) - [`contrib/claude/mcp.md`](contrib/claude/mcp.md) — MCP API patterns (specification.yaml, mcpgen, resolvers, type helpers) +- [`contrib/claude/react-components.md`](contrib/claude/react-components.md) — React component shape (file/export, props, configure vs data via hooks) - [`contrib/claude/relay.md`](contrib/claude/relay.md) — Frontend Relay client (queries, fragments, mutations, pagination) - [`contrib/claude/ui.md`](contrib/claude/ui.md) — `@probo/ui`, Tailwind, tailwind-variants, folders, skeletons, compound components - [`contrib/claude/release.md`](contrib/claude/release.md) — Release process (version bump, changelog, tag, push) diff --git a/contrib/claude/react-components.md b/contrib/claude/react-components.md new file mode 100644 index 000000000..576dd3785 --- /dev/null +++ b/contrib/claude/react-components.md @@ -0,0 +1,318 @@ +# React component conventions + +This document describes **how to define and shape** React components in Probo frontends (`apps/console`, [`packages/ui`](../../packages/ui), and related apps). It complements styling and package layout in [`contrib/claude/ui.md`](ui.md) and data loading in [`contrib/claude/relay.md`](relay.md). + +**The codebase does not fully match these rules yet.** Treat this guide as the target for new work and refactors. + +## Related guides + +| Topic | Guide | +|-------|--------| +| `@probo/ui`, Tailwind, `tailwind-variants`, folders, skeletons, compound modules | [`contrib/claude/ui.md`](ui.md) | +| Relay queries, fragments, loaders, `queryRef` | [`contrib/claude/relay.md`](relay.md) | + +## Component shape + +| Rule | Convention | +|------|------------| +| Paradigm | **Functional components only.** Class components are not used except in rare cases that require lifecycle methods unavailable as hooks (e.g. `ErrorBoundary`). | +| Syntax | **Traditional `function` declarations**, not `const` storing arrow functions. | +| Typing | **Do not use `React.FC` (or `FC`).** Props are typed via the function's parameter; the return type is inferred. | +| Props | **Always destructure props.** Prefer destructuring in the function parameters. When that would make the declaration line exceed the lint line-length limit, accept `props` as the parameter and destructure in the function body. | + +### Do / don't: component syntax + +```tsx +// Bad — arrow function assigned to const + FC +const UserCard: FC = ({ name }) => { + return
{name}
; +}; +``` + +```tsx +// Bad — arrow function without FC +const UserCard = ({ name }: UserCardProps) => { + return
{name}
; +}; +``` + +```tsx +// Good — traditional function declaration, no FC +export function UserCard({ name }: UserCardProps) { + return
{name}
; +} +``` + +### Do / don't: props destructuring + +```tsx +// Bad — accessing props without destructuring +export function UserCard(props: UserCardProps) { + return
{props.name}
; +} +``` + +```tsx +// Good — destructure in function parameters (preferred) +export function UserCard({ name }: UserCardProps) { + return
{name}
; +} +``` + +```tsx +// Good — destructure in body when parameter-level destructuring would exceed the line-length limit +export function VendorComplianceOverviewPanel( + props: VendorComplianceOverviewPanelProps, +) { + const { className, vendorKey, onStatusChange } = props; + // … +} +``` + +## File and export + +| Rule | Convention | +|------|------------| +| Components per file | **One primary component per file.** Colocate non-UI modules separately (`variants.ts`, `graphql` template strings, tiny helpers). | +| File name | **Matches the component name** in PascalCase (e.g. `UserProfileHeader.tsx` → `UserProfileHeader`). | +| Export | **Named export** (`export function UserProfileHeader`). **Exception:** route or lazy bundle **entry** components may use `export default` when the router or `lazy()` requires it (see relay.md route pages). | +| Props type | **`ComponentNameProps`**. Prefer **`interface`**; use **`type`** when you need unions, mapped types, or **`PropsWithChildren<…>`** (e.g. wrappers whose props are only `children`). | + +### Do / don’t: file, name, and export + +```tsx +// Bad — two components in one file (split into Panel.tsx and PanelSection.tsx) +export function Panel({ children }: PanelProps) { + return
{children}
; +} +export function PanelSection({ children }: PanelSectionProps) { + return
{children}
; +} +``` + +```tsx +// Good — one component per file: PanelSection.tsx +import type { PropsWithChildren } from "react"; + +export type PanelSectionProps = PropsWithChildren; + +export function PanelSection({ children }: PanelSectionProps) { + return
{children}
; +} +``` + +```tsx +// Bad — file UserThing.tsx exports Thing; name should match +export function Thing({ label }: ThingProps) { + return null; +} +``` + +```tsx +// Good — file Thing.tsx +export interface ThingProps { + label: string; +} + +export function Thing({ label }: ThingProps) { + return {label}; +} +``` + +```tsx +// Good — rare exception: route entry default export (names still clear in module) +type VendorsPageProps = { + queryRef: PreloadedQuery; +}; + +export default function VendorsPage({ queryRef }: VendorsPageProps) { + // … +} +``` + +## Props ordering + +Within `ComponentNameProps`, order members as follows: + +1. **Non-callback props first** — DOM/React attributes (`className`, `style`, …), `ref` (or `forwardRef` typing), static UI configuration (`title`, `hideSidebar`), initial UX state (`defaultOpen`, `initialTab`). +2. **Callback props last** — `onClose`, `onSave`, `onOpenChange`, etc. + +### Do / don’t: prop order + +```tsx +// Bad — callbacks mixed before configuration +interface FormActionsProps { + onSave: () => void; + title: string; + className?: string; + onCancel: () => void; +} +``` + +```tsx +// Good — configure first, then callbacks +interface FormActionsProps { + className?: string; + title: string; + initialOpen?: boolean; + onOpenChange?: (open: boolean) => void; + onSave: () => void; + onCancel: () => void; +} +``` + +## Props are for configuration and composition, not data + +**Do not use props to pass data** in the broad sense: not fetched domain records, not lists of DTOs, and not identifiers that the component (or a dedicated hook) could read from the URL via React Router’s `useParams` or a hook built on it. + +Props **configure** how a component behaves or looks, or **compose** it with UI fragments. **Everything else belongs in hooks** (Relay, router, local state, context, etc.) inside the component or an immediate parent that owns real wiring. + +### Configure + +Use props for: + +- Standard HTML element attributes and React patterns: `className`, `style`, `id`, `aria-*`, `role`, and **`ref`** (including forwarded refs). +- **Static UI parameters:** `title`, `variant`, `hideSidebar`, `align`. +- **Initial client state** (parent does not own the live state): `defaultOpen`, `initialValue` — paired with `on*` if the parent must react. +- **Parent coordination callbacks** so the parent can update **its** state: `onCloseDropdown`, `onSubmit`, `onSelectionChange`. + +### Compose + +Use props for: + +- **`children`** and other **`ReactNode` slots** (`header`, `footer`, `icon`) that are **UI building blocks**, not serialized API payloads. +- Render props or slot components when they express **layout or UI variation**, not “here is the loaded entity.” + +### Hooks for data and URL-derived identity + +- **Fetched data:** Colocate Relay fragments and queries per [`contrib/claude/relay.md`](relay.md) (`useFragment`, `useLazyLoadQuery`, `usePreloadedQuery`, etc.) in the component that needs the data. +- **Route parameters:** Call `useParams()` (or a small `useOrganizationId()`-style hook) **inside** the component that needs the id — avoid drilling `organizationId` / `vendorId` from a parent that only read the URL to pass them down. + +### Relay: framework wiring is not “business data props” + +Relay sometimes requires **opaque handles** on props: e.g. **`queryRef`** for `usePreloadedQuery` on route pages, or a **fragment key** (`SomeFragment$key`) for `useFragment`. Those are **GraphQL/Relay wiring**, not passing arbitrary loaded objects through the tree. Keep using the patterns in [`contrib/claude/relay.md`](relay.md). Do not use those exceptions as a reason to pass plain domain objects or URL ids as props when a hook could read them instead. + +### Do / don’t: URL params + +```tsx +// Bad — parent only needed the param to pass it down +function VendorLayout() { + const { vendorId } = useParams(); + return ( +
+ +
+ ); +} + +function VendorSummary({ vendorId }: { vendorId: string }) { + return
{/* … */}
; +} +``` + +```tsx +// Good — component that needs the id reads it (or uses a dedicated hook) +function VendorLayout() { + return ( +
+ +
+ ); +} + +function VendorSummary() { + const { vendorId } = useParams(); + if (vendorId == null) { + return null; + } + return
{/* use vendorId in a hook / query … */}
; +} +``` + +### Do / don’t: fetched data + +```tsx +// Bad — parent loaded data and passes fields as props +function VendorPage() { + const vendor = useLazyLoadQuery(/* … */); + return ( + + ); +} +``` + +```tsx +// Good — header colocates its fragment and reads via useFragment +const vendorHeaderFragment = graphql` + fragment VendorHeader_vendor on Vendor { + name + riskScore + updatedAt + } +`; + +interface VendorHeaderProps { + className?: string; + vendorKey: VendorHeader_vendor$key; +} + +export function VendorHeader({ className, vendorKey }: VendorHeaderProps) { + const vendor = useFragment(vendorHeaderFragment, vendorKey); + return ( +
+ {/* render from vendor … */} +
+ ); +} +``` + +### Do / don’t: composition vs data-as-props + +```tsx +// Bad — every display field is a prop filled from fetched data elsewhere +interface ContactCardProps { + fullName: string; + email: string; + role: string; + createdAt: string; +} + +export function ContactCard({ fullName, email }: ContactCardProps) { + return ( +
+

{fullName}

+

{email}

+
+ ); +} +``` + +```tsx +// Good — props configure layout / slots; content is composed or read via hooks +import type { ReactNode } from "react"; + +interface PageSectionProps { + className?: string; + title: string; + icon?: ReactNode; + children: ReactNode; +} + +export function PageSection({ className, title, icon, children }: PageSectionProps) { + return ( +
+

+ {icon} + {title} +

+ {children} +
+ ); +} +``` + +(Snippet names and GraphQL types are illustrative; align with real schema and fragment names in the app.) diff --git a/contrib/claude/ui.md b/contrib/claude/ui.md index eba63d62d..315f4bb15 100644 --- a/contrib/claude/ui.md +++ b/contrib/claude/ui.md @@ -58,8 +58,8 @@ const row = tv({ }, defaultVariants: { bordered: true }, }); -export function Row(props: { bordered?: boolean; children: React.ReactNode }) { - return
{props.children}
; +export function Row({ bordered, children }: { bordered?: boolean; children: React.ReactNode }) { + return
{children}
; } ``` @@ -159,12 +159,12 @@ export const imageCard = tv({ // ImageCard/ImageCardShell.tsx — Good — slot class names on wrapping tags import { imageCard } from "./variants"; -export function ImageCardShell(props: { image: React.ReactNode; text: React.ReactNode }) { - const { shell, image, text } = imageCard(); +export function ImageCardShell({ image, text }: { image: React.ReactNode; text: React.ReactNode }) { + const { shell, image: imageSlot, text: textSlot } = imageCard(); return (
-
{props.image}
-
{props.text}
+
{image}
+
{text}
); } @@ -174,13 +174,13 @@ export function ImageCardShell(props: { image: React.ReactNode; text: React.Reac // ImageCard/ImageCard.tsx — Good — Root owns logic; Shell receives region nodes as props import { Image, Text } from "@probo/ui"; -function ImageCardRoot(props: { image: React.ReactNode; text: React.ReactNode }) { +function ImageCardRoot({ image, text }: { image: React.ReactNode; text: React.ReactNode }) { const id = useId(); // state, effects, data wiring … return ( {props.image}} - text={{props.text}} + image={{image}} + text={{text}} /> ); } @@ -192,12 +192,12 @@ function ImageCardRoot(props: { image: React.ReactNode; text: React.ReactNode }) // // Bad — data hooks or state live on Shell -function ImageCardShellWithData(props: { image: React.ReactNode; text: React.ReactNode }) { +function ImageCardShellWithData({ image, text }: { image: React.ReactNode; text: React.ReactNode }) { const data = useQuery(/* … */); // move to Root (or above) return (
- {props.image} - {props.text} + {image} + {text}
); }