`machine-remote-data.ts` defined a third encoding of an in-flight fetch: `LoadLifecycle`. It had three call sites, all one identical line, and the type was never imported by name. Move the mapping into `remote-data.ts` as `fromLoadLifecycle`, beside its neighbour `fromResource` — a `RemoteData` constructor, not a sixth encoding. The lowercase `loading`/`failed`/`loaded` tags on `BriefState`, `OrgTemplateState` and `StamdataEditorState` existed only because `LoadLifecycle` required them. Now that the constraint is inline and PascalCase, the three machines' load-lifecycle tags become `Loading`, `Failed` and `Loaded` — matching their own PascalCase message tags in the same file. `stamdata-editor.machine.spec.ts` no longer asserts a PascalCase message producing a lowercase state. `BriefStatus` (the letter's draft/submitted/approved/rejected/sent status, parsed off the wire from `BriefViewDto`) is a separate tag family and is untouched — its tag count stays 54 before and after this change. Delete `machine-remote-data.ts` and merge its spec into `remote-data.spec.ts`. Regenerate `behaviour-spec.mdx` (the `machineRemoteData` section heading becomes `fromLoadLifecycle`) and confirm `gen:snippets` reports no drift, since `remote-data.ts` carries a showcase region. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
172 lines
8.4 KiB
Markdown
172 lines
8.4 KiB
Markdown
# RD-11 — Fold the lifecycle projection into `remote-data.ts`, and PascalCase the 3 machines
|
|
|
|
Status: done
|
|
Source: PLAN.md 1b#3
|
|
|
|
## Why
|
|
|
|
`machine-remote-data.ts` is 24 lines defining a **third** encoding of "in flight / ok /
|
|
failed": `LoadLifecycle = { tag: 'loading' } | { tag: 'failed'; reason } | { tag: 'loaded' }`.
|
|
It has three call sites, all the identical line, and `LoadLifecycle` is never imported by
|
|
name anywhere — it is a purely structural constraint.
|
|
|
|
That constraint is the **only** reason three machines carry lowercase state tags while their
|
|
message tags are PascalCase in the same file. `stamdata-editor.machine.spec.ts:61` shows the
|
|
confusion in one line today:
|
|
|
|
```ts
|
|
expect(reduce(seedLoaded(), { tag: 'Loading' })).toEqual({ tag: 'loading' });
|
|
```
|
|
|
|
A PascalCase message producing a lowercase state. Relocate the projection with PascalCase
|
|
keys and the dialect drift resolves itself — no separate renaming pass, and one named concept
|
|
disappears.
|
|
|
|
## Read first
|
|
|
|
- `libs/shared/src/application/machine-remote-data.ts` — all 24 lines
|
|
- `libs/shared/src/application/machine-remote-data.spec.ts` — 20 lines, to be merged
|
|
- `libs/shared/src/application/remote-data.ts` — note `fromResource`, the neighbour and
|
|
precedent for the new function
|
|
- `libs/shared/docs/remote-data.mdx:72` — teaches `s.tag === 'loaded'`, so it must change too
|
|
- `apps/ssp/src/app/brief/domain/brief.ts:68` — **`BriefStatus`. Read this before renaming
|
|
anything.** See decision 4.
|
|
|
|
## Decisions (pre-made, don't relitigate)
|
|
|
|
1. **Relocate, do not simply delete.** The mapping has to exist somewhere, because
|
|
`<app-async>` takes a `RemoteData`. Deleting the module re-inlines a 6-line switch in three
|
|
stores, recreating the duplication WP-31 removed. Move it into `remote-data.ts` as
|
|
`fromLoadLifecycle`, beside `fromResource`, where it reads as what it is: **a `RemoteData`
|
|
constructor, not a sixth encoding.** Keep the `Extract<S, { tag: 'Loaded' }>` Success
|
|
payload so all three call sites stay one line.
|
|
|
|
2. **Key it PascalCase**: `Loading | Failed{reason} | Loaded`. Merge
|
|
`machine-remote-data.spec.ts` into `remote-data.spec.ts` and delete both old files.
|
|
|
|
3. **Rename only the three load-lifecycle tags, and catch all four syntactic forms.** Measured
|
|
counts of the construction form alone (39 across 11 files) understate it. The forms are:
|
|
|
|
| Form | Example |
|
|
| ------------- | ---------------------------------------------- |
|
|
| construction | `tag: 'loading'` |
|
|
| comparison | `s.tag === 'loaded'`, `s.tag !== 'loaded'` |
|
|
| type-level | `Extract<OrgTemplateState, { tag: 'loaded' }>` |
|
|
| documentation | `remote-data.mdx:72` |
|
|
|
|
Files in scope: the three machines (`brief.machine.ts`, `org-template.machine.ts`,
|
|
`stamdata-editor.machine.ts`), their three specs, `brief.store.ts`, `brief.store.spec.ts`,
|
|
`org-template.store.ts`, `stamdata.store.ts`, `brief.page.ts`, and `remote-data.mdx`.
|
|
|
|
4. **`BriefStatus` IS NOT IN SCOPE. This is the one way to break this ticket.**
|
|
`brief.machine.ts` contains **two** independent lowercase tag families:
|
|
- `BriefState`'s load lifecycle — `loading`/`failed`/`loaded` — **rename these**
|
|
- `BriefStatus`'s letter status — `draft`/`submitted`/`approved`/`rejected`/`sent`, defined
|
|
in `brief.ts:68` — **leave these alone**
|
|
|
|
`brief.machine.ts:278` has both in one line:
|
|
`if (s.tag !== 'loaded' || s.brief.status.tag !== from …)`. The first is in scope, the
|
|
second is not. `BriefStatus` is parsed off the wire from `BriefViewDto`, so renaming its
|
|
tags breaks the parse boundary and the backend contract. **Never rename by "all lowercase
|
|
tags in this file".**
|
|
|
|
5. **Three more collision sites must not be touched.** They use the same words for unrelated
|
|
things, and anchoring on `tag: '` already excludes them — but verify rather than assume:
|
|
- `scenario.ts` / `scenario.interceptor.ts` — `'loading'` is a `?scenario=` **URL param
|
|
value**, not a state tag
|
|
- `upload.machine.ts` and the four upload UI components — `UploadStatus` discriminates on
|
|
**`type:`**, not `tag:`, with `'failed'`/`'complete'`/`'uploading'`
|
|
- `registratie-lookup.store.ts` — `'loading'` is an Angular `resource()` status
|
|
|
|
6. **Do not touch `ActionState`, `SaveState`, or `pendingPublish`.** RD-12, RD-13 and RD-14
|
|
own those, and they must follow this ticket or the same tags get renamed twice.
|
|
|
|
## Files
|
|
|
|
Add to / edit: `libs/shared/src/application/remote-data.ts` (+ `.spec.ts`),
|
|
`libs/shared/docs/remote-data.mdx`.
|
|
Delete: `libs/shared/src/application/machine-remote-data.ts` (+ `.spec.ts`).
|
|
Rename tags in: `brief.machine.ts` (+ spec), `org-template.machine.ts` (+ spec),
|
|
`stamdata-editor.machine.ts` (+ spec), `brief.store.ts` (+ spec), `org-template.store.ts`,
|
|
`stamdata.store.ts`, `brief.page.ts`.
|
|
|
|
## Steps
|
|
|
|
1. Add `fromLoadLifecycle` to `remote-data.ts` with PascalCase keys (decisions 1-2).
|
|
2. Rename the load-lifecycle tags across the files in decision 3, one file at a time, letting
|
|
the type-checker find the next site. **Do not blanket-sed.**
|
|
3. Point the three stores at `fromLoadLifecycle`; delete `machine-remote-data.ts` and merge
|
|
its spec cases into `remote-data.spec.ts`.
|
|
4. Update `remote-data.mdx:72`.
|
|
5. Run `npm run gen:behaviour-spec` — `behaviour-spec.mdx:839` has a `machineRemoteData`
|
|
section that must become the new name.
|
|
6. Update this ticket's `Status:` to `done` and the README's RD-11 row to `done`.
|
|
7. Commit all of it together.
|
|
|
|
## Acceptance criteria
|
|
|
|
The third encoding is gone and nothing lowercase survives in the three machines:
|
|
|
|
```bash
|
|
git grep -n "machineRemoteData\|LoadLifecycle" -- apps libs # MUST return nothing
|
|
ls libs/shared/src/application/machine-remote-data* # MUST be "No such file"
|
|
|
|
M="apps/ssp/src/app/brief/domain apps/ssp/src/app/brief/application \
|
|
apps/ssp/src/app/brief/ui libs/beheer/src/domain libs/beheer/src/application"
|
|
git grep -n "tag: 'loading'\|tag: 'failed'\|tag: 'loaded'" -- $M # MUST return nothing
|
|
git grep -n "tag === 'loaded'\|tag !== 'loaded'" -- $M # MUST return nothing
|
|
```
|
|
|
|
`BriefStatus` is untouched — this is the check that matters most (decision 4):
|
|
|
|
```bash
|
|
# Measured before this ticket was written: the total is exactly 54. It MUST still be 54.
|
|
git grep -c "tag: 'draft'\|tag: 'submitted'\|tag: 'approved'\|tag: 'rejected'\|tag: 'sent'" \
|
|
-- apps/ssp/src/app/brief | awk -F: '{s+=$NF} END {print s}' # MUST print 54
|
|
|
|
git diff --stat apps/ssp/src/app/brief/domain/brief.ts # MUST be empty — brief.ts unchanged
|
|
```
|
|
|
|
If that number moves, you have renamed a wire contract. Stop and revert rather than adjusting
|
|
the number.
|
|
|
|
The collision sites are untouched:
|
|
|
|
```bash
|
|
git diff --name-only | git grep -c "scenario\|upload" || true # expect no such files
|
|
```
|
|
|
|
```bash
|
|
npm run ci # exits 0
|
|
```
|
|
|
|
## Verification
|
|
|
|
`npm run ci`. Also run `npm run ci -- --full` **with an explicit long timeout**: this edits
|
|
`remote-data.mdx`, which Storybook globs, and a broken MDX import is invisible to plain `ci`.
|
|
|
|
Note for whoever runs it: an 8-minute command cannot complete in the default 120s foreground
|
|
window and the harness will move it to the background. Pass `timeout: 600000` on the Bash
|
|
call so it runs to completion in the foreground, then commit.
|
|
|
|
## Out of scope
|
|
|
|
- `ActionState` / `SaveState` / `pendingPublish` — RD-12, RD-13, RD-14 (decision 6).
|
|
- `BriefStatus` (decision 4). If a `BriefStatus` tag changes, the ticket has failed.
|
|
- `UploadStatus`'s `type:` discriminant — optional RD-35.
|
|
- The `NO_SUBORGS`/`NO_TABLES`-should-be-`Empty` finding — optional RD-34.
|
|
|
|
## Risks
|
|
|
|
- **`BriefStatus` (decision 4) is the failure mode to fear.** Its tags are a wire contract.
|
|
Rename by union, never by file.
|
|
- **Do not blanket-sed `'loading'`/`'failed'`/`'loaded'`.** Five files legitimately use those
|
|
words for other purposes (decision 5). Renaming one file at a time and following the
|
|
type-checker is slower and correct.
|
|
- **`brief.store.ts` and `brief.page.ts` use only the comparison form**, so a
|
|
construction-only grep misses them. That is why decision 3 lists four forms.
|
|
- **`behaviour-spec.mdx` drift**: it has a `machineRemoteData` section heading at :839 which
|
|
changes with the function name. Run `gen:behaviour-spec` in the same commit.
|
|
- **`remote-data.ts` carries a `// #region showcase:fold` marker at :30.** If your edit moves
|
|
or splits that region, run `npm run gen:snippets` in the same commit too.
|