Cloonar/deckle

Fork 0

Shared UI primitives: createAsyncAction and one errorMessage #117

New issue

Closed

opened 2026-07-18 14:20:09 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-18 14:20:09 +02:00

Owner

This was generated by AI during triage.

Found by an architecture review (deepening opportunities — turning shallow modules into deep ones). Rescoped during triage: the third primitive originally proposed here, a shared createDragReorder drag-DOM hook, was split out to #124 because its contract needs a design pass; this issue keeps the two mechanical extractions.

Problem — pure duplication in the web editor (Solid.js):

errorMessage(err: unknown): string exists as an export of the mutations store (which PagePane already imports) and is re-declared verbatim in five more components (MediaModule, PageSettings, LinkPicker, TreeBrowser, MediaPicker). Deleting any local copy just makes it reappear at its siblings — fails the deletion test.
Every dialog sub-component in MediaModule (7 of them) re-implements the same ~15-line async shell: its own busy and error signals plus a submit that does setBusy → try one api call → catch setError(errorMessage(err)) → finally setBusy(false).

Solution: a createAsyncAction(fn) primitive exposing { busy, error, run }, and a single canonical home for errorMessage that every component imports.

Benefits: mostly locality and consistency; modest but cheap, and it thins the panes ahead of (or alongside) the PagePane/TreePane controller extraction.

> *This was generated by AI during triage.* Found by an architecture review (deepening opportunities — turning shallow modules into deep ones). **Rescoped during triage**: the third primitive originally proposed here, a shared `createDragReorder` drag-DOM hook, was split out to #124 because its contract needs a design pass; this issue keeps the two mechanical extractions. **Problem — pure duplication in the web editor (Solid.js):** 1. `errorMessage(err: unknown): string` exists as an export of the mutations store (which PagePane already imports) and is re-declared verbatim in five more components (MediaModule, PageSettings, LinkPicker, TreeBrowser, MediaPicker). Deleting any local copy just makes it reappear at its siblings — fails the deletion test. 2. Every dialog sub-component in MediaModule (7 of them) re-implements the same ~15-line async shell: its own `busy` and `error` signals plus a `submit` that does setBusy → try one api call → catch setError(errorMessage(err)) → finally setBusy(false). **Solution**: a `createAsyncAction(fn)` primitive exposing `{ busy, error, run }`, and a single canonical home for `errorMessage` that every component imports. **Benefits**: mostly *locality* and consistency; modest but cheap, and it thins the panes ahead of (or alongside) the PagePane/TreePane controller extraction.

dominik.polakovics added the

needs-triage

enhancement

labels

2026-07-18 14:20:09 +02:00

dominik.polakovics referenced this issue

2026-07-18 15:45:20 +02:00

Split web/src/api.ts: one request<T> seam, typed errors, stores out #112

dominik.polakovics referenced this issue

2026-07-18 17:36:34 +02:00

refactor(web): split api.ts into transport seam, domain modules, and stores #119

dominik.polakovics referenced this issue

2026-07-19 00:12:17 +02:00

Define effective page type once (currently three byte-for-byte mirrors) #115

~~dominik.polakovics referenced this issue 2026-07-19 01:13:49 +02:00~~

Shared drag-reorder primitive: createDragReorder owning DOM drag plumbing #124

dominik.polakovics changed title from ~~Shared UI primitives: createAsyncAction, one errorMessage, createDragReorder~~ to Shared UI primitives: createAsyncAction and one errorMessage

2026-07-19 01:14:01 +02:00

dominik.polakovics commented

2026-07-19 01:14:25 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Extract two shared web-editor primitives — one canonical errorMessage helper and a createAsyncAction async-shell primitive — and adopt them everywhere the pattern is currently copy-pasted.

Current behavior:
The web editor (Solid.js, under web/) duplicates two small patterns:

errorMessage(err: unknown): string (returns err.message for Error instances, String(err) otherwise) is exported from the mutations store and also re-declared verbatim as a local function in five components: MediaModule, PageSettings, LinkPicker, TreeBrowser, MediaPicker. PagePane already imports the shared export.
Each of MediaModule's seven dialog sub-components declares its own busy and error signals and a submit function with the same shape: optional client-side validation → setBusy(true) / setError("") → await one api call → on success call a parent callback → catch sets setError(errorMessage(err)) → finally setBusy(false).

Desired behavior:

Exactly one declaration of errorMessage exists in the web source, in a home that doesn't create odd import directions (a dedicated shared module is fine, as is keeping the existing store export — pick whichever reads best and re-point every consumer). No component declares its own copy.
A createAsyncAction primitive wraps an async function and exposes reactive busy and error accessors plus a run method. run sets busy, clears the previous error, awaits the wrapped function, records errorMessage(err) on failure, and always clears busy. Concurrent run calls while busy should be a no-op (matching what the disabled submit buttons already enforce).
All seven MediaModule dialogs use it, dropping their per-dialog signals and try/catch shells. Pre-flight validation that sets an error message without calling the api (e.g. the slug check in the new-folder dialog) must still work — the primitive should allow setting/clearing the error from outside run, or the dialog keeps that check before invoking run.
Other components with the identical shell shape may adopt it opportunistically, but conversion beyond MediaModule's dialogs is optional.

Key interfaces:

