diff --git a/docs/project/readable-codebase/RD-09-docs-effect-idiom.md b/docs/project/readable-codebase/RD-09-docs-effect-idiom.md new file mode 100644 index 0000000..ae05f82 --- /dev/null +++ b/docs/project/readable-codebase/RD-09-docs-effect-idiom.md @@ -0,0 +1,132 @@ +# RD-09 — Update the two documents that teach the deleted submit idiom + +Status: done +Source: PLAN.md 1c (A5, as corrected) + +## Why + +RD-05 through RD-08 replaced the hand-called `runIfSubmitting` with `createStore`'s effect +map, and `grep` now finds the method nowhere in `apps/` or `libs/`. Two teaching documents +still present it as the house idiom, in a code block, as the answer to "how does a submit +happen?". + +Both blocks are doubly stale: they teach a deleted idiom **and** call +`submitHerregistratie(s.data)`, a function that no longer exists either. + +A document that teaches a deleted idiom is worse than no document — a reader who follows it +reintroduces the silent-failure bug RD-06 fixed. + +## Read first + +- `libs/shared/src/application/store.ts` — the effect map, the trigger rule, the `Seed` + exemption, and the "must never throw" contract. This is the new idiom to document. +- Any migrated call site, as a worked example — `besluit-form.component.ts` is the smallest. +- `docs/reference/architecture/ARCHITECTURE.md` §2d (the block at 306-322) and the write + walkthrough mention at :574. +- `docs/reference/fp-tea-atomic-design.md` (the block at 336-350). + +## Decisions (pre-made, don't relitigate) + +1. **Scope is exactly two documents.** An earlier version of this plan claimed + `plop-templates/form-machine.hbs` generates `runIfSubmitting` and that + `.claude/skills/form-machine/SKILL.md` teaches it. **Both are false**, verified by grep: + the plop template is machine-only (74 lines, no `@Component`), and the skill never mentions + it. Do not "fix" either file. If you find yourself editing a `.hbs` or a `SKILL.md`, stop. + +2. **Rewrite the code block, do not delete the section.** §2d ("Side effects (HTTP) without + polluting the reducer") and its fp-tea counterpart answer a real question and answer it + well. The answer changed; the question did not. + +3. **The new block shows the effect map.** Same worked example (a herregistratie submit with + its optimistic `begin`/`confirm`/`rollback`), expressed as a registration: + + ```ts + private store = createStore(initial, reduce, { + Submitting: async (s, store) => { … }, + }); + ``` + + Keep the numbered inline comments — they are what makes the block teachable. + +4. **State the two properties the old idiom lacked**, because they are the reason for the + change and a reader should see them: + - Entering the state runs the effect. A `dispatch` cannot silently skip it. + - Double-submit protection is structural: the effect fires only on a tag **transition**, so + a second `Submit` while already submitting is a reducer no-op and fires nothing. + +5. **Document the `Seed` exemption where the idiom is taught**, in one sentence: a mount or + restore message must not trigger a submit. That is the non-obvious part of the rule, and + `store.ts` explains it in full for anyone who needs more. + +6. **Fix `ARCHITECTURE.md:574`'s write walkthrough** too — it opens with `runIfSubmitting() +(§2d) → createSubmitChangeRequest`. Replace the first hop with the effect map. Leave the + rest of §6a alone: **RD-31 rewrites that section**, including its stale `L` citations and + two dead file paths. Touch only the `runIfSubmitting` hop here. + +7. **Drop the `submitHerregistratie(...)` call from both blocks.** That module no longer + exists (`submit-herregistratie.ts` was deleted). Use `draftSync.submit(...)`, which is what + the two herregistratie wizards actually call today. + +## Files + +- `docs/reference/architecture/ARCHITECTURE.md` (§2d block, and the one hop at :574) +- `docs/reference/fp-tea-atomic-design.md` (the block at 336-350) + +No code files. No `.hbs`. No `SKILL.md`. + +## Steps + +1. Rewrite `ARCHITECTURE.md` §2d's code block per decisions 3-5. +2. Fix the first hop of the write walkthrough at :574 per decision 6. +3. Rewrite the equivalent block in `fp-tea-atomic-design.md` per decisions 3-5 and 7. +4. Update this ticket's `Status:` to `done` and the README's RD-09 row to `done`. +5. Commit all of it together. + +## Acceptance criteria + +The deleted idiom is no longer taught anywhere: + +```bash +grep -rn "runIfSubmitting\|runIfIndienen" docs/ apps/ libs/ .claude/ # MUST return nothing +grep -rn "submitHerregistratie" docs/ # MUST return nothing +``` + +Every code identifier the new blocks name must exist: + +```bash +grep -n "StoreEffects\|effects?" libs/shared/src/application/store.ts # the API is real +grep -rn "draftSync.submit" apps/ssp | head -3 # the call is real +``` + +```bash +npm run ci # exits 0 +``` + +## Verification + +`npm run ci`. This ticket touches only two Markdown files under `docs/reference/`, so +`--full` is not required — neither is globbed by Storybook. + +Read both rewritten blocks end to end and check a reader could follow them without opening +`store.ts`. That is the actual acceptance test for a teaching document, and no command +checks it. + +## Out of scope + +- The rest of `ARCHITECTURE.md` §6a — its stale `L` line citations, its two non-existent + file paths, and its post-RD-16 `parseDashboardView` claim. **RD-31.** +- `fp-tea-atomic-design.md`'s 11 broken pre-monorepo `src/app/…` paths and its broken anchor. + **RD-32.** +- `plop-templates/form-machine.hbs` and `.claude/skills/form-machine/SKILL.md` (decision 1). +- CLAUDE.md. **RD-33.** + +## Risks + +- **Do not chase the other staleness in these two files.** Both are also targets of RD-31 and + RD-32. Editing their line citations or paths here creates a conflict with those tickets for + no gain. Change only what teaches the submit idiom. +- **`fp-tea-atomic-design.md`'s surrounding paths are already broken** (they use the + pre-monorepo `src/app/…` prefix). Leaving them broken in this commit is correct — RD-32 owns + them. Do not half-fix the file. +- **Prettier formats Markdown.** Run `npx prettier --write` on both files, or `format:check` + fails. diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md index e5f0dca..ce5be1a 100644 --- a/docs/project/readable-codebase/README.md +++ b/docs/project/readable-codebase/README.md @@ -103,7 +103,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di | RD-06 | **Bug fix:** 2 single-step forms to the effect map + retry affordance | 05 | yes | done | | RD-07 | Add `Primary` to the 3 wizard machines + specs | 05 | | done | | RD-08 | Migrate the 3 wizards to the effect map + `Primary` | 07 | yes | done | -| RD-09 | **Docs + generator:** `form-machine.hbs`, ARCHITECTURE, fp-tea, skill | 08 | | todo | +| RD-09 | **Docs + generator:** `form-machine.hbs`, ARCHITECTURE, fp-tea, skill | 08 | | done | | RD-10 | `WizardStatus` to a payload-carrying `WizardPhase` | 08 | yes | todo | | RD-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | todo | | RD-12 | `ActionState` becomes `action` on `BriefState.Loaded` | 11 | | todo | diff --git a/docs/reference/architecture/ARCHITECTURE.md b/docs/reference/architecture/ARCHITECTURE.md index fa86ef5..e9186cb 100644 --- a/docs/reference/architecture/ARCHITECTURE.md +++ b/docs/reference/architecture/ARCHITECTURE.md @@ -307,20 +307,42 @@ In the template you don't mutate anything — you send messages: ### 2d. Side effects (HTTP) without polluting the reducer `reduce` is pure — it must not call the network. So how does a submit happen? -The component has a small **command** method that does the impure work and then -sends messages describing the outcome: +`createStore` (`libs/shared/src/application/store.ts`) takes an optional **effect +map**: one handler per state tag, registered next to the reducer, run when the +store **enters** that tag: ```ts -async runIfSubmitting() { - if (this.state().tag !== 'Submitting') return; - this.profile.beginHerregistratie(); // 1. optimistic (see below) - const r = await submitHerregistratie(s.data); // 2. the actual call - if (r.ok) { this.dispatch({ tag: 'SubmitConfirmed' }); this.profile.confirmHerregistratie(); } - else { this.dispatch({ tag: 'SubmitFailed', error: r.error }); this.profile.rollbackHerregistratie(); } -} +private store = createStore(initial, reduce, { + Submitting: async (s, store) => { + this.profile.beginHerregistratie(); // 1. optimistic (see below) + const r = await this.draftSync.submit({ uren: s.data.uren, documents: s.data.documents }); // 2. the actual call + if (r.ok) { + store.dispatch({ tag: 'SubmitConfirmed' }); // 3. tell the reducer what happened + this.profile.confirmHerregistratie(); + } else { + store.dispatch({ tag: 'SubmitFailed', error: r.error }); + this.profile.rollbackHerregistratie(); + } + }, +}); ``` -So the split is: **reducer = "what the new state is", command = "go do the thing, +Two properties follow from "run on entry", and they are the reason this replaced an +earlier idiom where a component called a hand-written submit method by hand after +every `dispatch`: + +1. **Entering the state runs the effect.** A `dispatch` cannot silently skip it — + there is no separate call to forget. +2. **Double-submit protection is structural.** The effect fires only on a tag + **transition** (`Editing → Submitting`). A second `Submit` message while the + store is already in `Submitting` is a reducer no-op, so no second effect fires. + +One message is exempt from this rule: `Seed`, the mount/restore message every +wizard sends on load. A `Seed` transition into `Submitting` (a resumed draft, a +Storybook story) must not trigger a submit, so `createStore` skips the effect for +it — see `store.ts` for the full contract. + +So the split is: **reducer = "what the new state is", effect = "go do the thing, then tell the reducer what happened."** ### 2e. Optimistic update + rollback, and shared state across pages @@ -571,7 +593,7 @@ client.dashboardView() })` → GET `/api/v1/dashboard-view` → `httpClientFetch` → proxy → backend → back through the `parseDashboardView(json): Result` trust boundary → `RemoteData` → rendered. -**A write (change address):** `runIfSubmitting()` (§2d) → `createSubmitChangeRequest` +**A write (change address):** the `Submitting` effect (§2d) → `createSubmitChangeRequest` ([`submit-change-request.ts`](../../../apps/ssp/src/app/registratie/application/submit-change-request.ts)) → `runSubmit` — the one try/catch that mints the `Idempotency-Key` and maps RFC-7807 ProblemDetails → string ([`submit.ts`](../../../libs/shared/src/application/submit.ts)) → diff --git a/docs/reference/fp-tea-atomic-design.md b/docs/reference/fp-tea-atomic-design.md index 08dfa06..610e355 100644 --- a/docs/reference/fp-tea-atomic-design.md +++ b/docs/reference/fp-tea-atomic-design.md @@ -332,35 +332,43 @@ sends messages on events — it never mutates: That is the loop: `state → template → event → dispatch(Msg) → reduce → new state → template`. -### 4d. Effects → a command that dispatches the outcome +### 4d. Effects → the store's effect map dispatches the outcome -`reduce` is pure, so it can't call the network. The component holds a small **command** -method. It does the impure work, then dispatches a `Msg` describing what happened — the -result re-enters through the same pure loop: +`reduce` is pure, so it can't call the network. `createStore` +(`libs/shared/src/application/store.ts`) takes an optional **effect map**: one +handler per state tag, run when the store **enters** that tag. The handler does the +impure work, then dispatches a `Msg` describing what happened — the result re-enters +through the same pure loop: ```ts -private async runIfSubmitting() { - const s = this.state(); - if (s.tag !== 'Submitting') return; - this.profile.beginHerregistratie(); // optimistic flag (shared store) - const r = await submitHerregistratie(s.data); // the actual I/O — a Result - if (r.ok) { this.dispatch({ tag: 'SubmitConfirmed' }); this.profile.confirmHerregistratie(); } - else { this.dispatch({ tag: 'SubmitFailed', error: r.error }); this.profile.rollbackHerregistratie(); } -} +private store = createStore(initial, reduce, { + Submitting: async (s, store) => { + this.profile.beginHerregistratie(); // optimistic flag (shared store) + const r = await this.draftSync.submit({ uren: s.data.uren, documents: s.data.documents }); // the actual I/O — a Result + if (r.ok) { + store.dispatch({ tag: 'SubmitConfirmed' }); + this.profile.confirmHerregistratie(); + } else { + store.dispatch({ tag: 'SubmitFailed', error: r.error }); + this.profile.rollbackHerregistratie(); + } + }, +}); ``` -The command itself (`src/app/herregistratie/application/submit-herregistratie.ts`) -returns a `Result` — success-or-error as a value, never a thrown exception: +Running the effect **on entry**, instead of a component calling a submit method by +hand after every `dispatch`, gives two properties the hand-called version lacked: -```ts -export async function submitHerregistratie(data: Valid): Promise> { - await new Promise((r) => setTimeout(r, 800)); - if (data.uren === 0) return err('Aanvraag afgewezen: geen gewerkte uren geregistreerd.'); - return ok(undefined); -} -``` +1. **Entering the state runs the effect.** A `dispatch` cannot silently skip it. +2. **Double-submit protection is structural.** The effect fires only on a tag + **transition** (`Editing → Submitting`). A second `Submit` message while the + store is already `Submitting` is a reducer no-op, so no second effect fires. -The split, in one line: **reducer = "what the new state is"; command = "go do the +One message is exempt: `Seed`, the mount/restore message every wizard sends on +load. A `Seed` transition into `Submitting` (a resumed draft, a Storybook story) +must not trigger a submit, so `createStore` skips the effect for it. + +The split, in one line: **reducer = "what the new state is"; effect = "go do the thing, then say what happened."** Incoming effects (an arriving HTTP value, a server-owned config) are wired with `effect()` and `untracked()` so the dispatch doesn't loop on its own write — see the BRP prefill and policy-threshold effects in