ehoandClaude Opus 5
57fc72f254
Decision 3 said the `@shared/ui/` occurrence count must not change at 200, while decision 4, nine lines below, mandated converting five relative imports into `@shared/ui/` aliases — five new occurrences by construction. The answer is 205, and the agent verified by hand that the 21 occurrences now inside `libs/shared/src/ui/` are the 16 from before plus exactly those five. This is the fourth instance of the pattern named in the previous docs commit, and the first ticket written after naming it. Recorded as such: when two decisions both touch the thing you are counting, add them up first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
228 lines
12 KiB
Markdown
228 lines
12 KiB
Markdown
# RD-27 — Make the folder equal the layer in `libs/shared/src/ui/`
|
|
|
|
Status: done
|
|
Source: PLAN.md 4a, Phase 4
|
|
|
|
## Why
|
|
|
|
CLAUDE.md decision 2 says "folder = layer": `libs/shared/ui` atoms → molecules → organisms.
|
|
Today `libs/shared/src/ui/` is 26 flat directories, and the only record of a component's layer
|
|
is its story title and a header comment. Nothing stops an atom importing an organism.
|
|
|
|
This ticket makes the structure say what the rule says. It is a **pure move**: no component
|
|
changes, no story title changes, no behaviour. RD-29 then adds the dependency-cruiser rules
|
|
that the folders make expressible.
|
|
|
|
**This is the highest-risk ticket in the arc**, because a broken `.mdx` story import compiles
|
|
fine and only fails when Storybook builds. The README says it must not be pushed without
|
|
`npm run ci --full`.
|
|
|
|
## Read first
|
|
|
|
- `libs/shared/src/ui/` — 25 flat component directories plus `upload/` with 8 of its own.
|
|
- `libs/shared/docs/atomic-design.mdx:2-5` — three of the seven `.mdx` story imports that break.
|
|
- PLAN.md 4a — the design record, including why `layout/` does not move.
|
|
- `.storybook-ssp/main.ts:11-14` — the globs, which are recursive and need no edit.
|
|
|
|
## Decisions (pre-made, don't relitigate)
|
|
|
|
1. **Three layer folders, and the full 33-directory mapping. This table is the ticket.**
|
|
|
|
`libs/shared/src/ui/atoms/` — 12 flat:
|
|
|
|
```
|
|
alert button checkbox heading link masked-value
|
|
placeholder-chip radio-group skeleton spinner status-badge text-input
|
|
```
|
|
|
|
`libs/shared/src/ui/molecules/` — 13 flat:
|
|
|
|
```
|
|
application-link application-list async choice-link choice-list confirmation
|
|
data-block data-row form-field review-section rich-text-editor stepper task-list
|
|
```
|
|
|
|
`upload/` **keeps its feature subfolder inside each layer** — 8 directories:
|
|
|
|
| New location | Directories |
|
|
| ---------------------- | ----------------------------------------------------------------------------------------------------- |
|
|
| `ui/atoms/upload/` | `delivery-channel-toggle`, `document-chip`, `file-input`, `upload-progress-bar`, `upload-status-icon` |
|
|
| `ui/molecules/upload/` | `single-upload` |
|
|
| `ui/organisms/upload/` | `document-category`, `document-upload` |
|
|
|
|
`ui/organisms/` holds nothing but `upload/`. That is correct and worth seeing.
|
|
|
|
The mapping is derived from each component's own story title, not invented — every one of
|
|
the 33 already declares its layer as `Design System/<Layer>/…`.
|
|
|
|
2. **Use `git mv` per directory, so the diff reads as renames.** `git diff --stat -M` must show
|
|
renames plus one-line import edits, nothing else.
|
|
|
|
3. **Rewrite the 28 distinct specifiers, longest key first.** All 28 are listed by the mapping
|
|
in decision 1; 25 are flat and 3 are the upload components imported from outside
|
|
(`upload/document-upload` → `organisms/upload/document-upload`, `upload/file-input` →
|
|
`atoms/upload/file-input`, `upload/single-upload` → `molecules/upload/single-upload`).
|
|
|
|
Verified: **no specifier is a prefix of another**, so ordering cannot corrupt a rewrite here.
|
|
Do it longest-first anyway — it costs nothing and the property is not guaranteed to hold if
|
|
this is ever repeated.
|
|
|
|
There are 200 occurrences across 73 files, and this rewrite alone changes none of them.
|
|
|
|
**Corrected after the ticket ran: the total lands at 205, not 200.** Decision 4 converts five
|
|
relative imports into `@shared/ui/…` aliases, and each of those is itself a new occurrence.
|
|
This decision's "must not change" governs _this_ rewrite; decision 4 adds five on top. The
|
|
two decisions were written as if they counted different things, and they do not.
|
|
|
|
The honest check is per-source: 200 from the specifier rewrite, plus exactly the 5 that
|
|
decision 4 names.
|
|
|
|
4. **Five of the seven relative imports inside `upload/` become aliases; two stay relative.**
|
|
A `../sibling/` import only breaks when the sibling lands in a different layer:
|
|
|
|
| File (new location) | Import | Becomes |
|
|
| ------------------------------------ | ---------------------------- | ------------------------------------------------- |
|
|
| `organisms/upload/document-category` | `../delivery-channel-toggle` | `@shared/ui/atoms/upload/delivery-channel-toggle` |
|
|
| `organisms/upload/document-category` | `../file-input` | `@shared/ui/atoms/upload/file-input` |
|
|
| `organisms/upload/document-category` | `../single-upload` | `@shared/ui/molecules/upload/single-upload` |
|
|
| `molecules/upload/single-upload` | `../document-chip` | `@shared/ui/atoms/upload/document-chip` |
|
|
| `molecules/upload/single-upload` | `../upload-progress-bar` | `@shared/ui/atoms/upload/upload-progress-bar` |
|
|
|
|
Unchanged, because both ends stay in the same layer: `atoms/upload/document-chip` →
|
|
`../upload-status-icon`, and `organisms/upload/document-upload` → `../document-category`.
|
|
|
|
**Use the alias, not `../../../`.** Cross-directory imports inside `libs/shared` already use
|
|
`@shared/…` (see `wizard-shell.component.ts`), and a three-level relative path is exactly the
|
|
thing a later move breaks silently.
|
|
|
|
5. **Seven `.mdx` story imports break. All seven are real imports, not prose.** The Order table
|
|
says eight; measured, it is seven:
|
|
|
|
```
|
|
a11y.mdx:2 ../src/ui/alert/alert.stories -> ui/atoms/alert/…
|
|
a11y.mdx:3 ../src/ui/form-field/form-field.stories -> ui/molecules/form-field/…
|
|
atomic-design.mdx:2 ../src/ui/button/button.stories -> ui/atoms/button/…
|
|
atomic-design.mdx:3 ../src/ui/form-field/form-field.stories -> ui/molecules/form-field/…
|
|
atomic-design.mdx:5 ../src/ui/upload/document-upload/… -> ui/organisms/upload/document-upload/…
|
|
fp-in-ui.mdx:2 ../src/ui/async/async.stories -> ui/molecules/async/…
|
|
remote-data.mdx:2 ../src/ui/async/async.stories -> ui/molecules/async/…
|
|
```
|
|
|
|
`atomic-design.mdx:4` imports `../src/layout/page-shell/…`. **Leave it alone** — `layout/`
|
|
does not move.
|
|
|
|
6. **`layout/` does not move, and neither does `libs/beheer/src/ui/`.** CLAUDE.md §5 explicitly
|
|
sanctions `libs/shared/layout` holding several layers, and `libs/beheer` is a bounded context
|
|
that lives in `libs/` only because two apps share it. Both are settled; do not revisit them.
|
|
|
|
7. **Change no story title, no layer-tag comment, and no component code.** The titles already
|
|
say the right thing, which is what made decision 1's mapping derivable. The two mislabelled
|
|
tags (`async` has `/** Convenience: */`, `breadcrumb` says `/** Chrome: */`) and the two
|
|
missing ones (`masked-value`, `rich-text-editor`) are **RD-28's** job, not this ticket's.
|
|
|
|
8. **No barrel file.** The repository has none and does not need one. A barrel would also hide
|
|
exactly the layer boundary this ticket exists to expose.
|
|
|
|
## Files
|
|
|
|
- 33 directories moved under `libs/shared/src/ui/`
|
|
- ~73 files with a one-line specifier edit
|
|
- 4 `.mdx` files (7 import lines)
|
|
|
|
No changes to `angular.json`, `eslint.config.mjs`, `plopfile.mjs`, any `tsconfig.json`,
|
|
`.dependency-cruiser.*`, `scripts/check-tokens.sh`, or `e2e/`. Verified: `@shared/*` maps to
|
|
`libs/shared/src/*`, so a deeper path resolves unchanged; both Storybook globs are recursive;
|
|
dependency-cruiser's `ui-not-infrastructure` pattern is `(/ui/|/layout/)`, which still matches a
|
|
nested path; and `check-tokens.sh`'s CIBG-GAP check keys on the **directory basename**, which a
|
|
parent-folder move preserves.
|
|
|
|
## Steps
|
|
|
|
1. `mkdir` the three layer folders, then `git mv` all 33 directories per decision 1.
|
|
2. Rewrite the 28 specifiers across `apps/` and `libs/` (decision 3).
|
|
3. Fix the five relative imports inside `upload/` (decision 4).
|
|
4. Fix the seven `.mdx` imports (decision 5).
|
|
5. `npm run typecheck` — four tsconfigs, and the fastest way to catch a missed specifier.
|
|
6. `git add -A`, then run the acceptance commands.
|
|
7. Update this ticket's `Status:` to `done` and the README's RD-27 row to `done`.
|
|
8. Commit all of it together.
|
|
|
|
## Acceptance criteria
|
|
|
|
Measured against the tree before handover. Run after `git add -A`.
|
|
|
|
The structure is three folders, and every directory landed:
|
|
|
|
```bash
|
|
ls -d libs/shared/src/ui/*/ | wc -l # is 26 -> MUST be 3
|
|
ls -d libs/shared/src/ui/atoms/*/ | wc -l # MUST be 13 (12 + upload)
|
|
ls -d libs/shared/src/ui/atoms/upload/*/ | wc -l # MUST be 5
|
|
ls -d libs/shared/src/ui/molecules/*/ | wc -l # MUST be 14 (13 + upload)
|
|
ls -d libs/shared/src/ui/molecules/upload/*/ | wc -l # MUST be 1
|
|
ls -d libs/shared/src/ui/organisms/*/ | wc -l # MUST be 1 (upload)
|
|
ls -d libs/shared/src/ui/organisms/upload/*/ | wc -l # MUST be 2
|
|
```
|
|
|
|
**The single strongest check in this ticket** — every specifier now names a layer, so only three
|
|
distinct values may remain:
|
|
|
|
```bash
|
|
git grep -ho "@shared/ui/[a-z0-9-]*" -- apps libs | sort -u # is 26 values -> MUST be exactly these 3:
|
|
# @shared/ui/atoms
|
|
# @shared/ui/molecules
|
|
# @shared/ui/organisms
|
|
```
|
|
|
|
Nothing was lost or duplicated in the rewrite:
|
|
|
|
```bash
|
|
git grep -ho "@shared/ui/" -- apps libs | wc -l # is 200 -> MUST be 205 (200 + decision 4's 5)
|
|
git grep -c "src/ui/" -- '*.mdx' | awk -F: '{s+=$NF} END {print s+0}' # is 7 -> MUST still be 7
|
|
git grep -c "from '\.\./" -- libs/shared/src/ui/ | awk -F: '{s+=$NF} END {print s+0}' # is 7 -> MUST be 2
|
|
```
|
|
|
|
The move reads as a move (decision 2):
|
|
|
|
```bash
|
|
git diff --cached --stat -M | tail -1 # inspect: renames + one-line edits, no rewritten files
|
|
```
|
|
|
|
```bash
|
|
npm run ci --full # exits 0 — REQUIRED, see Verification
|
|
```
|
|
|
|
## Verification
|
|
|
|
**`npm run ci --full` is mandatory and non-negotiable for this ticket.** Plain `npm run ci` does
|
|
not build Storybook, and a `.mdx` importing a moved story path is invisible to the type-checker,
|
|
the linter and every unit test. It fails only when `build-storybook` runs. The README calls out
|
|
RD-27 by name for this reason.
|
|
|
|
Run `npm run typecheck` first anyway (step 5): it covers four tsconfigs and catches a missed
|
|
`@shared/ui/*` specifier in seconds rather than at the end of a full gate.
|
|
|
|
`libs/shared/docs/layers.mdx` deep-links two story ids
|
|
(`design-system-molecules-application-link--navigatie`,
|
|
`domein-registratie-aanvraag-block--concept`). Story ids derive from **titles**, and decision 7
|
|
changes no title, so both links survive. Do not "fix" them.
|
|
|
|
## Out of scope
|
|
|
|
- The dependency-cruiser ladder rules. RD-29 adds them, and it depends on this ticket.
|
|
- Layer-tag comments and the `libs/beheer` title rule — RD-28.
|
|
- `layout/` (decision 6).
|
|
- Any component's code, template, styles or story title.
|
|
|
|
## Risks
|
|
|
|
- **A broken `.mdx` import is invisible until Storybook builds.** This is the whole reason the
|
|
ticket carries `--full`. Decision 5 lists all seven; check each one after the move.
|
|
- **`git mv`, not `mv` + `git add`.** Both produce the same tree, but only the first keeps the
|
|
diff readable as renames — and this diff is 33 directories wide.
|
|
- **Do not flatten `upload/`** (decision 1). Its subfolder survives inside each layer.
|
|
- **Do not reach for `../../../`** (decision 4). Use the alias.
|
|
- **The occurrence count is the tripwire for a bad `sed`.** 200 before, 200 after. A rewrite
|
|
that accidentally matches twice, or drops a line, moves that number.
|
|
- **`atomic-design.mdx` has four story imports and only three of them move.** The fourth is
|
|
`page-shell`, in `layout/`.
|