diff --git a/docs/framework-remediation-plan.md b/docs/framework-remediation-plan.md new file mode 100644 index 00000000..4c292221 --- /dev/null +++ b/docs/framework-remediation-plan.md @@ -0,0 +1,499 @@ +# Framework remediation plan + +Written 2026-08-09, after the 0.8.6 release. Every number here was measured +against the tree at commit `7a2b5865`, not estimated. Where a cause is not yet +proven the item says so and makes proving it step one — nothing in this document +should be implemented on the strength of a guess. + +Each work item has the same four fields: + +- **Issue** — what is wrong, stated so it can be disagreed with. +- **Evidence** — how we know, with the file or the measurement. +- **Change** — what to do. +- **How to test** — the check that fails before and passes after. If an item's + test cannot fail before the change, the test is wrong; see the note on + vacuous gates in §0.2. + +--- + +## 0. Standing conventions + +These are not tasks. They are the rules the tasks below are written against, and +they apply to all new work from now on. + +### 0.1 A component owns everything it needs + +**Every `.wrn` component is self-contained.** Markup, behaviour and styles live +in that one file. A component that needs a rule writes it in its own `style {}` +block, scoped to its own classes. + +`ui.css` holds **global styles only** — resets, tokens, base element styling and +genuinely cross-cutting primitives. It is not a place to put component rules, +and no new component rule may be added to it. The reason is delivery: a +component's own styles ship only when that component renders, while `ui.css` is +served whole to every page on the site. See §4.1 for the migration and the +numbers. + +Practical rules: + +- CSS comments in a `style {}` block are `/* */` only. `//` is not a CSS comment + and silently swallows the rule that follows it. +- Style the component's own BEM classes (`wire-thing`, `wire-thing__part`). + Variants are data attributes: `[data-variant="soft"]`, not a class explosion. +- No Tailwind utility classes in component markup. See §4.3. + +### 0.2 A test that cannot fail is not a test + +Two gates in this repo passed for months while testing nothing, because both +were written against `getBoundingClientRect()`, which returns zeros in +happy-dom. The 0.8.5 focus trap had no coverage at all as a result. + +**Before committing any test in this plan, delete the fix, watch the test fail, +then restore the fix.** If it passes with the fix removed, it is measuring +nothing. Never gate testable behaviour on measured geometry. + +--- + +## 1. Bugs from this session + +All five are **fixed and shipped in 0.8.6**. They are recorded here because each +one has a permanent guard that must not be removed, and because the pattern they +share is the subject of §2. + +### 1.1 camelCase outputs never reached a parent binding — FIXED + +**Issue.** HTML lowercases attribute names, so a parent writing `@sizeChange` +registered a handler under `sizechange`, while the component emitted +`sizeChange`. The registry lookup missed, fell through to a DOM dispatch, and +the parent binding was never invoked. No error at either end. + +**Evidence.** All 17 camelCase outputs in the library were undeliverable: +`DataTable.pageChange`, `DataTable.rowClick`, `Map.markerClick`, +`ChatBubble.messageClick`, `LayoutSplitter.sizeChange`, the four `carousel.*` +and four `DragAndDrop.*` names, and the rest. Confirmed in a browser by reading +`__wrnexusOutputHandlers` off the component root and seeing the key casing. + +**Change.** Done — `invokeComponentOutput` in +`packages/csr/src/reactive-runtime.ts` now falls back to a case-insensitive +lookup. + +**How to test.** + +```bash +bun test packages/csr/test/reactive.test.ts -t "camelCase output" +``` + +Verified failing with the fallback removed. Keep it that way. + +### 1.2 Eighteen components emitted into the void — FIXED + +**Issue.** An output only reaches a parent `@binding` when the component calls +`output.()`. The runtime resolves parent handlers from a registry that +only `invokeComponentOutput` reads, so a component dispatching its own +`CustomEvent` — even a bubbling one on its own root — emits to nobody. + +**Evidence.** 18 components did this. Card, Footer, Breadcrumb, Accordion, +alert, Badge, AnnouncementBar, AvatarGroup, ToggleCount and InputNumber were +converted; Marquee, Map, Timeline, List and SearchBox additionally had to +_declare_ the outputs they were already firing. + +**Change.** Done. Note the deliberate exception: dispatching on `window` is +still correct and is left alone — that is how Toaster, Modal and DataTable +signal across component boundaries, where there is no parent binding to reach. + +**How to test.** + +```bash +bun test packages/ui/test/ui.test.ts -t "hand-built CustomEvent" +``` + +### 1.3 Ten theme tokens were referenced and never defined — FIXED + +**Issue.** An undefined custom property does not warn; it resolves to nothing. +Focus rings drew with no colour in 14 components, every soft surface rendered +transparent in 27. + +**Change.** Done — derived in `packages/styles/src/theme.ts`. Two traps worth +remembering: a check that reads the theme _source_ produces about a dozen false +positives because most tokens are generated per palette, so any such check must +read the **rendered** CSS; and the semantic spread must sit _before_ the +primary/secondary palette entries or it overrides them. + +**How to test.** Load a page, then in the console collect every `var(--wire-*)` +referenced by stylesheet rules and check each against +`getComputedStyle(document.documentElement)`. Expect 38 referenced, 0 undefined. + +### 1.4 The modal focus trap had no coverage — FIXED + +**Issue.** `isDialogVisible` and `focusableWithin` both gated on +`getBoundingClientRect()`, which is all zeros in happy-dom. The trap never ran +under test. + +**Change.** Done — both read markers instead. One regression to remember: the +first fix used `data-show`, which broke Drawer, because a Drawer _animates_ open +and so cannot use `data-show`; every closed Drawer then looked open and held +`body { overflow: hidden }` forever. It reads `data-open` now. + +**How to test.** `bun test packages/csr` — the dialog tests must fail if the +marker checks are reverted to geometry. + +### 1.5 Roving focus matched `="false"` — FIXED + +**Issue.** Attribute-presence selectors match any value, including `"false"`, so +read-only Steppers captured arrow keys from the page. + +**Change.** Done — `rovingItemOff` treats bare as on and `"false"` as off. + +**How to test.** `bun test packages/csr` roving cases; a read-only Stepper must +not consume `ArrowRight`. + +--- + +## 2. The model is incomplete + +This section is the real work. These are capability gaps, not defects, and they +are what currently limits what anyone can build. **§2.1 is the highest-value +item in this entire document.** + +### 2.1 The framework fails silently, everywhere + +**Issue.** Nothing in §1 produced a single diagnostic at any layer. That is not +five unlucky bugs; it is a systemic property. The failure mode is always +"nothing happens", which is the most expensive kind to debug because a bug is +indistinguishable from something you configured wrong. + +**Evidence.** `packages/csr/src/reactive-runtime.ts` has 55 `catch` blocks, 11 +of which swallow entirely, and 18 `console.error`/`console.warn` calls across +~70 kB of runtime. Undefined theme token → renders nothing. Wrong output casing +→ delivers nothing. Deferred state write → dropped. Component in a slot → +dropped. Object prop written inline → parsed as interpolation. None warn. + +**Change.** Add a **dev-mode diagnostic layer**, stripped from production +builds. It should warn on at least: + +1. `output.()` called with no handler registered **and** no listener for + the event — include the registry keys actually present, which is exactly what + would have made §1.1 a five-minute bug. +2. A `@binding` naming a function that does not exist in scope. +3. A `var(--wire-*)` referenced by a component but absent from the rendered + theme. +4. A state write discarded because it happened after the client function + returned (see §2.2). +5. A component tag dropped during slot flattening (see §2.4). +6. A prop declared as an object or array that arrived as an unparsed string. + +Gate it on `process.env.NODE_ENV !== "production"` so the production bundle is +byte-identical to today. Route everything through one `warnOnce(code, message)` +helper with stable `WRN-` codes, so warnings are greppable and documentable and +a loop cannot spam the console. + +**How to test.** A test per diagnostic that asserts the warning fires, plus one +asserting the production build contains none of them: + +```bash +bun test packages/csr/test/diagnostics.test.ts +bun run build && grep -c "WRN-DEV-" examples/basic-app/dist/reactive.js # expect 0 +``` + +Add a runtime size assertion so the production bundle does not grow: it is +69,947 bytes minified today. + +### 2.2 Deferred state writes are silently dropped + +**Issue.** State written after a client function returns — in a `setTimeout`, a +promise callback, an observer, a `pointermove` — is discarded. The peer-function +wrapper flushes the entry-time snapshot, so the later write is simply lost. + +**Evidence.** This is why the focus trap, scrollspy, roving focus and the +splitter all had to be written as bespoke controllers inside the runtime. +Component-specific controllers are roughly 18% of `reactive.js` — that is a +model limitation being paid for in bundle size. + +**Change.** Two parts, in order. + +1. **Diagnose it** (part of §2.1). A dropped write must warn with the state key + and the function name. Cheap, and it stops the silent-loss class immediately. +2. **Support it.** Let a client function return a promise, or expose an explicit + `commit()`, so async writes reach the scope. This is the design decision — + it wants a short written proposal before implementation, because it + interacts with re-render batching. + +**How to test.** A test where a client function writes state inside a +`setTimeout` and the DOM reflects it after the timer. It must fail today. + +### 2.3 Props and slot content render once + +**Issue.** Component props and slot content do not track page state. The +controlled-component pattern does not work at all: passing `value={pageState}` +renders the initial value and never updates. + +**Why it matters most.** This is the biggest ceiling in the framework. It means +composition breaks down exactly where a component library needs it — a parent +cannot drive a child. Every workaround today is "hoist into page state and +re-render", which does not compose. + +**Change.** Make prop expressions reactive bindings rather than one-time +interpolations, the way attribute bindings already work (`data-wrn-bind-*`). +The mechanism exists for attributes; extend it to component props. + +If there is a hard reason this cannot work, **document that reason in the +component authoring guide** and give people a supported pattern instead. What +is not acceptable is the current state, where it looks like it should work and +quietly doesn't. + +**How to test.** A page with `state n`, a component bound to `value={n}`, and a +button incrementing `n`; the component's rendered output must follow. Must fail +today. + +### 2.4 A component tag inside another component's slot is dropped + +**Issue.** Nesting a component tag in another component's slot drops the tag and +leaves its children orphaned. + +**Change.** Fix slot flattening to preserve nested component boundaries. Until +it is fixed, emit the §2.1 diagnostic — a dropped component must never be +silent. + +**How to test.** Render `` and assert the Badge +root element exists in the output. Must fail today. + +### 2.5 Object props must be hand-hoisted + +**Issue.** An object or array prop written inline in an attribute is read as +interpolation. Three encodings were tried this session and all failed: bare +`{}` produced a 500, `({})` made `JSON.parse` throw so demos rendered silently +empty, and entity-escaping failed because the compiler passes attributes +undecoded. The only thing that works is hoisting to page state with +`JSON.parse`. + +**Change.** Either support inline object literals in prop position, or reject +them at **compile time** with a message naming the prop and showing the hoist +pattern. A 500 at runtime and a silently empty component are both unacceptable +outcomes for a plain authoring mistake. + +**How to test.** A compiler test asserting the specific diagnostic for +`items={{ a: 1 }}`, and one asserting the hoisted form still compiles. + +### 2.6 i18n text is not server-rendered + +**Issue.** Translated text ships as empty spans and is filled after hydration. +Navigation is blank until the runtime loads. + +**Why it matters.** This is an SSR-first framework failing to server-render +text. It is an SEO problem, a layout-shift problem, and it undercuts the +framework's central claim. + +**Change.** Resolve the active locale during SSR and emit the translated text +into the HTML, keeping the client path for locale switches. + +**How to test.** + +```bash +curl -s http://localhost:3520/ | grep -c "Navigation" # expect > 0 +``` + +Assert against the raw server response with JavaScript disabled — not the +hydrated DOM, which already looks correct today. + +--- + +## 3. Smaller but real + +### 3.1 Twenty-two outputs still have no emitter + +**Issue.** 22 outputs across 11 components are declared and never fired: +FileUpload (upload, progress, success, cancel, remove), ToastNotifications (add, +dismiss, clear, action), AdvancedDatePicker (open, close, clear), +AdvancedRangeSlider (start, end), Chart (dataPointClick, legendToggle), Confetti +(start, complete), TreeView (expand, collapse), Toast (dismiss), CopyMarkup +(success). + +Unlike §1.2 these are not miswired — the components genuinely do nothing. Each +needs a real implementation. + +**Change.** For each: implement the behaviour, or delete the output. Do not +leave a component advertising what it does not do. Deleting is a breaking change +and needs a migration note; implementing Chart and FileUpload is real work and +should be scheduled, not squeezed in. + +**How to test.** The ratchet in `packages/ui/test/ui.test.ts` is pinned at 22 +and only ever moves down. Lower the ceiling in the same commit that fixes a +component. + +### 3.2 Twenty-three components are still scaffolds + +**Issue.** No style block, no functions — the same shape as the Table scaffold +that was removed and the LayoutSplitter scaffold that was rebuilt. + +**Change.** Rebuild them properly under §0.1, or remove them. A scaffold in a +published library is a promise the library does not keep. + +**How to test.** Per component: it renders, its interactive behaviour works in a +browser, its outputs fire, and it carries its own styles. + +### 3.3 Seven components still use Tailwind utilities + +**Issue.** `List` (23 utility classes), `InputNumber` (22), `Marquee` (15), +`TextLink` (14), `Map` (12), `SearchBox` (8), `Timeline` (8) still style +themselves with utility classes in markup. + +**Change.** Migrate to `wire-*` BEM classes in the component's own `style {}` +block, per §0.1. Note this changes the rendered class list, so application CSS +selecting on those utilities stops matching — it needs a migration note, exactly +as the layout group did in 0.8.6. + +**How to test.** + +```bash +bun run check:ui-visual # review the rendered diff deliberately +bun run generate:ui-visual # only after reviewing +``` + +Compare screenshots before and after; the visual result should be unchanged. + +### 3.4 `List` only fires `select` for items with an href + +**Issue.** The `@click` handler sits on the `item.href` branch only, so an item +without a link is not selectable. Pre-existing, and surprising. + +**Change.** Decide deliberately: either fire `select` for all items, or document +that `List` is a navigation component and non-link items are inert. + +**How to test.** Render a `List` with items lacking `href`, click one, assert +whichever behaviour was chosen. + +--- + +## 4. Delivery and the dev loop + +### 4.1 `ui.css` ships whole to every page + +**Issue.** `ui.css` is **175,848 bytes, 26,350 gzipped**, served on every page. +Only **42 of 108** components have a local `style {}` block, so 66 still depend +on it. + +**Change.** Migrate the remaining 66 under §0.1. This is mechanical, high-value, +and shrinks `ui.css` toward the global-only baseline it should be. Do it in +groups, one commit per group, regenerating the visual contract each time. + +**How to test.** Track the number down, every time: + +```bash +ls packages/ui/components/*.wrn | wc -l # 108 +grep -l '^ style {' packages/ui/components/*.wrn | wc -l # 42 today +bun run build && gzip -c examples/basic-app/dist/ui.css | wc -c # 26350 today +``` + +Add a ratchet test asserting the gzipped size only ever decreases, so this +cannot silently regress. + +### 4.2 Dev server: `packages/ui` edits do not take effect + +**Issue.** Editing a component under `packages/ui` does not hot-reload. Every UI +change this session needed a full server restart, which is the single biggest +drag on the edit loop. + +**Evidence, and what is _not_ yet proven.** The watcher is wired correctly: +`componentDirs` includes `uiComponentsDir()` and is passed as `extraDirs` +(`packages/dev-server/src/index.ts:706-723`), external paths arrive absolute +(`packages/dev-server/src/watch.ts:104`), and `hotUpdate` calls +`invalidateModule` and rebuilds the router with `componentDirs` +(`index.ts:600-616`). So the change _is_ observed. **Why it does not take effect +is not yet diagnosed.** + +One concrete lead, not a conclusion: `index.ts:619` filters to +`files.filter((file) => !isAbsolute(file))`, which excludes every +`packages/ui` path from the app-file branch below it. + +**Change.** Step one is to confirm the cause — add a log at the top of +`hotUpdate` printing the received paths, edit a `.wrn` under `packages/ui`, and +follow it. Only then fix. Do not implement against the lead above without +confirming it. + +**How to test.** Start the dev server, edit visible text in a +`packages/ui/components/*.wrn`, reload the page **without restarting**, and see +the change. That is the whole acceptance criterion. + +### 4.3 Running tests wipes the cache of a live dev server + +**Issue.** Running `bun test` or `check:production` while a dev server is up +empties `.wrnexus`, and every client module then 404s until a restart. This +produced several confusing "the Sidebar is broken" symptoms this session that +were not component bugs at all. + +**Evidence.** `resetDevCache` in `packages/dev-server/src/cache.ts:24-30` +`rmSync`s the whole cache directory on startup, unless +`WRNEXUS_PRESERVE_CACHE=1`. Any dev server a test spawns against the repo root +therefore deletes the running server's cache. + +**Change.** Two options, both cheap: + +- Give each dev-server instance its own cache directory (keyed by port or pid), + so instances cannot collide; or +- Have the test harness set `WRNEXUS_PRESERVE_CACHE=1` and use a temp cache dir + for every spawned server. + +Prefer the first — it fixes the class rather than one caller. Either way, +`resetDevCache` should refuse to delete a cache directory another live server +owns. + +**How to test.** Start the dev server, load a page, run `bun test packages` to +completion, reload the page without restarting. It must still work. This fails +reliably today. + +### 4.4 Generated artefacts have an order dependency + +**Issue.** The component reference must be regenerated **before** the showcase, +or newly added props and outputs are invisible to the showcase generator. This +is undocumented and cost real time. + +**Change.** Make the showcase generator depend on the reference explicitly, or +fail loudly when the reference is older than the component sources. + +**How to test.** Add a prop to a component, run the showcase generator without +regenerating the reference, and assert it errors rather than silently emitting a +stale page. + +### 4.5 Two authoring traps worth compiler errors + +**Issue.** Block comments are not allowed inside `props {}` — line comments +only — and `state page` collides with the `page` keyword. Both currently fail in +confusing ways. + +**Change.** Detect both at compile time with a message that names the problem. + +**How to test.** Compiler tests asserting the specific diagnostics. + +--- + +## 5. Order of work + +Ranked by return, not by size. The first item changes the cost of every item +below it, which is why it is first. + +| # | Item | Why now | +| --- | -------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | +| 1 | §2.1 dev-mode diagnostics | Changes the debugging economics of everything else. Every §1 bug would have been minutes instead of hours. | +| 2 | §2.3 reactive props | The biggest capability ceiling. Composition does not work without it. | +| 3 | §2.6 server-rendered i18n | Contained work; the core SSR claim currently fails on text. | +| 4 | §4.2 + §4.3 dev loop | Cheap, and it compounds across every task below. | +| 5 | §4.1 finish the CSS migration | 66 components, mechanical, takes ~26 kB off every page. | +| 6 | §3.1 implement or delete the 22 dead outputs | Honesty. Deleting is an afternoon; implementing is scheduled work. | + +§2.2, §2.4 and §2.5 fold into item 1 as diagnostics first, then into item 2 as +model work. + +**What I would resist: adding more components.** The library is 108 wide and +roughly 60% deep. Everything above is depth. + +## 6. A note on strategy + +47 packages before 1.0 is a large surface area. Each is a public API you have +committed to, a version to align and a migration note to write — the 0.8.6 +release touched 117 files to change one behaviour. The gate makes that +tractable, but tractable is not free. Some packages (`native`, `mobile`, +`playground`, `mcp`, `graphql`) look like option value rather than load-bearing. +Consolidating them would buy speed on exactly the model gaps in §2 that +currently limit users. + +This is a judgement call, not a defect, and it is yours to make.