docs: teach the effect map, not the deleted submit method (RD-09)
RD-05 through RD-08 replaced the hand-called submit method with createStore's effect map. Two teaching documents still showed the old method in a code block, as the answer to "how does a submit happen?". Both blocks also called a function that no longer exists. Rewrite the code block in ARCHITECTURE.md section 2d and its fp-tea-atomic-design.md counterpart. Both now show the effect map, keyed on the Submitting tag, using the same herregistratie worked example with its optimistic begin/confirm/rollback calls. Both use draftSync.submit, the call the two herregistratie wizards make today. State the two properties the old idiom lacked, since they are the reason for the change: entering a state runs its effect, so a dispatch cannot skip it; and double-submit protection is structural, because the effect fires only on a tag transition. Add one sentence on the Seed exemption: a mount or restore message must not trigger a submit. Fix the one runIfSubmitting() hop in the write walkthrough at ARCHITECTURE.md's line 574. The rest of section 6a stays stale on purpose — RD-31 owns it, including its line citations and dead paths. fp-tea-atomic-design.md's broken pre-monorepo paths stay stale too — RD-32 owns those. Set RD-09's Status to done and its README row to done in the same commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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<WizardState, WizardMsg>(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<n>` 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<n>` 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.
|
||||||
@@ -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-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-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-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-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-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | todo |
|
||||||
| RD-12 | `ActionState` becomes `action` on `BriefState.Loaded` | 11 | | todo |
|
| RD-12 | `ActionState` becomes `action` on `BriefState.Loaded` | 11 | | todo |
|
||||||
|
|||||||
@@ -307,20 +307,42 @@ In the template you don't mutate anything — you send messages:
|
|||||||
### 2d. Side effects (HTTP) without polluting the reducer
|
### 2d. Side effects (HTTP) without polluting the reducer
|
||||||
|
|
||||||
`reduce` is pure — it must not call the network. So how does a submit happen?
|
`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
|
`createStore` (`libs/shared/src/application/store.ts`) takes an optional **effect
|
||||||
sends messages describing the outcome:
|
map**: one handler per state tag, registered next to the reducer, run when the
|
||||||
|
store **enters** that tag:
|
||||||
|
|
||||||
```ts
|
```ts
|
||||||
async runIfSubmitting() {
|
private store = createStore<WizardState, WizardMsg>(initial, reduce, {
|
||||||
if (this.state().tag !== 'Submitting') return;
|
Submitting: async (s, store) => {
|
||||||
this.profile.beginHerregistratie(); // 1. optimistic (see below)
|
this.profile.beginHerregistratie(); // 1. optimistic (see below)
|
||||||
const r = await submitHerregistratie(s.data); // 2. the actual call
|
const r = await this.draftSync.submit({ uren: s.data.uren, documents: s.data.documents }); // 2. the actual call
|
||||||
if (r.ok) { this.dispatch({ tag: 'SubmitConfirmed' }); this.profile.confirmHerregistratie(); }
|
if (r.ok) {
|
||||||
else { this.dispatch({ tag: 'SubmitFailed', error: r.error }); this.profile.rollbackHerregistratie(); }
|
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."**
|
then tell the reducer what happened."**
|
||||||
|
|
||||||
### 2e. Optimistic update + rollback, and shared state across pages
|
### 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
|
→ GET `/api/v1/dashboard-view` → `httpClientFetch` → proxy → backend → back through the
|
||||||
`parseDashboardView(json): Result` trust boundary → `RemoteData<DashboardView>` → rendered.
|
`parseDashboardView(json): Result` trust boundary → `RemoteData<DashboardView>` → 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))
|
([`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
|
→ `runSubmit` — the one try/catch that mints the `Idempotency-Key` and maps RFC-7807
|
||||||
ProblemDetails → string ([`submit.ts`](../../../libs/shared/src/application/submit.ts)) →
|
ProblemDetails → string ([`submit.ts`](../../../libs/shared/src/application/submit.ts)) →
|
||||||
|
|||||||
@@ -332,35 +332,43 @@ sends messages on events — it never mutates:
|
|||||||
That is the loop: `state → template → event → dispatch(Msg) → reduce → new state →
|
That is the loop: `state → template → event → dispatch(Msg) → reduce → new state →
|
||||||
template`.
|
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**
|
`reduce` is pure, so it can't call the network. `createStore`
|
||||||
method. It does the impure work, then dispatches a `Msg` describing what happened — the
|
(`libs/shared/src/application/store.ts`) takes an optional **effect map**: one
|
||||||
result re-enters through the same pure loop:
|
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
|
```ts
|
||||||
private async runIfSubmitting() {
|
private store = createStore<WizardState, WizardMsg>(initial, reduce, {
|
||||||
const s = this.state();
|
Submitting: async (s, store) => {
|
||||||
if (s.tag !== 'Submitting') return;
|
this.profile.beginHerregistratie(); // optimistic flag (shared 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
|
||||||
const r = await submitHerregistratie(s.data); // the actual I/O — a Result
|
if (r.ok) {
|
||||||
if (r.ok) { this.dispatch({ tag: 'SubmitConfirmed' }); this.profile.confirmHerregistratie(); }
|
store.dispatch({ tag: 'SubmitConfirmed' });
|
||||||
else { this.dispatch({ tag: 'SubmitFailed', error: r.error }); this.profile.rollbackHerregistratie(); }
|
this.profile.confirmHerregistratie();
|
||||||
}
|
} else {
|
||||||
|
store.dispatch({ tag: 'SubmitFailed', error: r.error });
|
||||||
|
this.profile.rollbackHerregistratie();
|
||||||
|
}
|
||||||
|
},
|
||||||
|
});
|
||||||
```
|
```
|
||||||
|
|
||||||
The command itself (`src/app/herregistratie/application/submit-herregistratie.ts`)
|
Running the effect **on entry**, instead of a component calling a submit method by
|
||||||
returns a `Result` — success-or-error as a value, never a thrown exception:
|
hand after every `dispatch`, gives two properties the hand-called version lacked:
|
||||||
|
|
||||||
```ts
|
1. **Entering the state runs the effect.** A `dispatch` cannot silently skip it.
|
||||||
export async function submitHerregistratie(data: Valid): Promise<Result<string, void>> {
|
2. **Double-submit protection is structural.** The effect fires only on a tag
|
||||||
await new Promise((r) => setTimeout(r, 800));
|
**transition** (`Editing → Submitting`). A second `Submit` message while the
|
||||||
if (data.uren === 0) return err('Aanvraag afgewezen: geen gewerkte uren geregistreerd.');
|
store is already `Submitting` is a reducer no-op, so no second effect fires.
|
||||||
return ok(undefined);
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
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
|
thing, then say what happened."** Incoming effects (an arriving HTTP value, a
|
||||||
server-owned config) are wired with `effect()` and `untracked()` so the dispatch
|
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
|
doesn't loop on its own write — see the BRP prefill and policy-threshold effects in
|
||||||
|
|||||||
Reference in New Issue
Block a user