From b2c0276731dcd058334cc58079e5c939016510d3 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Thu, 27 Aug 2026 16:18:22 -0400 Subject: [PATCH] docs(repo): realign Mosaic docs on the model/controller/view split The docs described a machine/controller/view split with the controller as the Clerk adapter. The UserButton settled on model/controller/view: the model is the only layer that touches Clerk, the controller holds local state, and the machine is one of two ways it can hold that state rather than a layer of its own. Rewrite the flow sections of the architecture reference and the mosaic skill around that, add a models.md for the Clerk-adapter layer, and fix the stale paths the audit turned up (utils/reset.styles.ts, the non-existent sections/ test templates, machine/ vs machines/). --- .../mosaic-docs-model-controller-view.md | 2 + .claude/skills/mosaic/SKILL.md | 61 ++-- .../skills/mosaic/references/controllers.md | 149 +++++--- .claude/skills/mosaic/references/headless.md | 2 +- .claude/skills/mosaic/references/machines.md | 49 ++- .claude/skills/mosaic/references/migration.md | 54 ++- .claude/skills/mosaic/references/models.md | 97 +++++ .../skills/mosaic/references/parity-audit.md | 20 +- .claude/skills/mosaic/references/testing.md | 293 +++++++-------- .claude/skills/mosaic/references/views.md | 113 ++++-- references/mosaic-architecture.md | 345 +++++++++++++----- 11 files changed, 805 insertions(+), 380 deletions(-) create mode 100644 .changeset/mosaic-docs-model-controller-view.md create mode 100644 .claude/skills/mosaic/references/models.md diff --git a/.changeset/mosaic-docs-model-controller-view.md b/.changeset/mosaic-docs-model-controller-view.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-docs-model-controller-view.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.claude/skills/mosaic/SKILL.md b/.claude/skills/mosaic/SKILL.md index 523a341bc07..6a9f74da1ed 100644 --- a/.claude/skills/mosaic/SKILL.md +++ b/.claude/skills/mosaic/SKILL.md @@ -2,13 +2,14 @@ name: mosaic description: >- Work on Mosaic UI: styling a component with StyleX (`stylex.create`, `--cl-*` - tokens, `themeProps`), or building a flow — authoring a state machine - (`setup`, states/guards/`invoke`, wiring to React with `useMachine`/`useActor`/ - `useSelector`), writing the controller (Clerk adapter) or view (rendering) layer, - testing any of those layers, or migrating a legacy / pre-Mosaic component into the - machine / controller / view split. Use when building, styling, debugging, testing, or - migrating anything Mosaic. `references/mosaic-architecture.md` (repo root) holds the - design-system contract; this skill is the how-to layer. + tokens, `themeProps`), or building a flow — writing the model (the Clerk + adapter), the controller (local state, in React state or a state machine: + `setup`, states/guards/`invoke`, wired to React with `useMachine`/`useActor`/ + `useSelector`), or the view (rendering), testing any of those layers, or + migrating a legacy / pre-Mosaic component into the model / controller / view + split. Use when building, styling, debugging, testing, or migrating anything + Mosaic. `references/mosaic-architecture.md` (repo root) holds the design-system + contract; this skill is the how-to layer. --- # Mosaic UI @@ -19,28 +20,45 @@ Two things live under Mosaic, and this skill covers the how-to for both: the styles, `themeProps` emits the part's public identity (the `.cl-` class plus `data-` attrs), and `mergeStyleProps` fuses the two with the consumer's `className`/`style`. -- **Flows** follow a **machine → controller → view** split that keeps Clerk - resource logic out of visual components and makes behavior testable without a - running Clerk app: +- **Flows** follow a **model → controller → view** split — _where the data comes + from_ → _what the user is doing to it_ → _what that looks like_. What crosses + each boundary is plain data: no Clerk resource reaches the controller, no + machine snapshot reaches the view. ```text -machine Pure flow rules: states, events, guards, async invokes, errors. - No React hooks. No Clerk hooks. No Clerk resource objects. +model Clerk adapter: reads Clerk hooks and resources, resolves the + environment, gates permissions, and answers with plain data plus + plain callbacks under an explicit `status`. The only layer that may + import Clerk hooks or call Clerk resource methods. -controller Clerk/data adapter: reads Clerk hooks/resources, injects async - effects into machine context, gates permissions, derives view props. - The only layer that may import Clerk hooks or call resource methods. +controller Local state: what is open, what is in flight, what the view may do + next — held in React state or a state machine, whichever the + interaction's complexity calls for. Wraps the model's callbacks so + an action can report pending, hold the surface still while it runs, + and close on success. No Clerk imports. -view Rendering only: receives a snapshot plus explicit props, renders UI, - sends events. No Clerk imports. No data-fetching. No mutations. +view Rendering: takes plain props and callbacks, renders UI, calls them + back. No Clerk imports. No data-fetching. No machine snapshot. ``` +A machine is **not a fourth layer**, and not a requirement. It is one of the two +ways a controller can hold its state, and picking one is a complexity call: +`useState` for a boolean that never touches async, a machine once the +interaction has an async lifecycle or two values that must change together, and +sometimes both in one controller. Either way the controller returns plain props, +so the view cannot tell and neither can its tests. Criteria and worked +before/afters: `packages/ui/src/mosaic/machine/ADOPTION.md`. + `references/mosaic-architecture.md` (repo root, read by all agents) is the canonical contract for the whole design system — the `--cl-*` tokens, the `.cl-` + `data-` styling API, the CSS build, and the "Flow and data architecture" section that defines the split. Read it for the _what_; this skill is the _how-to_. +`packages/ui/src/mosaic/user-button/` is the fullest worked example of the split +in the repo — model, controller, view, wrapper, types, messages, and a test per +layer. Copy from it. + ## Which reference to read | You are… | Read | @@ -48,13 +66,14 @@ is the _how-to_. | Building on / authoring a headless primitive (`@clerk/headless`) | `references/headless.md` | | Styling a component (tokens, `stylex.create`, `themeProps`, CSS build) | `references/stylex.md` | | Building an enter/exit transition, or any motion that reads as wrong | `references/motion.md` | +| Writing the model (the Clerk adapter, `status`, permissions) | `references/models.md` | +| Writing the controller (local state, pending, action wrapping) | `references/controllers.md` | | Authoring or debugging a state machine, or wiring one to React | `references/machines.md` → in-tree `machine/README.md` | -| Writing the controller (Clerk adapter, permissions, revalidate) | `references/controllers.md` | -| Writing the view (rendering a snapshot, sending events) | `references/views.md` | -| Testing a machine, controller, or view | `references/testing.md` | +| Writing the view (rendering plain props) | `references/views.md` | +| Testing a model, controller, or view | `references/testing.md` | | Migrating a legacy component into Mosaic (the end-to-end workflow) | `references/migration.md` | | Running the parity audit that guards a migration | `references/parity-audit.md` | The migration workflow (`migration.md`) ties the flow references together: it -treats the legacy component as the spec and drives you through the machine, +treats the legacy component as the spec and drives you through the model, controller, and view layers, then verifies parity with `parity-audit.md`. diff --git a/.claude/skills/mosaic/references/controllers.md b/.claude/skills/mosaic/references/controllers.md index 75dd3166696..44ca61fe4b0 100644 --- a/.claude/skills/mosaic/references/controllers.md +++ b/.claude/skills/mosaic/references/controllers.md @@ -1,61 +1,128 @@ # Controllers -The controller is the adapter from Clerk resources into machine context and view -props. It is the **only layer in a Mosaic flow that may import Clerk hooks or -call Clerk resource methods** — the machine (`machines.md`) and view -(`views.md`) stay Clerk-free so they remain testable in plain JS. +The controller sits between the view and the outside world. It holds the **local +state** — what is open, what is in flight, what the view may do next — and wraps +the model's actions so the surface can report and survive them. -See `references/mosaic-architecture.md` → "Controllers" for the canonical -example. This file is the practical checklist. +It does **not** touch Clerk. Its effects arrive as injected plain functions: +from a model (`models.md`) when a wrapper composes the two, or as a prop when a +leaf view calls its own controller. That is what lets a controller test run +against a fake object instead of a mocked Clerk. + +Worked examples: + +- `packages/ui/src/mosaic/user-button/user-button.controller.tsx` — wraps a model +- `packages/ui/src/mosaic/user-profile/user-profile-delete-section/user-profile-delete-section.controller.ts` + — takes its one effect as a prop + +See `references/mosaic-architecture.md` → "Controllers" for the layer contract. + +## Which one holds the state + +A controller is not a machine wrapper. It is the layer that owns the +interaction, and it holds that state in whichever tool the interaction's +complexity calls for: + +| The interaction… | Hold it in | +| ------------------------------------------------------------------------ | --------------------------- | +| Is a boolean or a controlled value, no async, nothing else depends on it | `useState` | +| Has an async lifecycle, or two values that must change together | A machine, same file | +| Has a coordinated async core plus some UI-only flags beside it | Both — machine for the core | + +`useUserProfileDeleteSectionController` earns a machine on three counts: the +delete is async, a failure must land back on the previous step with a reason, +and `deleted` is terminal so the dialog must never reopen. `useUserButtonController` +earns one because `open` and `pendingKey` constrain each other and dismissing +must not abandon an in-flight invoke. + +A single `isEditing` boolean earns nothing. A two-state machine with one event +is a boolean spelled long: + +```tsx +export function useSectionController({ onSave }: { onSave: () => void }) { + const [isEditing, setIsEditing] = React.useState(false); + return { + isEditing, + onEdit: () => setIsEditing(true), + onCancel: () => setIsEditing(false), + }; +} +``` + +The choice is invisible from outside — the controller returns plain props either +way, so the view and its tests are unaffected, and swapping one for the other +later is a change to one file. Don't front-load a machine for a flow that has +not earned one; don't leave a coordinated async flow in flag soup because it +started as one boolean. + +`packages/ui/src/mosaic/machine/ADOPTION.md` is the full criteria, including its +"honest boundary" table of what stays `useState` inside a component that does +have a machine. ## Responsibilities -- **Read Clerk state.** Call hooks like `useOrganization()`, `useUser()`, - `useSession()`. This is the only layer allowed to. -- **Inject async effects + live props into machine context.** Pass plain - functions that close over live resources; `useMachine` re-seats context via - `useLayoutEffect` every render, so the machine always reads the latest prop. +- **Hold the interaction state.** In React state or a machine — see "Which one + holds the state" below. +- **Pass the model's `status` through.** The wrapper then branches on one value: ```tsx - const [snapshot, send, actor] = useMachine(deleteOrgMachine, { - context: { - organizationName: organization?.name ?? '', - destroyOrganization: () => organization?.destroy() ?? Promise.resolve(), - }, - }); + if (model.status !== 'ready') { + return { status: model.status }; + } ``` -- **Gate permissions and visibility.** Resolve `session.checkAuthorization(...)` - and collapse loading/permission/empty into an explicit status the wrapper - branches on — not scattered booleans: +- **Wrap actions to drive pending state.** One helper, so every action is wrapped + the same way and only one can be in flight: ```tsx - if (!canRead || !settings.enabled || !organization) return { status: 'hidden' as const }; - if (!firstPageLoaded) return { status: 'loading' as const }; - return { status: 'ready' as const, snapshot, send, canSubmit: actor.can({ type: 'CONFIRM' }) }; + const runAction = (keyFor, fn, closeOnSuccess = false) => + fn + ? (...args) => send({ type: 'RUN', key: keyFor(...args), run: async () => fn(...args), closeOnSuccess }) + : undefined; ``` -- **Own revalidate timing.** Call `data.revalidate()` / `.reload()` after - mutations. Deciding _when_ (fire-and-forget vs awaited) is controller logic, - not view logic. -- **Handle first-page-load empty-state.** Wait for the first page before - deciding to hide a section, so read-only users don't see a hide-then-show - flicker. -- **Derive view props.** Expose `actor.can(...)` results (e.g. `canSubmit`) so - the view never re-implements a machine guard. + An absent model callback stays absent, so a capability the instance does not + offer never reaches the view as a dead affordance. + +- **Decide what closes the surface.** Only the controller knows whether an action + ends the interaction. `onSelectOrganization` closes on success; a switch that + leaves the menu useful does not; a navigation closes _before_ it hands off. +- **Hold the surface still while an action runs.** `setActive` swaps the active + organization while its promise is still in flight, so the live model would + rearrange the popup mid-action. Freeze the model the action started from and + render that until it settles: + + ```tsx + const resolvedModel = context.frozenModel ?? model; + ``` + + Freezing also covers the model dropping `ready → loading` during Clerk's + transitive state, which would otherwise flash the fallback. + +- **Derive view props.** `actor.can(...)` results, a `pendingKey` run through + `useSpinDelay`, a `mode` forced by a capability flag — anything the view would + otherwise have to re-derive. ## Rules -- Pass **plain data and plain functions** into machines and views. Do **not** - pass Clerk resource objects through to the view. -- Branch the wrapper on the controller's `status`, not on raw `isLoaded` flags - sprinkled through the tree. +- **No Clerk imports.** If a controller needs a Clerk fact, the model supplies it + as data. +- **No machine snapshot in the return value.** Return the plain props the view + reads (`open`, `pendingKey`, `isDeleting`, `errorMessage`), never `snapshot` + and `send`. +- Dismissing must not abandon an in-flight effect. Model `open` as context, and + give `OPEN`/`CLOSE` no target in the busy state so they don't leave it: + + ```ts + busy: { + on: { CLOSE: { actions: assign(() => ({ open: false })) } }, + invoke: fromPromise(context => context.run(), { /* … */ }), + } + ``` ## Testing -Test the controller against a **mocked Clerk** for the gating / `hidden` / -empty-state logic. This is the **highest-risk, least-covered layer**: it holds -the Clerk resource semantics that the pure machine tests can't reach. When a -migration loses behavior, it is usually a controller responsibility (revalidate -timing, a permission gate, an empty-state rule) that quietly went missing — -concentrate scrutiny here. +Feed the controller a **fake model object** — a plain literal with +`status: 'ready'` and `vi.fn()` callbacks — and render a tiny harness that +surfaces what it returns. No Clerk mocking. Assert the pending key, what closes +the surface, and that an absent model callback stays absent. See `testing.md`. diff --git a/.claude/skills/mosaic/references/headless.md b/.claude/skills/mosaic/references/headless.md index 5fde047a15a..828902d76af 100644 --- a/.claude/skills/mosaic/references/headless.md +++ b/.claude/skills/mosaic/references/headless.md @@ -244,4 +244,4 @@ if (!element) return null; Tests run in **real Chromium** (vitest browser mode), not jsdom, and include `axe` accessibility assertions. `pnpm test` in `packages/headless`. See `testing.md` for the Mosaic flow-layer testing model (a different concern — that -covers machines/controllers/views, not these primitives). +covers models/controllers/views, not these primitives). diff --git a/.claude/skills/mosaic/references/machines.md b/.claude/skills/mosaic/references/machines.md index dcb7d125d9f..252055d9165 100644 --- a/.claude/skills/mosaic/references/machines.md +++ b/.claude/skills/mosaic/references/machines.md @@ -1,7 +1,21 @@ # Machines +A machine is **not a layer**. It is one of the two ways a controller +(`controllers.md`) can hold its state — the one to reach for when the +interaction has an async lifecycle, an error path back to a previous step, a +terminal state that must never reopen, or two values that must change together. +It is declared in the controller's own file — there is no `*.machine.ts` in a +feature — and the controller is the only thing that sends to it. + +A flow without those properties does not need one: `useState` in the controller +is the right answer for a boolean that never touches async, and a controller can +run a machine for its coordinated core with `useState` flags beside it. The view +cannot tell the difference either way. `machine/ADOPTION.md` is the judgement +call, with real before/after migrations and an "honest boundary" table of what +stays `useState`. + The machine runtime is documented **next to the code**, and that in-tree doc is -the source of truth (it's updated in the same diff when the runtime changes and +the source of truth (it is updated in the same diff when the runtime changes and is readable by every tool, not just Claude Code). Read: - **`packages/ui/src/mosaic/machine/README.md`** — the mental model (state / @@ -10,9 +24,32 @@ is readable by every tool, not just Claude Code). Read: glance (`assign`, `invoke`, `guard`, `always`, `entry`/`exit`, `final`, `mockActor`, `useActor`, `useSelector`, `recheck()`). - **`packages/ui/src/mosaic/machine/ADOPTION.md`** — when a flow is worth a - machine and when it isn't, with real before/after migrations. + machine and when it isn't. + +Two directories one letter apart: `machine/` is the runtime, `machines/` holds +standalone machines and the shared `__tests__/test-utils.ts`. A feature's own +machine goes in its `*.controller.tsx`, not in either. + +## Injected effects + +A machine never calls Clerk. The effect it invokes arrives through context as a +plain function, seated by the controller — `useMachine` re-seats context via +`useLayoutEffect` every render, so the machine always invokes the latest one: + +```ts +deleting: { + invoke: fromPromise(context => context.deleteAccount(), { + onDone: 'deleted', + onError: { + target: 'confirming', + actions: assign((_, event) => ({ + errorMessage: event.error instanceof Error ? event.error.message : 'Something went wrong.', + })), + }, + }), +} +``` -The machine is the pure flow layer: states, events, guards, async `invoke`, -errors — no React hooks, no Clerk. To wire it to Clerk data see `controllers.md`; -to render its snapshot see `views.md`; to test it see `testing.md`; to migrate a -legacy component into this pattern see `migration.md`. +To wire it to Clerk data see `models.md`; to render its state see `views.md`; to +test it see `testing.md`; to migrate a legacy component into this pattern see +`migration.md`. diff --git a/.claude/skills/mosaic/references/migration.md b/.claude/skills/mosaic/references/migration.md index 4339b05c6cb..1450b616fe6 100644 --- a/.claude/skills/mosaic/references/migration.md +++ b/.claude/skills/mosaic/references/migration.md @@ -1,8 +1,10 @@ # Migrating a component into Mosaic Migrating a legacy component means taking logic that was fused into one file and -pulling it apart into the machine / controller / view layers (see the skill +pulling it apart into the model / controller / view layers (see the skill overview and `references/mosaic-architecture.md` → "Flow and data architecture"). +`packages/ui/src/mosaic/user-button/` is the fullest worked example of the +finished shape. **The core risk this workflow exists to manage:** a legacy component fuses rendering, data-fetching, and flow logic into one blob. Splitting it three ways @@ -12,9 +14,9 @@ surfaces as a failing test or a type error. It only surfaces when someone diffs old against new. So the spine of this workflow is **treat the legacy component as the spec, and prove the new layers cover every line of it.** -Do not write the machine first and then ask "did I get everything?" — that means -proving a negative. Invert it: enumerate the legacy behavior first, then make -each layer account for a specific row. +Do not write the new layers first and then ask "did I get everything?" — that +means proving a negative. Invert it: enumerate the legacy behavior first, then +make each layer account for a specific row. --- @@ -49,22 +51,38 @@ label. This list is finite and is the contract the migration must satisfy. Assign each inventory row to exactly one layer. A row with no home is a behavior you are about to drop. -- Flow rules (states, events, guards, async `invoke`, error transitions) → - **machine** (`machines.md`). - Clerk reads, mutations, permission gating, revalidate timing, first-page-load - empty-state → **controller** (`controllers.md`). -- Rendering, labels, derived booleans → **view** (`views.md`). + empty-state, capability flags → **model** (`models.md`). +- Interaction state: what is open, what is in flight, what closes the surface, + and the flow rules behind it → **controller** (`controllers.md`). Whether it + holds that in `useState` or a machine is a Phase 3 decision, not a Phase 2 one + — the inventory rows are the same either way. +- Rendering and labels → **view** (`views.md`). + +Two rows deserve extra care because they have no obvious home: + +- **A capability the instance lacks** belongs in the model, expressed by omitting + the callback — not as a `disabled` prop the view has to interpret. +- **Pure derivation** (slot layout, ordering a consumer's list) belongs in + `*.layout.ts` / `*.utils.ts` beside the view, where it gets its own test. ## Phase 3 — Implement and test per layer -File shape: `.machine.ts` · `.controller.tsx` · -`.view.tsx` · `.tsx` (thin composition wrapper). +File shape: `.model.tsx` · `.controller.tsx` · +`.view.tsx` · `.tsx` (composition wrapper), plus +`.types.ts` for the data contract the model and view share, and +`.messages.ts` for the strings. + +Only now decide how the controller holds its state: the inventory tells you +whether the interaction has the async lifecycle and mutually-constraining values +that earn a machine, or whether it is `useState` (`controllers.md` → "Which one +holds the state"). Each layer is testable in isolation — that isolation is what makes the migration -verifiable. Follow the testing recipe in each layer's reference: machine via -`createActor`/`mockActor` (no React, no Clerk), view via a fake snapshot + fake -`send`, controller against a mocked Clerk. The controller is the highest-risk, -least-covered layer — concentrate scrutiny there. +verifiable. Follow the recipes in `testing.md`: the model against a mocked Clerk, +the controller against a fake model object, the view against plain props. The +**model** is the highest-risk, least-covered layer — concentrate scrutiny there. +Finish with one `*.integration.test.tsx` proving the layers compose. ## Phase 4 — Verify parity (the confidence step) @@ -72,15 +90,15 @@ Machine and view tests only cover branches you remembered to write. To catch the ones you didn't, run an automated diff of legacy against new. Launch an **Explore subagent** with the prompt in `parity-audit.md`. Give it the -legacy file paths and the new machine/controller/view paths. It returns a table +legacy file paths and the new model/controller/view paths. It returns a table classifying every legacy behavior as: - **Migrated** — points at a specific state / transition / context field. - **Deliberately changed** — names the new behavior and why (e.g. infinite scroll → "Load more" button). -- **Deferred** — a real tracked ticket, **not** a `// TODO` buried in a machine - file. A buried TODO is invisible at review time; that is exactly how the - domains-section migration shipped three regressions. +- **Deferred** — a real tracked ticket, **not** a `// TODO` buried in a + controller or model. A buried TODO is invisible at review time; that is exactly + how the domains-section migration shipped three regressions. Every inventory row from Phase 1 must land in exactly one bucket. The table is **ephemeral**: it drives the work and the PR discussion, then is discarded. It is diff --git a/.claude/skills/mosaic/references/models.md b/.claude/skills/mosaic/references/models.md new file mode 100644 index 00000000000..ff9fca17bb0 --- /dev/null +++ b/.claude/skills/mosaic/references/models.md @@ -0,0 +1,97 @@ +# Models + +The model is the adapter from Clerk into plain data. It is the **only layer in a +Mosaic flow that may import Clerk hooks or call Clerk resource methods** — the +controller (`controllers.md`) and the view (`views.md`) stay Clerk-free, which is +what makes both testable without a Clerk fixture. + +Worked example: `packages/ui/src/mosaic/user-button/user-button.model.tsx`. See +`references/mosaic-architecture.md` → "Models" for the layer contract. + +## Shape + +A discriminated union on `status`, so consumers branch on one value instead of a +scatter of `isLoaded` flags: + +```tsx +export type UserButtonModel = + | { status: 'loading' } + | { status: 'hidden' } + | (UserButtonData & UserButtonCallbacks & { status: 'ready'; organizationsEnabled: boolean }); +``` + +`hidden` is an **answer**, not an absence: signed out is settled, so the wrapper +drops the fallback instead of holding the space open. Keep the two apart. + +## Responsibilities + +- **Read Clerk state.** `useUser()`, `useSession()`, `useOrganization()`, + `useClerk()`, plus `useMosaicEnvironment()` / `useMosaicRouter()`. +- **Wait for everything that affects layout before answering `ready`.** Answering + early and filling in later is a reshuffle the user sees: + + ```tsx + if (!isUserLoaded || !isSessionLoaded || !isOrgLoaded || !environment) { + return { status: 'loading' }; + } + ``` + + A list that only fills part of a surface is the exception — expose it as a + `…Loading` flag in the data so the surface renders and that region stands in. + +- **Map resources to plain rows.** `toMembership(organization)`, + `toSession(id, user)` — the shapes in `*.types.ts`, never the resource itself. +- **Gate permissions and capability.** `session.checkAuthorization(...)`, + `user.createOrganizationEnabled`, `authConfig.singleSessionMode`. Express the + result by **omitting the callback**, not by a disabled flag: + + ```tsx + onInviteMembers: canInviteMembers ? () => clerk.openInviteMembers({ getContainer }) : undefined, + onSignOutAll: singleSessionMode ? undefined : () => clerk.signOut(), + ``` + + The view hides the affordance an absent callback drives, so the model never has + to describe UI. + +- **Own revalidate timing.** Call `.revalidate()` / `.reload()` after a mutation, + from inside the callback that made it. Deciding _when_ is model logic: + + ```tsx + onAcceptInvitation: async invitationId => { + try { + await invitationData.find(i => i.id === invitationId)?.accept(); + } finally { + // Always revalidate — a failed accept might be stale state. allSettled never throws, + // so a failed revalidate doesn't look like a failed accept. + await Promise.allSettled([userInvitations.revalidate?.(), userMemberships.revalidate?.()]); + } + }, + ``` + +- **Resolve navigation vs modal.** A consumer's URL is the whole opt-in; type the + pair so it cannot contradict itself: + + ```ts + type UserProfileMode = + | { userProfileUrl: string; userProfileMode?: 'navigation' } + | { userProfileUrl?: never; userProfileMode?: 'modal' }; + ``` + +## Rules + +- Return **plain data and plain functions**. A callback takes ids (`sessionId`, + `organizationId`), never a resource. +- An async callback returns its promise — the controller drives pending state off + it. Navigation callbacks stay fire-and-forget. +- No local UI state. What is open and what is in flight belong to the controller. +- No React state machinery beyond the Clerk hooks themselves; the model is a + derivation of what Clerk currently says. + +## Testing + +Mock `@clerk/shared/react` with mutable module-level vars reset in `beforeEach`, +then `renderHook` the model and assert its output. This is the **highest-risk, +least-covered layer**: it holds the Clerk resource semantics no other test can +reach. When a migration loses behavior, it is usually a model responsibility +(revalidate timing, a permission gate, an empty-state rule) that quietly went +missing — concentrate scrutiny here. See `testing.md`. diff --git a/.claude/skills/mosaic/references/parity-audit.md b/.claude/skills/mosaic/references/parity-audit.md index 3949584bd0e..32f446e6fb3 100644 --- a/.claude/skills/mosaic/references/parity-audit.md +++ b/.claude/skills/mosaic/references/parity-audit.md @@ -1,7 +1,7 @@ # Parity audit — Phase 4 reference The parity audit is the confidence step of a Mosaic migration. It diffs the -legacy component against the new machine/controller/view and classifies every +legacy component against the new model/controller/view and classifies every legacy behavior, so behavior that lived implicitly in the old blob can't be silently dropped. @@ -19,8 +19,8 @@ In the repo at , audit a Mosaic migration for behavioral parity. LEGACY component files (the spec — behavior must be preserved or consciously changed): -NEW Mosaic files (machine / controller / view / wrapper): - +NEW Mosaic files (model / controller / view / wrapper): + Enumerate EVERY behavior in the legacy files — each: effect, guard, error path, empty/loading state, permission gate, revalidate/reload call, reset-on-close, @@ -40,9 +40,9 @@ by a `// TODO` (these are the regressions at risk of shipping). ## Output table format -| Legacy behavior | Layer it should live in | Status | Evidence (legacy → new) | -| ------------------------------------------------------- | ----------------------- | --------------------------- | --------------------------------------------------------------------------------- | -| Per-field error mapping via `handleError(err, [field])` | machine/view | Deferred (only a `// TODO`) | `AddDomainForm.tsx` handleError → `*-add-verify.machine.ts` single `errorMessage` | +| Legacy behavior | Layer it should live in | Status | Evidence (legacy → new) | +| ------------------------------------------------------- | ----------------------- | --------------------------- | -------------------------------------------------------------------------- | +| Per-field error mapping via `handleError(err, [field])` | controller/view | Deferred (only a `// TODO`) | `AddDomainForm.tsx` handleError → `*.controller.tsx` single `errorMessage` | `Status` is one of: **Migrated** · **Deliberately changed** · **Deferred**. @@ -56,14 +56,14 @@ rg -n 'useEffect|handleError|card\.setError|useReverification|revalidate|.{machine,controller,view}.test.*`. - -Canonical templates to copy from: - -- `packages/ui/src/mosaic/sections/__tests__/delete-organization.machine.test.ts` -- `packages/ui/src/mosaic/sections/__tests__/delete-organization.controller.test.tsx` -- `packages/ui/src/mosaic/sections/__tests__/delete-organization.view.test.tsx` - -Shared helpers live in -`packages/ui/src/mosaic/machines/__tests__/test-utils.ts`: `deferred()` (a -promise whose `resolve`/`reject` are captured so you can assert an in-flight -state before settling it), `tick()` (flush microtasks so an `invoke`'s -`onDone`/`onError` runs), and `noop` (a no-op async dep). +A flow is three layers (`models.md` · `controllers.md` · `views.md`), and each is +tested in isolation. That isolation is the point: **only the model test mocks +Clerk**. The controller runs against a fake model object, and the view runs +against plain props. + +Tests are Vitest + React Testing Library, co-located in `__tests__/` next to the +feature and named for the layer they cover. + +`packages/ui/src/mosaic/user-button/__tests__/` is the canonical set to copy from: + +| File | Covers | +| ---------------------------------- | ----------------------------------------------------------- | +| `user-button.model.test.tsx` | Clerk → plain data. The only file that mocks Clerk. | +| `user-button.controller.test.tsx` | Fake model → view props. Pending, closing, gating. | +| `user-button.view.test.tsx` | Plain props → DOM. What each surface carries and withholds. | +| `user-button.test.tsx` | The wrapper's branching, with all three layers mocked out. | +| `user-button.integration.test.tsx` | Real layers against a mocked Clerk, driving the real DOM. | +| `user-button.layout.test.ts` | Pure derivation, no React. | +| `user-button.utils.test.ts` | Pure helpers, no React. | + +Shared helpers live in `packages/ui/src/mosaic/machines/__tests__/test-utils.ts`: +`deferred()` (a promise whose `resolve`/`reject` are captured, so you can +assert an in-flight state before settling it), `tick()` (flush microtasks so an +`invoke`'s `onDone`/`onError` runs), and `noop`. Run one file with `pnpm --filter @clerk/ui test `. --- -## Machine — plain JS, no React +## Model — mock Clerk, assert the plain data -Drive the actor directly. Inject async deps through `context` as `vi.fn()`s, send -events, assert `getSnapshot().value` / `.context`. Await `tick()` when an -`invoke` needs to settle. +Mock `@clerk/shared/react` with mutable module-level vars reset in `beforeEach`, +so a test opts into a condition by setting one flag rather than rewriting the +mock. Build the environment **per read**, not once, or a flag set inside a test +won't be seen: -```ts -import { describe, expect, it, vi } from 'vitest'; -import { createActor } from '../../machine/createActor'; -import { deleteOrgMachine } from '../delete-organization.machine'; - -const tick = () => new Promise(resolve => setTimeout(resolve, 0)); - -it('invokes the injected delete function after a valid confirmation', async () => { - const destroyOrganization = vi.fn(() => Promise.resolve()); - const actor = createActor(deleteOrgMachine, { - context: { organizationName: 'Acme Inc', destroyOrganization }, - }); - - actor.start(); - actor.send({ type: 'OPEN' }); - actor.send({ type: 'TYPE_CONFIRMATION', value: 'Acme Inc' }); - actor.send({ type: 'CONFIRM' }); - - expect(actor.getSnapshot().value).toBe('deleting'); - expect(destroyOrganization).toHaveBeenCalledTimes(1); +```tsx +let isUserLoaded: boolean; +let user: FakeUser | null; +let singleSessionMode: boolean; +let environmentHydrated: boolean; + +// Built per read rather than once, so a test setting any of the flags above is answered by it. +function environment() { + return environmentHydrated + ? { displayConfig: { branded }, authConfig: { singleSessionMode }, organizationSettings: { enabled: true } } + : null; +} - await tick(); - expect(actor.getSnapshot().value).toBe('deleted'); +vi.mock('@clerk/shared/react', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + useUser: () => ({ isLoaded: isUserLoaded, user }), + useSession: () => ({ isLoaded: isSessionLoaded, session }), + // Stubbed with a sentinel so the assertion is that this exact function reaches Clerk, + // rather than that some function did. + usePortalRoot: () => getContainer, + useClerk: () => ({ setActive, signOut, buildSignInUrl: () => '/sign-in', __internal_environment: environment() }), + }; }); ``` -For a transient/unreachable state you can't easily send your way to (mid-mutation, -a guard-gated wizard step), teleport in with `mockActor(machine, { value, -context })` — see `machine/README.md` → "Testing & docs". +Stub at the **helper the model actually reads through**, not one layer deeper — +the paginated lists come from `useOrganizationListInView`, so that is the fetch +boundary to mock. -## Controller — mock Clerk, assert `status` +Then `renderHook(() => useUserButtonModel(options))` and assert: -The controller is the only layer that touches Clerk, so this is the only test -that mocks it. Mock `@clerk/shared/react` with mutable module-level vars reset in -`beforeEach`, render a tiny harness that surfaces `controller.status` (and the -snapshot once ready), and assert the loading / hidden / ready gating. Use -`deferred()` + `act()` to hold an async effect open and observe the in-flight -state. +- `status` is `loading` until every load flag that affects layout has answered, + and `hidden` — not `loading` — once Clerk says nobody is signed in. +- A capability the instance lacks makes its callback `undefined` + (`singleSessionMode` → no `onSignOutAll`; no permission → no `onInviteMembers`). +- Calling a callback reaches Clerk with the right arguments, and revalidates + after the mutation. -```tsx -import { act, fireEvent, render, screen } from '@testing-library/react'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { deferred } from '../../machines/__tests__/test-utils'; -import { useDeleteOrganizationController } from '../delete-organization.controller'; +These are the assertions no other layer can make. When a migration loses +behavior, it is almost always here. -let destroy: ReturnType; -let revalidate: ReturnType; -let checkAuthorization: ReturnType; -let isLoaded: boolean; -let organization: { id: string; name: string; destroy: () => Promise; adminDeleteEnabled: boolean } | null; +## Controller — fake model, no Clerk -vi.mock('@clerk/shared/react', async importOriginal => { - const actual = await importOriginal(); +Build a `ready()` factory that returns a plain model literal with `vi.fn()` +callbacks, render a harness that surfaces what the controller returns, and drive +it. There is no Clerk mocking in this file at all: + +```tsx +function ready(overrides: Partial = {}): UserButtonReadyModel { return { - ...actual, - useOrganization: () => ({ isLoaded, organization, membership: null }), - useOrganizationList: () => ({ userMemberships: { revalidate } }), - useSession: () => ({ isLoaded: true, session: { id: 'sess_1', checkAuthorization } }), + status: 'ready', + organizationsEnabled: true, + activeSession: { sessionId: 'sess_1', name: 'Alice Smith', identifier: 'alice@example.com' }, + memberships: [], + additionalSessions: [], + ...overrides, }; -}); - -beforeEach(() => { - destroy = vi.fn(); - revalidate = vi.fn().mockResolvedValue(undefined); - checkAuthorization = vi.fn().mockReturnValue(true); - isLoaded = true; - organization = { id: 'org_1', name: 'Acme Inc', destroy, adminDeleteEnabled: true }; -}); -afterEach(() => vi.clearAllMocks()); +} -function Harness() { - const controller = useDeleteOrganizationController(); - if (controller.status !== 'ready') return {controller.status}; +function Harness({ model, ...options }: { model: UserButtonModel } & UserButtonControllerOptions) { + const c = useUserButtonController(model, options); + if (c.status !== 'ready') return {c.status}; return (
- {controller.snapshot.value} - + {String(c.open)} + {c.pendingKey ?? ''} +
); } -it('is hidden when the user lacks the delete permission', () => { - checkAuthorization.mockReturnValue(false); - render(); - expect(screen.getByTestId('state')).toHaveTextContent('hidden'); -}); +it('runs a model action through the machine and keys the affordance', async () => { + const onSelectOrganization = vi.fn(() => Promise.resolve()); + render(); -it('drives CONFIRM → deleting → resolve → deleted', async () => { - const gate = deferred(); - destroy.mockReturnValue(gate.promise); - render(); - // …open + type + confirm… - await act(async () => gate.resolve()); - expect(revalidate).toHaveBeenCalledTimes(1); + fireEvent.click(screen.getByText('open')); + fireEvent.click(screen.getByText('select-org')); + + expect(onSelectOrganization).toHaveBeenCalledWith('org_1'); + await waitFor(() => expect(screen.getByTestId('pending')).toHaveTextContent('select-org:org_1')); + + await act(async () => { + await tick(); + }); + expect(screen.getByTestId('open')).toHaveTextContent('false'); }); ``` -Assert controller responsibilities here that the machine can't see: the `hidden` -gate from `checkAuthorization`, `loading` until Clerk is loaded, and that -`revalidate` fires (only) on success. +Assert what only the controller decides: `loading`/`hidden` passing through, which +actions close the surface and which leave it open, that a hand-off closes _before_ +it runs, that an absent model callback stays absent, and that a second action +cannot start while one is in flight. Use `deferred()` + `act()` to hold an effect +open and observe the in-flight state. -## View — fake snapshot, no Clerk +For a transient state you can't easily drive to, teleport in with +`mockActor(machine, { value, context })` — see `machine/README.md` → +"Testing & docs". -Build a plain `snapshot` object and pass a `vi.fn()` `send`. Assert what renders -per `snapshot.value` and that interactions send the right event. **Wrap the view -in ``** — it's not a Clerk provider; it supplies the icon-override -context, and wrapping keeps the test tree matching production. No Clerk providers -or fixtures. +## View — plain props, no Clerk, no machine + +Pass plain data and `vi.fn()` callbacks. Assert what renders and that the right +callback fires. **Wrap in ``** — it is not a Clerk provider; it +supplies the icon-override context, and wrapping keeps the test tree matching +production. + +Give the fixture **every** callback by default, so a test opts a surface _out_ of +an affordance rather than having to opt into it — that way "this row is absent" +is an explicit assertion rather than an accident of the fixture. ```tsx -import { fireEvent, render, screen } from '@testing-library/react'; -import { describe, expect, it, vi } from 'vitest'; -import type { Snapshot } from '../../machine/types'; -import { MosaicProvider } from '../../MosaicProvider'; -import type { DeleteOrgContext } from '../delete-organization.machine'; -import { DeleteOrganizationView } from '../delete-organization.view'; - -function snapshot(overrides: Partial> = {}): Snapshot { - return { - value: 'confirming', - status: 'active', - context: { organizationName: 'Acme Inc', confirmationValue: '', destroyOrganization: async () => {}, error: null }, - ...overrides, - }; -} +const alice = { sessionId: 'sess_1', name: 'Alice Smith', identifier: 'alice@example.com' }; -function renderView(snap: Snapshot, send = vi.fn(), canSubmit = false) { +function renderView(overrides: Partial = {}) { + const props = { ...allCallbacks, activeSession: alice, memberships: [], ...overrides }; render( - + , ); - return { send }; + return props; } -it('emits a typed confirmation event from the input', () => { - const { send } = renderView(snapshot()); - fireEvent.change(screen.getByRole('textbox'), { target: { value: 'Acme Inc' } }); - expect(send).toHaveBeenCalledWith({ type: 'TYPE_CONFIRMATION', value: 'Acme Inc' }); +it('omits sign-out-of-all when the instance does not offer it', async () => { + renderView({ onSignOutAll: undefined }); + await userEvent.click(screen.getByRole('button', { name: /open account menu/i })); + expect(screen.queryByText(m.footer.signOutAll)).not.toBeInTheDocument(); }); +``` -it('shows machine errors without Clerk fixtures', () => { - renderView( - snapshot({ - context: { - organizationName: 'Acme Inc', - confirmationValue: 'Acme Inc', - destroyOrganization: async () => {}, - error: 'Delete failed', - }, - }), - ); - expect(screen.getByText('Delete failed')).toBeInTheDocument(); -}); +Because the view is pure rendering, a test can assert "props X render element Y +and clicking Z calls W" for every branch without any of the flow or data +machinery. + +## Wrapper — mock all three layers + +The wrapper's own job is only which status renders what, so mock the model, +controller, and view out and assert the branching: + +```tsx +vi.mock('../user-button.model', () => ({ useUserButtonModel: () => ({ status: 'loading' }) })); +vi.mock('../user-button.controller', () => ({ useUserButtonController: () => controller })); +vi.mock('../user-button.view', () => ({ UserButtonView: () => })); ``` -Pass derived booleans (`canSubmit`, from `actor.can(...)`) as props — the view -never re-implements a machine guard, so a view test never needs the machine. +## Integration — real layers, mocked Clerk + +One file per connected component, proving the layers compose. It mocks Clerk the +same way the model test does, then renders the real wrapper and drives the real +DOM with `userEvent`. It is the only place that catches wiring bugs the isolated +tests each pass: an action that should close the popup but doesn't, a model +callback the controller forgot to wrap, a prop name that drifted between layers. + +Keep it about **composition**, not coverage — the per-layer tests own the +branches. diff --git a/.claude/skills/mosaic/references/views.md b/.claude/skills/mosaic/references/views.md index e930daa2b79..1bc0f536243 100644 --- a/.claude/skills/mosaic/references/views.md +++ b/.claude/skills/mosaic/references/views.md @@ -1,57 +1,90 @@ # Views -The view renders a snapshot and emits events. Nothing else. +The view renders plain props and calls plain callbacks. Nothing else. -- **No Clerk imports.** No data-fetching hooks. No mutation calls. Everything the - view needs arrives as explicit props from the controller (`controllers.md`). -- **Branch on `snapshot.value`, not on context booleans.** The machine models - the states; the view reads them. -- **Take derived booleans from the controller.** `actor.can(...)` results (e.g. - `canSubmit`) are passed in — the view never re-implements a machine guard. +- **No Clerk imports.** No data-fetching hooks. No mutation calls. +- **No machine snapshot.** The controller (`controllers.md`) derives the props + the view branches on — `open`, `pendingKey`, `isDeleting`, `errorMessage` — so + the view never reaches into `snapshot.value` or calls `send`. +- **An absent callback hides the affordance it drives.** The model expresses "the + instance does not offer this" by omitting the callback, so the view's check is + a plain `? :` rather than a capability flag of its own. +- **Take derived booleans from the controller.** `actor.can(...)` results are + passed in — the view never re-implements a machine guard. ```tsx -
send({ type: 'SUBMIT' })}> - send({ type: 'TYPE_NAME', value: event.target.value })} - /> - - Save - -
+export function UserButtonView({ open, onOpenChange, pendingKey, onSignOutAll, ...data }: UserButtonProps) { + return ( + + {onSignOutAll ? ( + + {m.footer.signOutAll} + + ) : null} + + ); +} ``` -A **block** takes the flow's state as props. It owns only what nothing outside -it can use. `Destructive` is the example: it holds the half-typed confirmation -phrase and compares it, while `open`, `isDeleting`, and `errorMessage` come from -the machine, because those are what decide whether the dialog closes or explains -itself. +## Composition + +Two shapes, chosen by whether the slice fetches its own data: + +- A **wrapper composes** model + controller + view, and the view is a pure + function of props (`user-button.tsx`). +- A **leaf view owns its controller** and takes the effect as a prop + (`UserProfileDeleteSectionView` calls `useUserProfileDeleteSectionController` + with its `onDelete`). Still no Clerk — the effect arrives from above. + +## Where the strings live + +Every string a view renders comes from the feature's `*.messages.ts`, shaped the +way `@clerk/i18n` takes a base definition, so localizing is registering a +namespace rather than hunting literals down first. A plural message is its forms; +a parameterized one is its template. Import it as `m` and read through it: + +```tsx +import { fill, plural, userButtonBase as m } from './user-button.messages'; +``` + +## Blocks + +A **block** is a view fragment that owns one piece of state nothing outside it can +use, and takes the rest as props. `blocks/destructive` is the example: it holds +the half-typed confirmation phrase and compares it, while `open`, `isDeleting`, +and `errorMessage` come from the controller, because those are what decide +whether the dialog closes or explains itself. ```tsx send({ type: open ? 'OPEN' : 'CANCEL' })} - trigger={} - title='Delete organization?' - description="All of this organization's data will be permanently deleted." - fieldLabel='Type the organization name below to continue' - confirmationValue={organizationName} - actionLabel='Delete organization' - onDelete={() => send({ type: 'CONFIRM' })} - isDeleting={snapshot.value === 'deleting'} - errorMessage={snapshot.context.errorMessage} + open={isOpen} + onOpenChange={onOpenChange} + trigger={} + title={m.dialogTitle} + confirmationValue={m.fieldPlaceholder} + onDelete={onConfirm} + isDeleting={isDeleting} + errorMessage={errorMessage} /> ``` +## Pure derivation belongs beside the view, not in it + +Which affordance lands in which slot, and how a consumer's `order` array +rearranges a list, are decisions with no React in them. They live in +`*.layout.ts` / `*.utils.ts` and get their own tests — the view calls the result. + ## Testing -Render the view directly with a **fake snapshot and a fake `send`**. No Clerk -providers, no Clerk fixtures. Because the view is pure rendering, a test can -assert "state X renders element Y and clicking Z sends event W" for every branch -of `snapshot.value` without any of the flow or data machinery. +Render the view directly with **plain props and `vi.fn()` callbacks**. No Clerk +providers, no fixtures, no machine. **Wrap in ``** — it is not a +Clerk provider; it supplies the icon-override context, and wrapping keeps the +test tree matching production. See `testing.md`. See `references/mosaic-architecture.md` → "Views" for the layer contract. diff --git a/references/mosaic-architecture.md b/references/mosaic-architecture.md index 528dfeefd51..d00d6427612 100644 --- a/references/mosaic-architecture.md +++ b/references/mosaic-architecture.md @@ -119,7 +119,7 @@ export const Button = React.forwardRef(function `mergeStyleProps` applies its arguments in order — stable class + data attrs, then StyleX atoms, then the consumer's `className` and `style`. Only `style` wins by that ordering: it is an inline style, which outranks any stylesheet rule. A consumer's `className` wins for a different reason — class order inside the attribute has no effect on the cascade, so the consumer's rule wins because the Mosaic sheet is imported into a cascade layer (`@import '@clerk/ui/styles.css' layer(components)`) and an unlayered rule beats any layered one. -`components/reset.styles.ts` holds the per-element resets so a component does not re-declare UA-normalization. +`utils/reset.styles.ts` holds the per-element resets so a component does not re-declare UA-normalization; `utils/typography.styles.ts` and `utils/focus-outline.styles.ts` do the same for the treatments several components share. For the full StyleX authoring rules (token usage, the local `s(n)` spacing helper, the CSS build), see the `mosaic` Claude Code skill's `references/stylex.md`. @@ -131,143 +131,280 @@ Run it with `pnpm build:mosaic` in `packages/ui`. ## Flow and data architecture -Mosaic flow UI follows a **machine → controller → view** split. This keeps Clerk resource logic out of visual components and makes most behavior testable without a running Clerk app. +Mosaic flow UI follows a **model → controller → view** split. Read it as _where the +data comes from_ → _what the user is doing to it_ → _what that looks like_. What +crosses each boundary is plain data: no Clerk resource reaches the controller, and +no machine snapshot reaches the view. ```text -machine - Pure flow rules: states, events, guards, async invokes, errors. - No React hooks. No Clerk hooks. No Clerk resource objects. +model + Clerk adapter. Reads Clerk hooks and resources, resolves the environment, gates + on permissions, and answers with plain data plus plain callbacks under an + explicit `status`. The only layer that may import Clerk hooks or call Clerk + resource methods. controller - Clerk/data adapter: reads Clerk hooks/resources, injects async effects, derives actor-driven view props. - This is the only layer in the flow that may import Clerk hooks or call Clerk resource methods. + Local state. Owns the interaction — what is open, what is in flight, what the + view may do next — held in React state or a state machine, whichever the + interaction's complexity calls for. Wraps the model's callbacks so an action + can report pending, hold the surface still while it runs, and close on + success. No Clerk imports. view - Rendering only: receives a snapshot plus explicit props, renders UI, sends events. - No Clerk imports. No data-fetching hooks. No mutation calls. + Rendering. Receives plain props and callbacks, renders UI, calls them back. + No Clerk imports. No data-fetching. No mutations. No machine snapshot. ``` +A machine is not a layer of its own, and not a requirement. It is one of the two +ways a controller can hold its state, and which one a controller uses is a +complexity call: `useState` for a boolean that never touches async, a machine +once the interaction has an async lifecycle or two values that must change +together. A controller can also do both — a machine for the coordinated subset, +`useState` for the UI-only flags beside it. The choice is invisible from the +outside: the controller returns plain props either way, so the view cannot tell +and neither can its tests. `machine/ADOPTION.md` holds the criteria. + ### File shape -Flow slices should be split by role: +A flow slice is split by role, one file per layer, prefixed with the feature name: + +```text +user-button.model.tsx // Clerk adapter — the only file that imports Clerk +user-button.controller.tsx // local state (React state or a machine) + action wrapping +user-button.view.tsx // rendering only +user-button.tsx // composition wrapper and the public props type +``` + +Supporting files carry the parts that would otherwise bloat those four: ```text -delete-organization.machine.ts // pure state machine -delete-organization.controller.tsx // Clerk/mock adapter + actor wiring -delete-organization.view.tsx // view-only rendering -delete-organization.tsx // thin composition wrapper +user-button.types.ts // the data contract the model and view both agree on +user-button.messages.ts // every string the surface renders, in `@clerk/i18n` shape +user-button.layout.ts // pure derivation (which affordance goes in which slot) +user-button.utils.ts // pure helpers +user-button.styles.ts // `stylex.create` atoms (see the StyleX authoring rules) ``` -The exported component composes the controller and view: +`*.types.ts` is worth calling out: it holds the data contract so that neither the +model nor the view owns it, and the two cannot drift. + +### Composition + +Two shapes, chosen by whether the slice fetches its own data. + +**A wrapper composes the layers** when the slice is a connected component. The +wrapper resolves the model, hands it to the controller, and branches on `status`: ```tsx -export function DeleteOrganization() { - const controller = useDeleteOrganizationController(); - // Render nothing until the controller is ready (mirrors the legacy sections, - // which gate their own visibility and show no skeleton). - if (controller.status !== 'ready') { +export function UserButton(props: UserButtonProps = {}) { + const { renderTriggerLabel, mode, modePriority, fallback, ...options } = props; + const model = useUserButtonModel(options); + const controller = useUserButtonController(model, { mode, modePriority }); + + if (controller.status === 'loading') { + return <>{fallback}; + } + // Signed out is an answer, so the placeholder goes too rather than promising a button. + if (controller.status === 'hidden') { return null; } + const { status: _status, ...viewController } = controller; return ( - ); } ``` -### Machines - -Machines own the flow rules. For destructive confirmation flows, the confirmation input value and the guard live in the machine, not the view block: +**A view owns its controller** when the slice is a leaf that takes its effect as a +prop. There is no model: the Clerk call arrives from whoever renders it. -```ts -export type DeleteOrgEvent = - | { type: 'OPEN' } - | { type: 'TYPE_CONFIRMATION'; value: string } - | { type: 'CONFIRM' } - | { type: 'CANCEL' }; - -CONFIRM: { - target: 'deleting', - guard: context => context.confirmationValue === context.organizationName, +```tsx +export function UserProfileDeleteSectionView({ onDelete }: UserProfileDeleteSectionViewProps) { + const { isOpen, onOpenChange, onConfirm, isDeleting, errorMessage } = useUserProfileDeleteSectionController({ + onDelete, + }); + // …render… } ``` -Async effects are injected through context and invoked by the machine: +Either way the rule holds: the controller never imports Clerk, and its effects +arrive as injected plain functions. -```ts -deleting: { - invoke: fromPromise(ctx => ctx.destroyOrganization(), { - onDone: 'deleted', - onError: { - target: 'confirming', - actions: assign((_, event) => ({ error: String(event.error) })), - }, - }), +### Models + +The model reads Clerk and answers with a discriminated `status`, so every consumer +branches on one value rather than on a scatter of `isLoaded` flags. Every callback +it exposes is a plain function over plain ids — never a Clerk resource: + +```tsx +export type UserButtonModel = + | { status: 'loading' } + | { status: 'hidden' } + | (UserButtonData & UserButtonCallbacks & { status: 'ready'; organizationsEnabled: boolean }); + +export function useUserButtonModel(options?: UserButtonModelOptions): UserButtonModel { + const { isLoaded: isUserLoaded, user } = useUser(); + const { isLoaded: isSessionLoaded, session } = useSession(); + const clerk = useClerk(); + const environment = useMosaicEnvironment(); + + // These all affect layout, so wait for every one and avoid a reshuffle. + if (!isUserLoaded || !isSessionLoaded || !environment) { + return { status: 'loading' }; + } + if (!user || !session) { + return { status: 'hidden' }; + } + + return { + status: 'ready', + activeSession: toSession(session.id, user), + memberships: membershipData.map(m => toMembership(m.organization)), + onSelectOrganization: organizationId => clerk.setActive({ organization: organizationId }), + // Single-session apps cannot hold a second account, so the action is meaningless there. + onSignOutAll: singleSessionMode ? undefined : () => clerk.signOut(), + }; } ``` -Machine tests should use `createActor()` directly. They should not render React and should not require Clerk fixtures. +An action the instance does not offer is `undefined` rather than a disabled flag — +the view hides the affordance it drives, so the model never has to describe UI. ### Controllers -Controllers are the adapter from Clerk resources into machine context and view props. They may call hooks like `useOrganization()` and inject live resource methods: +The controller is the layer between the view and the outside world. It holds the +local state, wraps the model's actions to drive pending state, keeps the surface +stable while an action runs, and closes the surface on the actions that should +close it. + +How it holds that state is a complexity call. `UserButton` earns a machine — an +action is async, `open` and `pendingKey` constrain each other, and dismissing +must not abandon an in-flight invoke — so the machine is declared in the same +file and its context carries the injected effect: ```tsx -export function useDeleteOrganizationController() { - const { isLoaded, organization } = useOrganization(); - const [snapshot, send, actor] = useMachine(deleteOrgMachine, { - context: { - organizationName: organization?.name ?? '', - destroyOrganization: () => organization?.destroy() ?? Promise.resolve(), +const userButtonMachine = createMachine({ + id: 'userButton', + initial: 'idle', + context: { open: false, pendingKey: null, run: () => Promise.resolve(), closeOnSuccess: false }, + states: { + idle: { + on: { + OPEN: { actions: assign(() => ({ open: true })) }, + RUN: { + target: 'busy', + guard: context => context.open, + actions: assign((_, event) => ({ pendingKey: event.key, run: event.run })), + }, + }, }, - }); + // OPEN/CLOSE have no target so they do not leave this state and abandon the invoke. + busy: { + on: { OPEN: { actions: assign(() => ({ open: true })) } }, + invoke: fromPromise(context => context.run(), { onDone: 'idle', onError: 'idle' }), + }, + }, +}); - if (!isLoaded || !organization) { - return { status: 'loading' as const }; +export function useUserButtonController(model: UserButtonModel, options = {}): UserButtonController { + const [{ context }, send] = useMachine(userButtonMachine); + + if (model.status !== 'ready') { + return { status: model.status }; } + const runAction = (key, fn, closeOnSuccess = false) => + fn ? (...args) => send({ type: 'RUN', key: key(...args), run: () => fn(...args), closeOnSuccess }) : undefined; + return { - status: 'ready' as const, - snapshot, - send, - canSubmit: actor.can({ type: 'CONFIRM' }), + status: 'ready', + ...data, + open: context.open, + onOpenChange: next => send(next ? { type: 'OPEN' } : { type: 'CLOSE' }), + pendingKey: context.pendingKey, + onSelectOrganization: runAction(userButtonBusyKeys.selectOrganization, model.onSelectOrganization, true), }; } ``` -Controllers should pass plain data and plain functions into machines. Do not pass Clerk resource objects through to views. +The controller passes the model's `status` through, so the wrapper has one thing to +branch on rather than two. + +A controller whose interaction is a single boolean with no async and no second +value to keep in step is the same layer with `useState` inside it — still no +Clerk, still returning plain props: + +```tsx +export function useSectionController({ onSave }: { onSave: () => void }) { + const [isEditing, setIsEditing] = React.useState(false); + + return { + isEditing, + onEdit: () => setIsEditing(true), + onCancel: () => setIsEditing(false), + onSave: () => { + setIsEditing(false); + onSave(); + }, + }; +} +``` + +Reaching for a machine here would produce a two-state machine with one event, +which is a boolean spelled long. Reaching for `useState` in `UserButton` would +produce the flag soup the machine exists to prevent. `machine/ADOPTION.md` has +the criteria and worked before/afters for the calls in between. ### Views -Views render snapshots and emit events. They receive any derived booleans from the controller, including `actor.can(...)` results, so they do not duplicate machine guards: +Views take plain props and callbacks. They branch on the props the controller +derived — `open`, `pendingKey`, an absent callback — never on a machine snapshot, +so a view test needs neither the machine nor Clerk: ```tsx -export function DeleteOrganizationView({ snapshot, send, canSubmit }: DeleteOrganizationViewProps) { +export function UserButtonView({ open, onOpenChange, pendingKey, onSignOutAll, ...data }: UserButtonProps) { return ( - - Type {snapshot.context.organizationName} to confirm - send({ type: 'TYPE_CONFIRMATION', value: event.target.value })} - /> - {snapshot.context.error ? {snapshot.context.error} : null} - - + + {/* An action the instance does not offer arrives undefined, so the row is simply absent. */} + {onSignOutAll ? ( + + {m.footer.signOutAll} + + ) : null} + ); } ``` -View tests should render the view directly with a fake snapshot and fake `send`. They should not use Clerk providers or Clerk fixtures. +A **block** is a view fragment that owns one piece of state nothing outside it can +use, and takes the rest as props. `blocks/destructive` is the example: it holds the +half-typed confirmation phrase and compares it, while `open`, `isDeleting`, and +`errorMessage` come from the controller, because those are what decide whether the +dialog closes or explains itself. + +### Testing the layers + +Each layer is tested in isolation, and that isolation is the point — the model is +the only test that mocks Clerk, and the view needs no machinery at all. See the +`mosaic` skill's `references/testing.md` for the recipes. + +| Layer | Test file | What it needs | +| ---------- | ------------------------ | ----------------------------------------------- | +| model | `*.model.test.tsx` | Mocked Clerk. The highest-risk layer. | +| controller | `*.controller.test.tsx` | A fake model object. No Clerk. | +| view | `*.view.test.tsx` | Plain props and `vi.fn()` callbacks. | +| wrapper | `*.test.tsx` | All three layers mocked; asserts the branching. | +| whole | `*.integration.test.tsx` | Mocked Clerk, real layers, real DOM. | ## Coexistence with existing system @@ -277,6 +414,7 @@ View tests should render the view directly with a fake snapshot and fake `send`. - **Do not** use Emotion in Mosaic — no `css` prop, no `styled`, no theme callbacks - **Do** export every new component from `styles/index.ts`, or its CSS never ships - **Do** import from `src/mosaic/` directly (no barrel files) inside `packages/ui` +- **Do not** import Clerk hooks or call Clerk resource methods anywhere but a `*.model.tsx` ### What doesn't share @@ -300,23 +438,34 @@ To migrate a component from the old system to Mosaic: 4. Update token references — e.g. `theme.colors.$primary500` → `colorVars['--cl-color-primary']`. 5. Export the component from `styles/index.ts` and run `pnpm build:mosaic`. -The steps above cover the **styling** migration. For **flow** components — where the legacy component also fuses data-fetching and flow logic — splitting that logic into the machine/controller/view layers and verifying no implicit behavior is dropped is its own end-to-end workflow. See the `mosaic` Claude Code skill (`.claude/skills/mosaic/`), in particular its `references/migration.md`. +The steps above cover the **styling** migration. For **flow** components — where the legacy component also fuses data-fetching and interaction state into the same file — splitting that into the model/controller/view layers and verifying no implicit behavior is dropped is its own end-to-end workflow. See the `mosaic` Claude Code skill (`.claude/skills/mosaic/`), in particular its `references/migration.md`. ## Files -| File | Purpose | -| ---------------------------------------------- | -------------------------------------------------------------------- | -| `src/mosaic/tokens.stylex.ts` | `--cl-*` token groups declared with `stylex.defineVars` | -| `src/mosaic/props.ts` | `themeProps`, `mergeStyleProps`, `MosaicComponentProps` | -| `src/mosaic/MosaicProvider.tsx` | Provider for the `icons` prop (per-name glyph overrides) | -| `src/mosaic/icons/overrides.ts` | `MosaicIconOverrides` type + `useMosaicIcons()` context | -| `src/mosaic/icons/registry.tsx` | Built-in glyphs and the `IconName` union | -| `src/mosaic/components/reset.styles.ts` | Per-element UA resets shared by every component | -| `src/mosaic/styles/index.ts` | StyleX-only barrel — the entry the CSS build walks | -| `src/mosaic/machine/` | State-machine runtime (`createMachine`, `createActor`, `useMachine`) | -| `src/mosaic//*.machine.ts` | Pure flow rules for a Mosaic feature | -| `src/mosaic//*.controller.tsx` | Clerk/mock data adapters and actor wiring for a Mosaic feature | -| `src/mosaic//*.view.tsx` | Clerk-free view modules that render snapshots and send events | -| `src/mosaic/components/reset.test.tsx` | Reset specs | -| `src/mosaic/__tests__/MosaicProvider.test.tsx` | Icon-override context specs | -| `src/mosaic/components/button/button.test.tsx` | Component-level slot/state/variant specs | +| File | Purpose | +| ---------------------------------------------- | ------------------------------------------------------------------------- | +| `src/mosaic/tokens.stylex.ts` | `--cl-*` token groups declared with `stylex.defineVars` | +| `src/mosaic/props.ts` | `themeProps`, `mergeStyleProps`, `MosaicComponentProps` | +| `src/mosaic/MosaicProvider.tsx` | Provider for the `icons` prop (per-name glyph overrides) | +| `src/mosaic/icons/overrides.ts` | `MosaicIconOverrides` type + `useMosaicIcons()` context | +| `src/mosaic/icons/registry.tsx` | Built-in glyphs and the `IconName` union | +| `src/mosaic/components/` | One subdirectory per component, and nothing else | +| `src/mosaic/blocks/` | View fragments that own one piece of state of their own (`destructive`) | +| `src/mosaic/utils/*.styles.ts` | Atoms shared across components: `reset`, `typography`, `focus-outline` | +| `src/mosaic/hooks/` | Mosaic-only hooks (`useMosaicEnvironment`, `useMosaicRouter`, …) | +| `src/mosaic/styles/index.ts` | StyleX-only barrel — the entry the CSS build walks | +| `src/mosaic/machine/` | State-machine runtime (`createMachine`, `createActor`, `useMachine`) | +| `src/mosaic/machines/` | Standalone machines and the shared `__tests__/test-utils.ts` | +| `src/mosaic//*.model.tsx` | Clerk adapter — the only file in a feature that may import Clerk | +| `src/mosaic//*.controller.tsx` | Local state and action wrapping; holds the feature's machine | +| `src/mosaic//*.view.tsx` | Clerk-free rendering from plain props | +| `src/mosaic//*.types.ts` | The data contract the model and the view both agree on | +| `src/mosaic//*.messages.ts` | Every string the surface renders, shaped the way `@clerk/i18n` takes them | +| `src/mosaic/utils/reset.test.tsx` | Reset specs | +| `src/mosaic/__tests__/MosaicProvider.test.tsx` | Icon-override context specs | +| `src/mosaic/components/button/button.test.tsx` | Component-level slot/state/variant specs | +| `src/mosaic/user-button/__tests__/` | The canonical per-layer test set to copy from | + +`machine/` is the runtime; `machines/` is machines written with it. The one-letter +difference is easy to misread — a feature's own machine belongs in its +`*.controller.tsx`, not in either directory.