errorMessage(err: unknown): string — behavior unchanged, single definition.
createAsyncAction — new primitive; suggested shape createAsyncAction<Args extends unknown[]>(fn: (...args: Args) => Promise<void>) returning { busy: Accessor<boolean>, error: Accessor<string>, run: (...args: Args) => Promise<void>, setError?: ... }. Exact signature is the agent's call; it must stay a thin Solid primitive (signals + one function), not a framework.
The seven MediaModule dialog components — their rendered UI and parent-facing props (onClose, onCreated, etc.) must not change.

Acceptance criteria:

grep -rn "function errorMessage" web/src yields exactly one hit; all former call sites import it.
createAsyncAction has unit tests (the web test suite under web/src/test/ uses vitest) covering: busy true during a pending run and false after; error set from a rejecting fn via errorMessage; error cleared on the next run; re-entrant run while busy is a no-op.
No MediaModule dialog declares its own busy/error signals or try/catch submit shell.
Dialog behavior is unchanged: busy state still disables the confirm control, validation errors still render, success still fires the parent callback.
The full web test suite and go test ./... pass (build web/dist first; toolchains come from nix shell nixpkgs#go / nixpkgs#nodejs per the repo's dev-env notes).

Out of scope:

The drag-reorder DOM plumbing (createDragReorder) — split to #124.
The PagePane/TreePane controller extraction mentioned in the original review.
Any change to error message wording, dialog layout, or UX.
Refactoring dialogs that don't match the busy/error/submit shape.

> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Extract two shared web-editor primitives — one canonical `errorMessage` helper and a `createAsyncAction` async-shell primitive — and adopt them everywhere the pattern is currently copy-pasted. **Current behavior:** The web editor (Solid.js, under `web/`) duplicates two small patterns: 1. `errorMessage(err: unknown): string` (returns `err.message` for `Error` instances, `String(err)` otherwise) is exported from the mutations store and *also* re-declared verbatim as a local function in five components: MediaModule, PageSettings, LinkPicker, TreeBrowser, MediaPicker. PagePane already imports the shared export. 2. Each of MediaModule's seven dialog sub-components declares its own `busy` and `error` signals and a `submit` function with the same shape: optional client-side validation → `setBusy(true)` / `setError("")` → `await` one api call → on success call a parent callback → `catch` sets `setError(errorMessage(err))` → `finally` `setBusy(false)`. **Desired behavior:** 1. Exactly one declaration of `errorMessage` exists in the web source, in a home that doesn't create odd import directions (a dedicated shared module is fine, as is keeping the existing store export — pick whichever reads best and re-point every consumer). No component declares its own copy. 2. A `createAsyncAction` primitive wraps an async function and exposes reactive `busy` and `error` accessors plus a `run` method. `run` sets busy, clears the previous error, awaits the wrapped function, records `errorMessage(err)` on failure, and always clears busy. Concurrent `run` calls while busy should be a no-op (matching what the disabled submit buttons already enforce). 3. All seven MediaModule dialogs use it, dropping their per-dialog signals and try/catch shells. Pre-flight validation that sets an error message without calling the api (e.g. the slug check in the new-folder dialog) must still work — the primitive should allow setting/clearing the error from outside `run`, or the dialog keeps that check before invoking `run`. 4. Other components with the *identical* shell shape may adopt it opportunistically, but conversion beyond MediaModule's dialogs is optional. **Key interfaces:** - `errorMessage(err: unknown): string` — behavior unchanged, single definition. - `createAsyncAction` — new primitive; suggested shape `createAsyncAction<Args extends unknown[]>(fn: (...args: Args) => Promise<void>)` returning `{ busy: Accessor<boolean>, error: Accessor<string>, run: (...args: Args) => Promise<void>, setError?: ... }`. Exact signature is the agent's call; it must stay a thin Solid primitive (signals + one function), not a framework. - The seven MediaModule dialog components — their rendered UI and parent-facing props (`onClose`, `onCreated`, etc.) must not change. **Acceptance criteria:** - [ ] `grep -rn "function errorMessage" web/src` yields exactly one hit; all former call sites import it. - [ ] `createAsyncAction` has unit tests (the web test suite under `web/src/test/` uses vitest) covering: busy true during a pending run and false after; error set from a rejecting fn via `errorMessage`; error cleared on the next run; re-entrant `run` while busy is a no-op. - [ ] No MediaModule dialog declares its own `busy`/`error` signals or try/catch submit shell. - [ ] Dialog behavior is unchanged: busy state still disables the confirm control, validation errors still render, success still fires the parent callback. - [ ] The full web test suite and `go test ./...` pass (build `web/dist` first; toolchains come from `nix shell nixpkgs#go` / `nixpkgs#nodejs` per the repo's dev-env notes). **Out of scope:** - The drag-reorder DOM plumbing (`createDragReorder`) — split to #124. - The PagePane/TreePane controller extraction mentioned in the original review. - Any change to error message wording, dialog layout, or UX. - Refactoring dialogs that don't match the busy/error/submit shape.

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-07-19 01:14:30 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-19 01:26:22 +02:00

refactor(web): extract shared errorMessage and createAsyncAction primitives

dominik.polakovics referenced this issue from a pull request that will close it,

2026-07-19 01:26:36 +02:00

refactor(web): shared errorMessage and createAsyncAction primitives #125