Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions packages/v4/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -566,30 +566,33 @@ class Slider extends Base {
- No global instance registry is added. The subscription stays active through unmount and mount cycles, for the whole life of the watching instance.
- Unmounted instances announce from `document`, so one lazy, realm-shared listener serves every watcher and the document holds nothing but weak references to them. A listener per watcher would make every watching component immortal, since the document outlives the page's components.

### The page-wide lookup — `getInstances()`
### The page-wide lookup — `getInstances()` and its three siblings

```js
getInstances('Dialog').forEach((dialog) => dialog.close()); // page-wide
getInstances('Dialog', section); // one region
getInstances(el); // everything mounted on one element
getMountedInstances('Dialog').forEach((dialog) => dialog.close()); // the live ones
getInstances('Dialog', section); // every one built in a region
getUnmountedInstances('Dialog'); // built, then stood down
getInstance(el, 'Dialog'); // the one on this element, mounted or not
getInstances(el); // everything on one element
```

It derives the answer from the DOM. It keeps no registry of instances.

- A matching element with no instance is skipped.
- The filter is `$isMounted`, so an unmounted instance is never returned.
- **The name says which population it answers.** `getInstances()` returns every instance that exists; `getMountedInstances()` is the safe list to call a method on; `getUnmountedInstances()` is what a reversible `in-view` or `media:` strategy has stood down, plus a construct-then-mount-failure. The three take the same two overloads.
- **A matching element with no instance is skipped, and that is the whole narrowing.** `selectorFor(name)` over-matches on purpose — it lists the responsive spellings of `data-component` too — and the instance-map read is what removes an inactive declaration, because a breakpoint-withdrawn component is destroyed _and_ deleted from the map. A mount filter never did that work, which is why it is gone.
- **There is no `getMountedInstance`.** The singular returns one object; a caller reads `.$isMounted` on it.
- `root` is a `ParentNode` and the call is `querySelectorAll`, so an element root searches its descendants and never matches itself.
- `selectorFor(name)` on `/utils` is the one place that writes the name-to-selector contract.

### Where the instances live

An element publishes its instances under `Symbol.for('@studiometa/js-toolkit-v4/instances')`. It is not public API; `getInstances()` is. In a console, read them with one line:
An element publishes its instances under `Symbol.for('@studiometa/js-toolkit-v4/instances')`. It is not public API; the four lookups are, and between them they express every read the map answers. In a console, read them with one line:

```js
$0[Symbol.for('@studiometa/js-toolkit-v4/instances')];
```

The element overload `getInstances(el)` answers "what is mounted here", in mount order and with the `$isMounted` filter.
The element overload `getInstances(el)` answers "what is here", in mount order. It reads the map directly and never the DOM, so it is the only form that reaches a **detached** element — the string form cannot, since `document.querySelectorAll()` does not see one. Pass the detached root as `root` when a name lookup has to reach inside it.

### Shared state — provide/inject

Expand Down Expand Up @@ -1068,7 +1071,7 @@ See [RATIONALE.md — 12. Storage](./RATIONALE.md#12-storage).

## 13. Testing — the `/test` subpath

`@studiometa/js-toolkit-v4/test` ships the eight helpers a test of a component cannot write for itself: `mount(html)`, `settle()`, `frames(count?)`, `waitFor(predicate, options?)`, `resetDom()`, `captureDiagnostics(target?)`, `recordEvents(target, ...types)` and `resetRegistry()`.
`@studiometa/js-toolkit-v4/test` ships the nine helpers a test of a component cannot write for itself: `mount(html)`, `settle()`, `frames(count?)`, `countRequestedFrames(during)`, `waitFor(predicate, options?)`, `resetDom()`, `captureDiagnostics(target?)`, `recordEvents(target, ...types)` and `resetRegistry()`.

- **The subpath exists because the timing recipe is not derivable.** "Has this component mounted and finished its writes?" is answered by five rounds of a 10ms timer followed by `defaultScheduler.whenIdle()`, and those two numbers encode the mount observer's delivery latency and the scheduler's lane order. Neither half works alone: `whenIdle()` can resolve before the observer has reported the element, and a timer can return between two lanes. `defaultScheduler` and `nextFrame` were already public, so the pieces shipped and the recipe did not.
- **It depends on no test framework.** Nothing in the module imports a runner, an assertion library or a spy; it reads the DOM and the scheduler only, so it runs under Vitest, under Playwright and on a plain browser page. That is also why it is not on the root barrel: a page has no use for it, and the root export count stays what it was.
Expand All @@ -1078,7 +1081,8 @@ See [RATIONALE.md — 12. Storage](./RATIONALE.md#12-storage).
- **`captureDiagnostics()` reads the channel, and asserting on `console.warn` does not.** A recovered failure is reported on a cancelable event whose _default behaviour_ is the console line; a spy on that line cannot see the code, the severity or the reporting component, and it passes for the wrong diagnostic. The helper cancels each event as it arrives, which is the same act that suppresses the sink — so collecting and silencing are one step, not two, and no spy is involved. `target` defaults to `document`, which sees everything a connected element reported, because diagnostics bubble and compose.
- **`recordEvents(target, ...types)` keeps the order and the payloads.** A component's contract is "`open` came before `opened`, and `opened` carried the height it measured", and a call-counting spy throws both away. Recording several types into one array is the only place their relative order is visible; the richer `{ type, detail }` shape ships because a caller wanting names alone can map, and the reverse is impossible. `$emit` dispatches synchronously but is almost never _called_ synchronously, so the count is awaited with `waitFor`, not read.
- **`resetRegistry()` is the inverse `registerComponent()` deliberately lacks.** A page registers once and keeps it; a test suite is the one caller for which a page-wide registry surviving `resetDom()` is wrong, and the workaround it forces is a counter minting `Widget-1`, `Widget-2`. It is coarse rather than a targeted `unregisterComponent(name)` because the element→controller map is a `WeakMap`: it cannot be enumerated, so nothing can walk it to dispose live triggers, and a `querySelectorAll()` sweep would still miss detached elements. Clearing everything works because the mutation-observer path already disposes a controller as its element leaves the DOM — hence the call belongs _after_ `resetDom()`, and **never in an `afterEach`**, which would unregister the module-top-level registrations for every later test in the file.
- **What is deliberately not in it**: an instance lookup (`getInstances()` already answers it), frame counters, fetch stubs and pointer sequences. Each is either specific to one spec or already served by `vi.fn()` and `@vitest/browser`'s `userEvent`.
- **`countRequestedFrames(during)` patches a global, and says so.** It swaps `globalThis.requestAnimationFrame` for a counting wrapper that forwards to the original, and restores it in a `finally` — so it comes back whether `during` returns or throws. It is here despite the patch because the assertion it serves, "this did not schedule a frame per event", has no other seam: the framework's own scheduler makes the calls, so there is nothing of the component's for a spy to sit on. Two concurrent calls would nest their wrappers; do not.
- **What is deliberately not in it**: an instance lookup (`getInstance()` and `getInstances()` on the root barrel answer it), fetch stubs and pointer sequences. Each is either specific to one spec or already served by `vi.fn()` and `@vitest/browser`'s `userEvent`.

See [RATIONALE.md — 13. Testing](./RATIONALE.md#13-testing).

Expand Down
24 changes: 22 additions & 2 deletions packages/v4/RATIONALE.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,20 @@ The document already knows where the components are, and a second index of it ca

There is no selector-strategy seam behind it. v4 resolves components through `data-component` alone, so name-to-selector is the only lookup shape that there will ever be, and `selectorFor(name)` is the one place that writes it down.

### Why one lookup became four

`getInstances()` shipped with a silent `$isMounted` filter, and the filter was wrong twice over. It was invisible at the call site — `getInstances('Foo')` returning 2 where three elements declare `Foo`, one of them below its `media:` breakpoint, is not debuggable from the name — and it made the primitive unreachable, because the raw map read it hid had no public spelling. Fifty-two spec files answered that by importing a private `getInstance()` from `test-utils.ts`, which is the same finding `Action` produced one level up: a caller reaching past the public surface to write a lookup core could write in ten lines.

So the population is named at the call site instead: `getInstances()`, `getMountedInstances()`, `getUnmountedInstances()`, and the singular `getInstance(el, name)`. One internal `collect()` does the traversal and a predicate does the difference, so DOM order, mount order and the map read are written once.

**Dropping the filter resurrects nothing**, which is the fact the whole change rests on. The string form narrows three times — `querySelectorAll(selectorFor(name))`, then the `INSTANCES` read, then the mount check — and the middle step already does the work the last one is credited with. An inactive declaration has no instance, and a breakpoint-withdrawn one is destroyed _and_ deleted from the map by `reconcileElement()`. What the filter did hide was real, and small: the instances a reversible `in-view` or `media:` strategy stands down and keeps for the crossing back. That population now has a name.

`selectorFor()`'s own doc comment credited the narrowing to the mount check, so it is rewritten. A comment that misattributes an invariant is how the filter comes back.

There is no `getMountedInstance()`. The singular already returns one object, and a mounted-only variant would have to fold "no instance" and "not mounted" into the same `undefined`.

The element form is the only one that reaches a detached element. `document.querySelectorAll()` does not see one, but the instance is still on it — so the asymmetry is documented rather than left to be discovered, and the escape hatch is passing the detached root as `root`.

### Why the instances live under a symbol

v3 stores `Map<string, Base | 'terminated'>` under `el.__base__`. v4 stored `Map<string, Base>` under the same name. Two versions in one document then read the map of the other as their own: the teardown of v4 called `$unmount()` on the instances of v3 and on the `'terminated'` string that v3 leaves behind, which is a `TypeError`, while the child resolution of v3 accepted a v4 instance as one of its children. That blocked any migration page by page. `src/coexistence.spec.ts` mounts both versions in one document and holds the line.
Expand Down Expand Up @@ -875,8 +889,14 @@ Its footgun is stated in the same place because the shape of a spec file guarant

### What was refused

`getInstance(el, name)` — the public `getInstances()` answers it, and a second spelling of a lookup is a second thing to keep true. A frame counter that patches `requestAnimationFrame` — one caller, and a global patch is a poor thing to hand out. Fetch stubs and pointer-event sequences — `vi.fn()` and `@vitest/browser`'s `userEvent` do both better, and each was shaped by the one spec that grew it.
Fetch stubs and pointer-event sequences — `vi.fn()` and `@vitest/browser`'s `userEvent` do both better, and each was shaped by the one spec that grew it. Those two refusals hold.

Two others did not, and both are recorded here as reversals rather than quietly corrected.

`getInstance(el, name)` was refused because "the public `getInstances()` answers it, and a second spelling of a lookup is a second thing to keep true". `getInstances()` did not answer it: it answered a filtered, plural version of it, and the fifty-two spec files importing a private `getInstance()` from `test-utils.ts` are the measurement. The refusal was also aiming at the wrong module — the fix was not a test helper but a missing core export, and it ships from the root barrel next to the plural forms. See "Why one lookup became four".

A frame counter that patches `requestAnimationFrame` was refused for "one caller, and a global patch is a poor thing to hand out". The caller count was four by the time the module shipped, and the argument about the patch answered the wrong question. The patch is not a convenience a consumer could write around: the framework's own scheduler owns the `requestAnimationFrame` calls, so a component exposes no seam for a spy, and "this did not schedule a frame per event" has no other spelling. `countRequestedFrames()` ships on `./test` with the patch stated in its first line and restored in a `finally`.

An event recorder was refused on that same list and then shipped as `recordEvents()`, which is worth recording as a reversal rather than quietly correcting. The refusal was right about `vi.fn()` replacing a _counter_ and wrong about what the seven spec files hand-rolling one were actually doing: they were recording `{ type, detail }` across several event names into one array, because the assertion is the sequence and its payloads, and that is the one thing a per-listener spy cannot express. Two shapes existed in the wild — `type` only, and `{ type, detail }` — and the richer one ships because mapping down is free and mapping up is impossible.

A re-export shim in `test-utils.ts` was refused too. The old file keeps the fixtures and the two helpers that stay source-only, and the specs move onto the new module in one pass; a shim would be both a compatibility layer and a temporary solution meant to be replaced.
A re-export shim in `test-utils.ts` was refused too, and that refusal held all the way to the file's deletion. The specs moved onto the new module in one pass; a shim would have been both a compatibility layer and a temporary solution meant to be replaced. `test-utils.ts` is gone now — the waits went to `src/test/index.ts`, the todo tree to `src/todo.fixtures.ts`, `getInstance()` to core and `countRequestedFrames()` to `./test` — and the two build-script exclusions that named the file went with it.
9 changes: 4 additions & 5 deletions packages/v4/migration/Accordion/Accordion.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { afterEach, describe, expect, it } from 'vitest';
import { registerComponent } from '../../src/index.js';
import { getInstance } from '../../src/test-utils.js';
import { getInstance, registerComponent } from '../../src/index.js';
import { resetDom, settle } from '../../src/test/index.js';
import { Accordion } from './Accordion.js';
import { AccordionItem } from './AccordionItem.js';
Expand All @@ -27,7 +26,7 @@ function render(): HTMLElement {

function items(root: HTMLElement): AccordionItem[] {
return [...root.querySelectorAll('[data-component="AccordionItem"]')].map((el) =>
getInstance<AccordionItem>(el, 'AccordionItem'),
getInstance<AccordionItem>(el, 'AccordionItem')!,
);
}

Expand All @@ -36,7 +35,7 @@ describe('Accordion', () => {
const root = render();
await settle();

const accordion = getInstance<Accordion>(root, 'Accordion');
const accordion = getInstance<Accordion>(root, 'Accordion')!;
expect(accordion.items.size).toBe(3);
expect(accordion.items.items.every((item) => item instanceof AccordionItem)).toBe(true);
});
Expand Down Expand Up @@ -140,7 +139,7 @@ describe('Accordion', () => {
const root = render();
await settle();

const accordion = getInstance<Accordion>(root, 'Accordion');
const accordion = getInstance<Accordion>(root, 'Accordion')!;
root.querySelector('[data-component="AccordionItem"]')?.remove();
await settle();
expect(accordion.items.size).toBe(2);
Expand Down
12 changes: 9 additions & 3 deletions packages/v4/migration/Action/Action.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { Base, registerComponents, swap, SWAP_MODES, type BaseConfig } from '../../src/index.js';
import { getInstance } from '../../src/test-utils.js';
import {
Base,
getInstance,
registerComponents,
swap,
SWAP_MODES,
type BaseConfig,
} from '../../src/index.js';
import { mount, resetDom, settle } from '../../src/test/index.js';
import { Dialog } from '../Dialog/Dialog.js';
import { Action } from './Action.js';
Expand Down Expand Up @@ -37,7 +43,7 @@ registerComponents(Action, Target, Foo, Bar, Dialog, MountProbe);
afterEach(resetDom);

function at<T extends Base>(root: ParentNode, selector: string, name: string): T {
return getInstance<T>(root.querySelector(selector), name);
return getInstance<T>(root.querySelector(selector)!, name)!;
}

function click(el: Element): Event {
Expand Down
6 changes: 3 additions & 3 deletions packages/v4/migration/Action/ActionEvent.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getInstances, type Base } from '../../src/index.js';
import { getMountedInstances, type Base } from '../../src/index.js';
import { MODIFIERS, parseEventDefinition, type Modifier } from '../event-modifiers.js';
import { getEffect, type EffectFunction } from './expression.js';

Expand Down Expand Up @@ -63,7 +63,7 @@ export class ActionEvent {
/** Co-located mounted instances, recomputed for each event. */
get instances(): Map<string, Base> {
return new Map(
getInstances(this.action.$el).map((instance) => [instance.$config.name, instance]),
getMountedInstances(this.action.$el).map((instance) => [instance.$config.name, instance]),
);
}

Expand Down Expand Up @@ -99,7 +99,7 @@ export class ActionEvent {
// Ignore unparseable target parts.
continue;
}
for (const instance of getInstances(name)) {
for (const instance of getMountedInstances(name)) {
if (!selector || instance.$el.matches(selector)) {
targets.push({ [name]: instance });
}
Expand Down
15 changes: 7 additions & 8 deletions packages/v4/migration/AnchorNav/AnchorNav.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { afterEach, describe, expect, it } from 'vitest';
import { registerComponents } from '../../src/index.js';
import { getInstance } from '../../src/test-utils.js';
import { getInstance, registerComponents } from '../../src/index.js';
import { mount, resetDom, settle, waitFor } from '../../src/test/index.js';
import { AnchorNav } from './AnchorNav.js';
import { AnchorNavLink } from './AnchorNavLink.js';
Expand All @@ -26,9 +25,9 @@ describe('AnchorNav', () => {
it('enters the matching link once its target scrolls into view', async () => {
const { root, target } = await render();
const link = getInstance<AnchorNavLink>(
root.querySelector('[data-component="AnchorNavLink"]'),
root.querySelector('[data-component="AnchorNavLink"]')!,
'AnchorNavLink',
);
)!;

target.setAttribute('style', ONSCREEN);
await waitFor(() => link.state === 'entering');
Expand All @@ -42,9 +41,9 @@ describe('AnchorNav', () => {
it('leaves the matching link once its target scrolls back out of view', async () => {
const { root, target } = await render();
const link = getInstance<AnchorNavLink>(
root.querySelector('[data-component="AnchorNavLink"]'),
root.querySelector('[data-component="AnchorNavLink"]')!,
'AnchorNavLink',
);
)!;

target.setAttribute('style', ONSCREEN);
await waitFor(() => link.state === 'entering');
Expand All @@ -67,9 +66,9 @@ describe('AnchorNav', () => {
document.body.append(root);
await settle();
const link = getInstance<AnchorNavLink>(
root.querySelector('[data-component="AnchorNavLink"]'),
root.querySelector('[data-component="AnchorNavLink"]')!,
'AnchorNavLink',
);
)!;
const target = root.querySelector('#one') as HTMLElement;

target.setAttribute('style', ONSCREEN);
Expand Down
Loading
Loading