diff --git a/.do/inbox/2026-10-25-define-property-and-prototype-writes-bypass-traps.md b/.do/inbox/2026-10-25-define-property-and-prototype-writes-bypass-traps.md new file mode 100644 index 0000000..8a57c71 --- /dev/null +++ b/.do/inbox/2026-10-25-define-property-and-prototype-writes-bypass-traps.md @@ -0,0 +1,7 @@ +# `Object.defineProperty` and prototype writes bypass the traps + +**Status:** needs-triage + +The Proxy handler in `src/index.ts` at commit 18931ce defines `deleteProperty`, `get`, `has`, `ownKeys` and `set`. It has no `defineProperty`, `setPrototypeOf`, `getOwnPropertyDescriptor`, `getPrototypeOf` or `preventExtensions` trap, so those operations forward to the raw target without tracking, notification or hooks. A probe on 2026-10-25 (`probe-object.mjs`, session scratchpad) with an effect reading `$o.a` and an `onWrite` hook: `Object.defineProperty($o, 'a', { value: 2 })`, `Object.defineProperties` and `Reflect.defineProperty` change the raw value, wake nobody and call no hook; `Object.defineProperty($a, 'length', { value: 0 })` empties a reactive array the same silent way, while `$a.length = 0` notifies. `Object.setPrototypeOf($o, proto)` changes the prototype silently; `$o.__proto__ = proto` goes through the set trap, which assigns `rawTarget['__proto__']` and so does change the prototype, calls `onWrite` with property `__proto__`, and wakes only subscribers of `__proto__`, never the readers of an inherited property. Reads through `Object.getOwnPropertyDescriptor` and `Object.getPrototypeOf` are not tracked; `in`, `Object.keys` and `JSON.stringify` are. `Object.assign`, `Reflect.set` and `delete` behave like direct writes. `Object.freeze` and `Object.seal` forward silently and a later write throws `TypeError` from the strict-mode raw write, which the frozen and sealed tests already cover. Decide which of these count as supported write paths: a `defineProperty` trap that runs the set-trap bookkeeping would close the main gap; the prototype paths may be better documented as unsupported. No documentation names any of them (`grep` over `README.md`, `AGENTS.md`, `src/AGENTS.md` and the v2002.0.0 handbook tree). + +**Follow-up:** step 3 of 3 in the object-side chain proposed on 2026-10-25. Order: (1) `2026-10-25-drop-unread-hooks-slot-on-objects.md`, (2) `2026-10-25-fill-and-copywithin-not-batched.md`, (3) this note. Last because it needs a design decision before code: which `Object.*` paths Beacon supports (a `defineProperty` trap that runs the set-trap bookkeeping is the candidate), and whether `setPrototypeOf` and `__proto__` are documented as unsupported or intercepted. It also needs its own capability spec, since `array-mutation` covers the seven methods only; a new `object-mutation` capability under `openspec/specs/` would hold the trap contract for both kinds of target. Depends on step 1 for the hooked `state creation` benchmark case and trunk number, because the trap is added to the two handler objects in `src/index.ts`, `HOOKLESS_HANDLER` and the hooked one built in `state()`. New notifications and hook calls where there were none is a consumer-visible change, so it ships in the breaking release with PR 186 and step 2. The equal-value question in `2026-10-22-equal-value-write-in-sync-effect-raises-loop-error.md` should be settled in the same release, because the new trap would copy whichever order of loop check and equality check `performWrite` has at that time. diff --git a/.do/inbox/2026-10-25-drop-unread-hooks-slot-on-objects.md b/.do/inbox/2026-10-25-drop-unread-hooks-slot-on-objects.md new file mode 100644 index 0000000..4ef0391 --- /dev/null +++ b/.do/inbox/2026-10-25-drop-unread-hooks-slot-on-objects.md @@ -0,0 +1,11 @@ +# Drop the unread `HOOKS` slot write on hooked objects and the `frozenHooksCache` + +**Status:** ready-for-agent + +**Change:** drop-unread-hooks-slot-on-objects + +The maintainer ruled Q80 a on 2026-10-25: remove the dead work, close `HOOKS_TODO.md` item C1 with it. Facts at commit 18931ce in `src/index.ts`. `hooksSlotFor` returns the hooks object unchanged for a non-array target and a `DirectArrayContext` for an array. `definePropertyOnTarget` defines the `HOOKS` symbol on every extensible target whose slot is truthy, so each `state(obj, hooks)` call and each nested object wrapped through `wrapNestedObject` pays one `Object.defineProperty` that nothing reads: the only reads of `[HOOKS]` are `directPushWrapper` and `directPopWrapper`, and both act only on a `DirectArrayContext`. A hookless object passes `undefined` and skips the write. `storeHooksForFrozenTarget` writes hooked non-extensible targets into `frozenHooksCache`, which has one declaration and one `set` and no `get` anywhere in the file, so it is dead for objects and arrays alike. The hooks keep firing through the handler closures, as C1 already states. A micro probe of the two `defineProperty` calls on 500,000 fresh objects measured 102 ns per object for the `PROXY` symbol alone and 179 to 202 ns with the `HOOKS` symbol added; `scripts/benchmark.ts` has no hooked creation case (its three creation cases are hookless), so the suite does not show it. + +Smallest change: `hooksSlotFor` returns `undefined` for a non-array target; delete `storeHooksForFrozenTarget` and `frozenHooksCache`; `HooksSlot` narrows to `DirectArrayContext | undefined`; the `defineProxyProperties` catch and non-extensible branches keep returning `false` so the `proxyCache` entry still happens. Update the header comment in `tests/property-frozen-sealed.test.ts` that names `frozenHooksCache`, the `definePropertyOnTarget` row in `src/AGENTS.md`, and mark C1 shipped in `HOOKS_TODO.md`. No consumer-visible behavior changes: the symbol is module-private, so this is `perf(state)`, not breaking. Gate: the usual 1% per benchmark case against the base commit; add a hooked `state creation` case with a trunk number first (Q66 d) so the win is visible. + +**Follow-up:** step 1 of 3 in the object-side chain proposed on 2026-10-25. Order: (1) this note, (2) `2026-10-25-fill-and-copywithin-not-batched.md`, (3) `2026-10-25-define-property-and-prototype-writes-bypass-traps.md`. This step goes first because it is the only one with no consumer-visible change, so it can land and release on its own, and because it adds the hooked `state creation` benchmark case with its trunk number, which step 3 needs: a `defineProperty` trap is a sixth property on the handler object built once per hooked `state()` call, and that case is where its cost would show. Nothing depends on step 2. Touches `hooksSlotFor`, `definePropertyOnTarget`, `defineProxyProperties` and `HooksSlot` in `src/index.ts`; step 3 touches the two handler objects, `HOOKLESS_HANDLER` and the hooked one built in `state()`, so landing this first keeps the two diffs apart. diff --git a/.do/inbox/2026-10-25-fill-and-copywithin-not-batched.md b/.do/inbox/2026-10-25-fill-and-copywithin-not-batched.md new file mode 100644 index 0000000..a9f90c6 --- /dev/null +++ b/.do/inbox/2026-10-25-fill-and-copywithin-not-batched.md @@ -0,0 +1,56 @@ +# `fill` and `copyWithin` are mutating array methods that are not batched + +**Category:** enhancement +**Status:** ready-for-agent +**Change:** wrap-fill-and-copywithin + +`MUTATING_ARRAY_METHODS` in `src/index.ts` at commit 18931ce lists `push`, `pop`, `shift`, `unshift`, `splice`, `sort` and `reverse`. `Array.prototype.fill` and `Array.prototype.copyWithin` also mutate in place and are not in the set, so they run unwrapped with the Proxy as receiver: every element write passes the set trap on its own, outside any batch. A probe on 2026-10-25 (`probe-object2.mjs`, session scratchpad) with an effect reading all three indices of a reactive `[1, 2, 3]`: `fill(9)` re-ran the effect 3 times, `copyWithin(0, 1)` 2 times, `reverse()` once. Hooks and loop detection already see them because the writes go through the traps; only the one-wake-per-call promise of the `array-mutation` spec does not hold for them, and the spec names only the seven methods. Adding the two names to the set gives them the five-method wrapper (`applyInBatch` through the Proxy) with no other code change; the spec's method list and `AGENTS.md` would name nine methods. Benchmark impact: none expected on the seven existing cases, since the set lookup happens once per method name in the get trap and the result is cached in `arrayMethodCache`; a new case for `fill` would need a trunk number before judgement (Q66 d). + +**Follow-up:** step 2 of 3 in the object-side chain proposed on 2026-10-25. Order: (1) `2026-10-25-drop-unread-hooks-slot-on-objects.md`, (2) this note, (3) `2026-10-25-define-property-and-prototype-writes-bypass-traps.md`. No code dependency on step 1; it sits second because it is one line in `MUTATING_ARRAY_METHODS` plus the spec and doc lists, and it changes a consumer-visible wake count, fewer wakes per `fill` or `copyWithin` call, the same class of change the PR 186 description lists as breaking under ruling Q35 c. So it rides in the breaking release with PR 186, not on its own. Bundle its spec edit with the one `2026-10-23-per-property-wake-inside-effects.md` asks for, since both rewrite the "One wake per method call" requirement of `openspec/specs/array-mutation/spec.md`, and one change to that requirement is easier to review than two. + +## Triage notes + +Triaged on 2026-10-25 against trunk at 18f5a71 on Node 24.21.0. The claim reproduced: with an effect reading all three indices of a reactive `[1, 2, 3]`, `fill(9)` re-ran the effect 3 times, `copyWithin(0, 1)` 2 times and `reverse()` once, on hookless and on hooked state alike. Neither name appears in the library source, the tests, the handbook or the `array-mutation` spec, and `.do/out-of-scope/` holds nothing related, so this is neither implemented nor previously rejected. + +One correction to the report: loop detection does not treat the two methods like the seven. The spec's rule is that a method throws in a synchronous top-level effect because it reads the properties it writes. `fill` reads only `length` and writes indices, so `fill(9)` alone in such an effect does not throw; it throws only when the effect also read an index it fills. `copyWithin` throws only when its source and target ranges overlap (`copyWithin(0, 1)` on three elements throws, `copyWithin(2, 0, 1)` does not). That is the existing rule applied consistently, not a defect, and the brief keeps it. + +Rulings: Q85 (a) enhancement, ready-for-agent. Q86 (a) this issue stands alone; the per-property-wake issue of 2026-10-23 stays separate and may later reword the inside-effect semantics for all nine methods. At propose on the same day: Q87 (a) four tickets, Q88 (a) a straight chain, Q89 (a) one `fill` benchmark case on hookless state. The user also ruled that the benchmark case lands and is measured on the unchanged library before any library change, the same order Q66 d set for the previous change. + +## Agent brief + +**Summary:** `fill` and `copyWithin` join the set of mutating array methods that run through the Proxy inside a batch, so each wakes every subscriber of the array at most once per call, on both kinds of state, with a benchmark number taken on the unchanged library first. + +**Current behavior:** +Reactive state intercepts seven mutating array methods (`push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`): a read of one of those names on a reactive array returns a cached wrapper that runs the native method with the Proxy as receiver inside a batch, so every subscriber wakes at most once after the method returns. `Array.prototype.fill` and `Array.prototype.copyWithin` also mutate in place but are not in that set, so a read of either name returns the native function unwrapped. Each element write then passes the set trap on its own and wakes subscribers immediately: an effect reading every index of a three-element array re-runs three times for `fill(9)` and twice for `copyWithin(0, 1)`, where `reverse()` wakes it once. State hooks already observe both methods, because their reads and writes go through the traps. + +**Desired behavior:** +Both methods behave like the existing seven under every requirement of the `array-mutation` spec: one wake per subscriber per call from outside a batch, no wake when the call changes nothing, wakes deferred to the outermost boundary inside a user batch, loop detection by the existing rule with no new exemption, hooks seeing the method name and every property read and written, and return value and contents equal to the native method. The two names ship through the same cached-wrapper mechanism; no second mechanism and no direct fast path like the one push and pop have. + +Loop detection keeps its rule unchanged, and the spec says so explicitly for the two new names: `fill` writes properties it does not read, so it raises the loop error in a synchronous top-level effect only when that effect also read an index the call writes; `copyWithin` raises it only when its source and target ranges overlap, because only then does it write an index it read. The scenario that lists the methods which throw unconditionally does not gain the two names; a new scenario pins the conditional behavior for both. + +The benchmark suite gains one case for `fill` on a reactive array with one subscribed effect, shaped like the existing `array push/pop` cases, and it is committed and run on the unchanged library before the library change, so the change has a trunk number to compare against. The library change is then gated the usual way: no existing benchmark case regresses beyond the project's per-case threshold against the base commit, and the new case is reported before and after. + +**Key interfaces:** +- The set of method names the array method lookup consults in the get trap: gains `fill` and `copyWithin`; the lookup, the cache keyed by native method, and the batch-running wrapper are unchanged +- The `array-mutation` spec: its purpose and every method list name nine methods; the loop-detection requirement gains the conditional scenario above +- Root `AGENTS.md`, key behaviors: the sentence listing the intercepted methods names nine +- The benchmark script: one new case named in the style of the existing array cases +- Tests for array methods, their property-based counterpart, and the loop-detection tests: cover the two names on both kinds of state + +**Acceptance criteria:** +- [ ] A benchmark case for `fill` on a subscribed reactive array exists, and its first commit touches only the benchmark script; the full benchmark output from that commit, including the grand total line, is in that ticket's report +- [ ] From outside any batch, an effect reading every index of a reactive `[1, 2, 3]` re-runs exactly once after `fill(9)` and exactly once after `copyWithin(0, 1)`, on state created without hooks and on state created with hooks +- [ ] `fill` with the value every element already holds, and `copyWithin` whose copy changes no element, wake no subscriber +- [ ] Inside a user batch, neither method wakes a subscriber before the batch ends, and each subscriber wakes at most once after it +- [ ] In a synchronous top-level effect, `fill` raises the infinite-loop error when the effect read an index the call writes and raises nothing otherwise; `copyWithin` raises it when its source and target ranges overlap and raises nothing otherwise; a child effect and a microtask effect raise nothing for either +- [ ] On state with hooks, `onRead` sees the method name and every property the method reads, and `onWrite` sees every property the method writes with the same arguments a direct access produces +- [ ] Return value and contents after the call equal the native method on a plain array with the same contents and arguments, for the argument shapes both methods accept, including negative and out-of-range indices; a non-extensible array raises the native error unchanged +- [ ] The `array-mutation` spec and the root agent doc name nine methods, and the spec carries the conditional loop-detection scenario +- [ ] No existing benchmark case regresses beyond the project's per-case threshold against the base commit, and the new case's before and after figures are in the report + +**Out of scope:** +- A direct fast path for `fill` or `copyWithin` like the one push and pop have +- Any change to the inside-effect wake semantics; that is the per-property-wake issue of 2026-10-23 +- Writes that bypass the traps, such as `Object.defineProperty` or prototype writes; that is a separate inbox issue +- A version bump: the change rides the next breaking release under Q35 c +- Handbook pages: they wait for the release tree, as recorded for the previous array change diff --git a/.do/inbox/2026-10-25-rename-hooks-slot-parameter.md b/.do/inbox/2026-10-25-rename-hooks-slot-parameter.md new file mode 100644 index 0000000..7f5a104 --- /dev/null +++ b/.do/inbox/2026-10-25-rename-hooks-slot-parameter.md @@ -0,0 +1,5 @@ +# Rename the `hooks` parameter of the Proxy-setup functions to `slot` + +**Status:** needs-triage + +Review round 2 of ticket 02 in change `drop-unread-hooks-slot-on-objects` (commit `198e195`) left one low standards finding, recorded here instead of a third round. In `src/index.ts`, `definePropertyOnTarget(target, proxy, hooks: HooksSlot)` and `defineProxyProperties(target, proxy, hooks: HooksSlot)` keep the parameter name `hooks` although that change narrowed `HooksSlot` to `DirectArrayContext | undefined`, so the value is never a hooks object; the reviewer called it a possible Mysterious Name and proposed renaming the parameter to `slot` at both sites. The signature lines were not in the diff, only their meaning changed, which is why it was recorded rather than fixed. Smallest change: rename the parameter at the two definitions and nowhere else, since both are called positionally; a `refactor(state)` commit, no test or doc change, no benchmark gate needed. Could ride along with step 3 of the object-side chain (`2026-10-25-define-property-and-prototype-writes-bypass-traps.md`), which touches the same region of `state()`. diff --git a/.do/ledger/drop-unread-hooks-slot-on-objects.md b/.do/ledger/drop-unread-hooks-slot-on-objects.md new file mode 100644 index 0000000..9cf82f5 --- /dev/null +++ b/.do/ledger/drop-unread-hooks-slot-on-objects.md @@ -0,0 +1,14 @@ +# Ledger: drop-unread-hooks-slot-on-objects + +Ruling: Change branch is `perf/drop-unread-hooks-slot-on-objects`, reset to trunk `3376d26` on the user's answer to Q81 (a); the hand-made benchmark commit `e455235` from before the run was dropped so ticket 01's implementer produces it inside the reviewed process - the repository names branches `/` and the change's commit type is `perf` - if wrong, the branch is renamed before the pull request and nothing else moves. +Ruling: "The package's benchmark script at its default run count" in tickets 01 and 02 means `npm run benchmark`, which is `node --expose-gc scripts/benchmark.ts -R 3` - the previous change's ledger ruled the same and its baselines are at 3 cycles - if wrong, the two runs are at 3 cycles instead of 10 and the 1% gate is noisier than it could be. +Ruling: Ticket 02 criterion 1's shape test goes into `tests/state-hooks.test.ts`, and the test count for that file in `tests/AGENTS.md` moves from 18 to 19 - the test is about what `state(obj, hooks)` stores and that file holds the other hooked-state tests - if wrong, one test sits in a file whose name does not say what it covers. +Ruling: The design sentence "The `HOOKS` define then happens exactly for hooked arrays" was corrected to "exactly for array targets, hooked or not" before dispatch - the scout found the hookless path builds a direct-array context for an array too (`src/index.ts` lines 754 to 760 and 973), and ticket 02's own wording "only an array target keeps the slot" already matches the code - if wrong, a design sentence overstates the change and no code differs. +Ruling: Ticket 02's brief adds four prose sites the ticket does not name, on the user's answer to Q82 (a): the `HOOKS` symbol row in `src/AGENTS.md`, the two lines in `src/hooks/AGENTS.md` that say the symbol is never read and frozen targets go into the cache, and the comment in `tests/array-push-pop.test.ts` on a hooked object carrying hooks in the slot - the proposal's goal is that no prose describes the removed storage - if wrong, the ticket's commit touches two documents more than its criteria list. +Ruling: AGENTS.md files are maintained with the user's `agents-md-hierarchy` skill, so implementers edit only prose rows and leave the one-line index between the `BEACON-START` and `BEACON-END` markers untouched - no file is added or removed by this change, so the indexes stay true - if wrong, a later audit by that skill rewrites the index line. +Ruling: The ticket files are untracked, so a worktree does not contain them; the steering session writes each ticket's `## Report` section from the implementer's report - the implementer never touches the ticket file by the apply contract - if wrong, a report line is transcribed from the implementer's output rather than copied by it. +Verdict: 01 - approve - No findings; criteria 1 and 2 met in the diff, commit matches the convention, `npm run check` and `npm test` exit 0 in the worktree. Landed by fast-forward at `3be0a77`. +Ruling: The ticket 01 worktree `.claude/worktrees/agent-ae24f764692c76f84` and its branch `worktree-agent-ae24f764692c76f84` stay on disk - the merger's `git worktree remove` was stopped by the Bash sandbox with "Operation not permitted" after deleting 120 of 367 tracked files, and `git branch -d` refuses while the worktree is registered - if wrong, a half-deleted worktree holds nothing that is not at `3be0a77`, and the closing report carries the commands that remove it. +Ruling: Ticket 02 criterion 8 is read as the user stated on Q84: each case has a 1% ceiling on slowdowns against the ticket 01 baseline and no limit on speedups - the 3-cycle runs put 6 cases over 1% in one pairing and identical code varies by more than 1% on the same cases, so the gate was re-measured at 10 cycles in the order baseline, final, final, baseline, which left two cases over 1% in both pairings (`state creation` +1.4%/+1.1%, `array push/pop (hooks)` +2.0%/+1.1%), and a targeted probe of those two cases over 8 alternating processes per commit measured them at -0.59% and -0.37% with identical code spreading 2.6% - no case shows a slowdown above 1% that survives a measurement able to resolve 1%, and `state creation (hooks)` is 38% faster in every run, so the gate is met - if wrong, a sub-1% slowdown on a path the diff does not touch is hidden under the suite's noise floor, which the benchmark's own sd column already shows is above 1% on the small cases. +Verdict: 02 - approve - Round 1 verified all seven diff criteria with two low standards findings on commit message length, fixed by an amend to `198e195`; round 2 verified again with one low standards finding, the `hooks` parameter name on the narrowed slot type, recorded in `.do/inbox/2026-10-25-rename-hooks-slot-parameter.md`; checks exit 0 with coverage 100/100/100 in the worktree; gate met under the Q84 ruling. Landed by fast-forward at `198e195`. +Ruling: The ticket 02 worktree `.claude/worktrees/agent-a185ac3f1c4a26607` and its branch `worktree-agent-a185ac3f1c4a26607` stay on disk for the same reason as ticket 01's - the merger's `git worktree remove` was stopped by the Bash sandbox with "Operation not permitted", leaving 96 tracked files deleted in the working directory, and `git branch -d` refuses while the worktree is registered - if wrong, nothing is lost, every file is tracked content at `198e195`. diff --git a/.do/ledger/wrap-fill-and-copywithin.md b/.do/ledger/wrap-fill-and-copywithin.md new file mode 100644 index 0000000..bec3a67 --- /dev/null +++ b/.do/ledger/wrap-fill-and-copywithin.md @@ -0,0 +1,2 @@ +Ruling: Change branch is `fix/wrap-fill-and-copywithin`, created at trunk `18f5a71` on the user's answer to Q1 (a) - no earlier run of this change exists, the historian found `/` in every recent human PR branch, and the sibling change PR 186 used the `fix` type - if wrong, the branch is renamed before the pull request and nothing else moves. +Ruling: The uncommitted OpenSpec and .do/ files are committed on the change branch as `213bbcf` before the first dispatch, on the user's answer to Q2 (a) - worktrees check out tracked files only, and ticket 03 edits the main spec `openspec/specs/array-mutation/spec.md`, which was untracked - if wrong, one docs commit carries planning text onto the fix branch, which is harmless. diff --git a/.do/review/01-3376d26..3be0a77.diff b/.do/review/01-3376d26..3be0a77.diff new file mode 100644 index 0000000..9f07649 --- /dev/null +++ b/.do/review/01-3376d26..3be0a77.diff @@ -0,0 +1,86 @@ +# Review package: ticket 01, 3376d26..3be0a77 + +## Commits +### 3be0a77 +test(state): add hooked state creation benchmark case + +The blind-spots group names creation of a target that takes a second +symbol as a gap, but no case timed it. Add one shaped like the hookless +creation case with a single no-op onRead hook built once at module +level, so the next change can compare hooked creation against this +baseline. + + +## Files changed + scripts/benchmark.ts | 22 ++++++++++++++++++++++ + 1 file changed, 22 insertions(+) + +## Diff +diff --git a/scripts/benchmark.ts b/scripts/benchmark.ts +index e46ab1a..9685a7b 100644 +--- a/scripts/benchmark.ts ++++ b/scripts/benchmark.ts +@@ -458,20 +458,38 @@ function stateCreationSealed(): number { + for (let i = 0; i < STATE_CREATION_COUNT; i++) { + state( + Object.seal({ + v: i, + }) + ) + } + return performance.now() - start + } + ++// One hook is enough: any hook makes state() build the hooked handler and the slot value ++const creationHooks: StateHooks<{ v: number }> = { ++ onRead: (): void => {}, ++} ++ ++function stateCreationHooks(): number { ++ const start = performance.now() ++ for (let i = 0; i < STATE_CREATION_COUNT; i++) { ++ state( ++ { ++ v: i, ++ }, ++ creationHooks ++ ) ++ } ++ return performance.now() - start ++} ++ + function stateReadNested(): number { + const s = state({ + inner: { + v: 42, + }, + }) + const start = performance.now() + for (let i = 0; i < STATE_READ_COUNT; i++) { + void s.inner.v + } +@@ -624,20 +642,24 @@ const suite: SuiteEntry[] = [ + header: '--- Blind spots (creation shapes, nested read, equal and undefined writes) ---', + }, + { + fn: stateCreationArray, + name: 'state creation (array) ', + }, + { + fn: stateCreationSealed, + name: 'state creation (sealed) ', + }, ++ { ++ fn: stateCreationHooks, ++ name: 'state creation (hooks) ', ++ }, + { + fn: stateReadNested, + name: 'state read nested ', + }, + { + fn: stateWriteEqualValue, + name: 'state write equal value ', + }, + { + fn: arrayPushPopUndefined, diff --git a/.do/review/02-3be0a77..198e195.diff b/.do/review/02-3be0a77..198e195.diff new file mode 100644 index 0000000..69325cc --- /dev/null +++ b/.do/review/02-3be0a77..198e195.diff @@ -0,0 +1,420 @@ +# Review package: ticket 02, 3be0a77..198e195 + +## Commits +### 198e195 +perf(state): drop the hooks slot on objects and the frozen hooks cache + +A hooked non-array target kept its StateHooks under the HOOKS symbol, +and a hooked non-extensible target was written into frozenHooksCache, +but nothing read either: every trap reaches the hooks through its +handler closure. Only an array target needs the slot, for the +DirectArrayContext its direct push and pop read through the Proxy +receiver. + +hooksSlotFor now returns undefined for a non-array target, so the +symbol is defined exactly on extensible arrays, and the cache with its +writer is gone. The HooksSlot type says so. No consumer-visible change: +hooks fire as before on every kind of target, direct push and pop on +hooked arrays behave as before, and a non-extensible target still gets +its Proxy registered in proxyCache. + +Closes HOOKS_TODO.md item C1. + + +## Files changed + HOOKS_TODO.md | 2 +- + src/AGENTS.md | 12 +++++------- + src/hooks/AGENTS.md | 3 +-- + src/index.ts | 21 +++++++-------------- + tests/AGENTS.md | 2 +- + tests/array-push-pop.test.ts | 2 +- + tests/property-frozen-sealed.test.ts | 4 ++-- + tests/state-hooks.test.ts | 15 +++++++++++++++ + 8 files changed, 33 insertions(+), 28 deletions(-) + +## Diff +diff --git a/HOOKS_TODO.md b/HOOKS_TODO.md +index 8f70812..a5afd7c 100644 +--- a/HOOKS_TODO.md ++++ b/HOOKS_TODO.md +@@ -34,21 +34,21 @@ ## Hook contract + 6. `onCompute` fires before `computeFn` with the previous value. The new value is only visible through `onCacheHit`. + 7. Hooks bind by closure at `state()` / `effect()` / `derive()` / `batch()` call time. Nested objects wrapped later inherit the parent's hooks through that closure, not through any per-target storage. + 8. The presence of a hook never changes which subscribers wake or when. Hooked and hookless state share one set trap, so a write notifies the same subscribers at the same moment on every path: outside a batch, inside a batch from outside any effect, and inside an effect. + + Anything that needs veto, throw-through, delay, or post-run timing is `blocked` until a new slot or a return-value contract exists. + + ## Open core items + + | # | Item | Status | Evidence | + |---|------|--------|----------| +-| C1 | `frozenHooksCache` is written (`storeHooksForFrozenTarget`) and never read. The `HOOKS` symbol is defined on extensible targets and never read. Hooks on frozen targets fire through the closure either way. | `decide` | `grep -n frozenHooksCache src/index.ts` shows the declaration and one `set`; probe: `state(raw, {onRead})` then `Object.freeze(raw)`, a read fires the hook once | ++| C1 | The `HOOKS` symbol is defined only on extensible array targets, holding the `DirectArrayContext` their direct `push`/`pop` read; a non-array target gets no slot and no cache entry, since hooks fire through the handler closure on every target. | `shipped` | `tests/state-hooks.test.ts`, "defines the hooks symbol on an array target only" | + | C2 | `state(obj, hooksB)` on an already-proxied `obj` returns the cached proxy and drops `hooksB` silently. A nested object proxied before its parent keeps its own (possibly empty) hooks. | `decide` | probe: parent `onRead` sees `["inner"]` only; a second `state()` call fires hooksA, never hooksB | + | C3 | Reading a nested object through a frozen proxy throws a Proxy invariant `TypeError`, with or without hooks. | `open` | probe: `Object.freeze(state({nested:{b:2}}))` then `.nested.b` | + | C4 | File layout for built-in hooks: one file per hook under `src/hooks/` (original Phase 4 plan) or a single `src/hooks/builtins.ts`. | `decide` | none yet | + + ## Catalog + + Import path for anything built here is `@nerdalytics/beacon/hooks`. Per-hook subpaths such as `@nerdalytics/beacon/hooks/logRead` are not in the exports map and are not planned. + + ### Debugging + +diff --git a/src/AGENTS.md b/src/AGENTS.md +index 39f0baf..7d366f1 100644 +--- a/src/AGENTS.md ++++ b/src/AGENTS.md +@@ -52,30 +52,29 @@ ## EffectFunction Properties + | `__schedule` | `EffectSchedule \| undefined` | `'microtask'` routes scheduling through `pendingMicroEffects` | + | `effectName` | `string \| undefined` | Optional name for debugging/hooks | + + ## Internal Data Structures (WeakMap-based) + + | WeakMap | Type | Purpose | + |---------|------|---------| + | `proxyCache` | `object → object` | The Proxy of a target that could not take the `[PROXY]` symbol (non-extensible, or a define that threw); an extensible target carries its Proxy in `[PROXY]` and gets no entry, since an entry per `state()` cost 2.6x on creation (GC ephemeron work) | + | `proxyCacheSubs` | `object → Set` | Fallback subscriber storage for frozen/sealed objects | + | `arrayMethodCache` | `ArrayMethod → ArrayMethod` | One wrapper per native mutating array method; every wrapper acts on its call receiver, so no per-target entry is needed (`push` and `pop` map to `directPushWrapper`/`directPopWrapper`) | +-| `frozenHooksCache` | `object → StateHooks \| DirectArrayContext` | What the `[HOOKS]` slot would have held, for non-extensible targets; written, never read (see `HOOKS_TODO.md` C1) | + + ## Symbols + + | Symbol | Purpose | + |--------|---------| + | `OWN_KEYS_SYMBOL` | Tracks key enumeration reads (ownKeys trap) | + | `SUBSCRIBERS` | Stores the subscriber Set on target objects; absent until the first subscription and on targets that cannot take it (those keep the set in `proxyCacheSubs`) | + | `PROXY` | Stores Proxy reference on target objects | +-| `HOOKS` | On an extensible array target: its `DirectArrayContext` (raw target, raw hooks, composed `onDelete`/`onRead`/`onWrite`), which the direct `push`/`pop` read through the Proxy receiver (`proxy[HOOKS]` returns the raw slot; the raw target as receiver runs the native method silently, Q71 b). On any other extensible hooked target: its StateHooks, never read (nested propagation is by closure). A WeakMap keyed by the Proxy cost 2.7x on array state creation | ++| `HOOKS` | Only on an extensible array target: its `DirectArrayContext` (raw target, raw hooks, composed `onDelete`/`onRead`/`onWrite`), which the direct `push`/`pop` read through the Proxy receiver (`proxy[HOOKS]` returns the raw slot; the raw target as receiver runs the native method silently, Q71 b). A non-array target gets no slot: its hooks reach the traps by closure, and nested propagation is by closure too. A WeakMap keyed by the Proxy cost 2.7x on array state creation | + + ## Internal Function Architecture + + ### Proxy Handlers + + Five handler factories, each returns a ProxyHandler method. `state()` without hooks shares one prebuilt `HOOKLESS_HANDLER`; the get factory returns a leaner variant when no hooks are passed, the set factory returns one trap for every state object. + + | Handler | Traps | Key behavior | + |---------|-------|-------------| + | `createGetHandler()` | `get` | One keyed load of the raw target, then the internal-symbol early return (a separate load site for the symbols saw only array maps, lost its feedback when those arrays were collected, and deoptimized the trap on the next direct push/pop); delegates to `trackDependency` for reads and `wrapNestedObject` for nested wrapping; both variants route mutating array methods through `getWrappedArrayMethod` before the non-object early return | +@@ -91,22 +90,22 @@ ### Proxy Handlers + | `trackDependency` | First run: adds `currentEffect` to the target's subscribers, then `recordEffectRead`. Re-run (`isTrackingOnly`): `recordEffectRead` in silent mode | + | `recordEffectRead` | Non-silent: records into `__deps`/`__reads` + fires `onDependencyAdd`. Silent: advances the read-list replay while it matches, else replays the prefix into temp collections and falls back to `recordSilentRead` | + | `recordSilentRead` | Records into `rerunTempDeps`/`rerunTempReads` (and `buildingReadList`) without hooks or subscriber registration | + | `readListMatchesRead` | Checks the next `[target, prop]` pair of the replayed read list | + | `replayReadListToTempCollections` | Copies the matched read-list prefix into fresh temp collections when the fast path breaks | + | `wrapNestedObject` | Returns the nested object's `[PROXY]`, else `state()` (which finds a non-extensible object's Proxy in `proxyCache` or wraps it) | + | `isInternalSymbol` | Guards access to `SUBSCRIBERS`, `PROXY`, `HOOKS` symbols | + | `getWrappedArrayMethod` | Returns the wrapper for a mutating array method read from an array target (`Array.isArray` first, so plain-object reads pay one call and one check; then the `arrayMethodCache` hit, before the method-name check). Its body is a read-path hot spot: any change to it, even code a plain-object read never reaches, cost 2 to 5% on the read benchmark, so the direct `push`/`pop` dispatch lives in the cached wrappers | + | `applyInBatch` | Runs a method on a receiver inside the hookless batch bookkeeping (`batchDepth++`, then `abortBatch` or `endBatch`, without the `batch(fn)` closure); shared by the Proxy-receiver wrappers and the direct `push`/`pop` | + | `wrapArrayMethod` | Wraps a native mutating array method once, cached in `arrayMethodCache`: the native `push` and `pop` get `directPushWrapper`/`directPopWrapper`; the other five get a wrapper that applies them via `applyInBatch` to the call receiver (the Proxy), so every read and write passes the traps (seen by `onRead`/`onWrite`, subject to the loop check inside an effect) and each subscriber wakes at most once per call | +-| `directPushWrapper` / `directPopWrapper` | Read the receiver's `[HOOKS]` slot (optional chaining, so a nullish receiver reads no slot): a `DirectArrayContext` (checked with `instanceof`, since a hooked object receiver keeps its hooks there) on a receiver that is not the context's own raw target runs `directPush`/`directPop` on it inside `applyInBatch`; any other receiver (such as `push.call(otherArray)`, the raw array target, `null` or `undefined`) gets the native method inside `applyInBatch`: the raw target never enters the traps, so the call is silent (no hook call, no wake, Q71 b), and a nullish receiver raises the native TypeError | +-| `hooksSlotFor` | What `state()` stores in the `[HOOKS]` slot: for an array target a new `DirectArrayContext`, otherwise the hooks | ++| `directPushWrapper` / `directPopWrapper` | Read the receiver's `[HOOKS]` slot (optional chaining, so a nullish receiver reads no slot): a `DirectArrayContext` (checked with `instanceof`, so only a reactive array's slot qualifies) on a receiver that is not the context's own raw target runs `directPush`/`directPop` on it inside `applyInBatch`; any other receiver (such as `push.call(otherArray)`, the raw array target, `null` or `undefined`) gets the native method inside `applyInBatch`: the raw target never enters the traps, so the call is silent (no hook call, no wake, Q71 b), and a nullish receiver raises the native TypeError | ++| `hooksSlotFor` | What `state()` stores in the `[HOOKS]` slot: for an array target a new `DirectArrayContext`, otherwise `undefined` (no slot) | + | `directPush` / `directPop` | Run the native method on the raw target inside `applyInBatch` and replay by hand what the traps would have done: `push` reads `length`, then sets each new index and `length`; `pop` reads `length` and the last index, deletes that index (`onDelete`, readers of the index scheduled, no loop or equal check) and sets `length`; `pop` returns the nested wrapper the get trap would | + | `recordDirectRead` | The get trap's bookkeeping for a property the native method read: `trackDependency` inside an effect, then `onRead` | + | `recordDirectWrite` | The set trap's bookkeeping for a write the native method already performed: the loop check (inside an effect only), the equal skip, then `onWrite`, shared by both paths; only the scheduling branches: per property inside an effect (plus `length` for an index write that grew the array), one `ARRAY_DIRTY_PROPS` entry per batch outside. A write that grew the array went to an absent index, so it is never skipped as equal (Q65 a) | + | `isUnchangedWrite` | The equal skip of every set path: `Object.is` against the current read, and for an equal `undefined` a `prop in rawTarget` check, so a write of `undefined` to an absent property counts as a change (Q65 a); a read that is not `undefined` proves the property exists | + | `performUnbatchedWrite` | Outside any effect and batch: skips an unchanged write (`isUnchangedWrite`), skips scheduling when the target has no subscribers, else notifies per property | + | `performWrite` | Checks infinite loop, skips an unchanged write (`isUnchangedWrite`), notifies subscribers | + | `checkInfiniteLoop` | Throws if effect writes to a property it read (exempt: child effects, microtask effects) | + | `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty | + | `getArrayLengthBeforeMutation` | Captures array length before index assignment | + | `notifyLengthChangeIfNeeded` | Notifies `length` subscribers if array grew/shrank | +@@ -203,23 +202,22 @@ ### Settle Observation + | `onIdle()` | Public API — adds a callback to `idleCallbacks`, returns an unsubscribe | + | `notifyIfIdle` | Calls idle callbacks (errors swallowed) when the graph is idle (no effect queued, no microtask flush armed) and work happened since the last call; a pending async derive does not block it | + | `notifyIdleOutsideFlush` | Notify entry outside `flushEffects`: sets `isNotifying` around `notifyIfIdle` so callback writes queue, then `flushEffects` | + | `until()` | Public API — throws inside a reactive scope, rejects at once on an aborted signal, else wraps `runUntil` in a Promise | + | `runUntil` | Runs the predicate in an effect; resolves on a microtask once it is truthy and no read derive was pending; `timeout`/`signal` reject; a throwing predicate rejects with its error on a microtask instead of propagating to the writer; disposes its effect, timer and abort listener on resolve or reject | + + ### Proxy Setup + + | Function | Purpose | + |----------|---------| +-| `defineProxyProperties` | Stores `[PROXY]` and `[HOOKS]` symbols on extensible targets and returns `true`; falls back to `storeHooksForFrozenTarget` and returns `false` otherwise, which makes `state()` register the Proxy in `proxyCache` | +-| `definePropertyOnTarget` | `Object.defineProperty` for the `PROXY` symbol, and for the `HOOKS` symbol when the slot has a value | +-| `storeHooksForFrozenTarget` | Writes the `HOOKS` slot value to `frozenHooksCache` for non-extensible targets | ++| `defineProxyProperties` | Stores the `[PROXY]` symbol, and `[HOOKS]` on an array, on an extensible target and returns `true`; returns `false` for a non-extensible target or a define that threw, which makes `state()` register the Proxy in `proxyCache` | ++| `definePropertyOnTarget` | `Object.defineProperty` for the `PROXY` symbol, and for the `HOOKS` symbol when the slot has a value (array targets only) | + + ### Utilities + + | Function | Purpose | + |----------|---------| + | `callHookSafe` | Invokes hook in try-catch — hook errors never propagate to core | + | `didEffectReadProp` | Checks if effect read a specific property (for infinite loop detection) | + | `depsMatch` | Compares two dep sets by size + per-target property sets | + + ## Critical Invariants +diff --git a/src/hooks/AGENTS.md b/src/hooks/AGENTS.md +index 429bff0..b66e44a 100644 +--- a/src/hooks/AGENTS.md ++++ b/src/hooks/AGENTS.md +@@ -44,16 +44,15 @@ ## Integration with Core + - `state(initial, hooks?: StateHooks)` + - `effect(fn, name?, hooks?: EffectHooks, options?: EffectOptions)` + - `derive(computeFn, hooks?: DeriveHooks)` + - `batch(fn, hooks?: BatchHooks)` + + The core `index.ts` imports `composeHook` from `./hooks/compose.ts`. Published as the `./hooks` subpath export (`package.json`, `jsr.json`). + + ## Design Constraints + + - All hook invocations wrapped in try-catch — never propagate to core +-- State hooks propagate to nested objects by closure: the get handler passes the parent's hooks to `state(value, hooks)` when wrapping a nested object. The `HOOKS` symbol is defined on extensible targets but never read +-- Non-extensible targets get their hooks stored in `frozenHooksCache`; nothing reads it (see `HOOKS_TODO.md` item C1) ++- State hooks propagate to nested objects by closure: the get handler passes the parent's hooks to `state(value, hooks)` when wrapping a nested object. The `HOOKS` symbol is defined only on extensible array targets, holding the `DirectArrayContext` their direct `push`/`pop` read; no target stores its hooks object anywhere + - All hooks fire synchronously during their respective operations + - Types defined in `../types.ts`, not in this module + + [Hooks Module Index]|root: ./src/hooks|IMPORTANT: Hook types live in ../types.ts. This module only re-exports and composes.|.:{index.ts,compose.ts} +diff --git a/src/index.ts b/src/index.ts +index dfdcc7b..e103daa 100644 +--- a/src/index.ts ++++ b/src/index.ts +@@ -102,21 +102,21 @@ type ArrayMethod = (...args: unknown[]) => unknown + const arrayMethodCache: WeakMap = new WeakMap() + // Only these two take the direct path (Q52 a); any other function under their names keeps the Proxy receiver + const NATIVE_PUSH: ArrayMethod = Array.prototype.push as ArrayMethod + const NATIVE_POP: ArrayMethod = Array.prototype.pop as ArrayMethod + + type ArrayTarget = ProxyTarget & unknown[] + + // What the direct push and pop of one array act on: its raw target and its composed hooks. An array target + // carries it in its [HOOKS] slot, where the shared wrappers read it through the Proxy receiver's internal-symbol + // guard: a WeakMap keyed by the Proxy cost 2.7x on array state creation (GC ephemeron work). A class, so the +-// wrappers can tell it from the hooks object a non-array target keeps in the same slot ++// wrappers can tell a reactive array's slot from whatever another receiver holds under the same symbol + class DirectArrayContext { + readonly hooks: StateHooks | undefined + readonly onDelete: HookFunction<[PropertyKey, boolean, object]> | undefined + readonly onRead: HookFunction<[PropertyKey, unknown, object]> | undefined + readonly onWrite: HookFunction<[PropertyKey, unknown, unknown, object]> | undefined + readonly target: ArrayTarget + + constructor( + target: ArrayTarget, + hooks: StateHooks | undefined, +@@ -125,24 +125,24 @@ class DirectArrayContext { + onWrite: HookFunction<[PropertyKey, unknown, unknown, object]> | undefined + ) { + this.hooks = hooks + this.onDelete = onDelete + this.onRead = onRead + this.onWrite = onWrite + this.target = target + } + } + +-// What the [HOOKS] slot of a target holds: the hooks of a non-array target, the direct context of an array +-type HooksSlot = StateHooks | DirectArrayContext | undefined ++// What the [HOOKS] slot holds: the direct context of an array target; a non-array target has no slot, since ++// its hooks reach every trap by closure ++type HooksSlot = DirectArrayContext | undefined + +-const frozenHooksCache: WeakMap = new WeakMap() + const MAX_IDLE_CYCLES = 100 + const MAX_MICROTASK_FLUSH_CHAIN = 100 + // Up to this many subscribers a scan is cheaper than the per-property index lookups + const INDEXED_SCHEDULE_MIN_SUBSCRIBERS = 4 + + // Hooks consulted on the hot path; stored on the effect only when at least one is set + type EffectInternalHooks = { + onDependencyAdd?: HookFunction<[object, PropertyKey, string | undefined]> | undefined + onDependencyChange?: HookFunction<[object, PropertyKey]> | undefined + onSchedule?: HookFunction<[string | undefined]> | undefined +@@ -736,29 +736,29 @@ function createGetHandler( + callHookSafe(onRead, prop, value, rawTarget as T) + + const wrapped = getWrappedArrayMethod(rawTarget, prop, value) + if (wrapped) return wrapped + + if (value === null || typeof value !== 'object') return value + return wrapNestedObject(value as object, hooks as StateHooks | undefined) + } + } + +-// An array target's [HOOKS] slot holds the context its direct push and pop find through the Proxy receiver ++// Only an array target gets a [HOOKS] slot: the context its direct push and pop find through the Proxy receiver + function hooksSlotFor( + target: ProxyTarget, + hooks: StateHooks | undefined, + onDelete: HookFunction<[PropertyKey, boolean, T]> | undefined, + onRead: HookFunction<[PropertyKey, unknown, T]> | undefined, + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined + ): HooksSlot { +- if (!Array.isArray(target)) return hooks as StateHooks | undefined ++ if (!Array.isArray(target)) return undefined + return new DirectArrayContext( + target as ArrayTarget, + hooks as StateHooks | undefined, + onDelete as DirectArrayContext['onDelete'], + onRead as DirectArrayContext['onRead'], + onWrite as DirectArrayContext['onWrite'] + ) + } + + function createHasHandler( +@@ -904,26 +904,20 @@ function performWrite( + rawTarget[prop] = value + + callHookSafe(onWrite, prop, oldValue, value, rawTarget as T) + + scheduleSubscribersForTarget(rawTarget, prop) + notifyLengthChangeIfNeeded(rawTarget, oldLength) + + return true + } + +-function storeHooksForFrozenTarget(target: ProxyTarget, hooks: HooksSlot): void { +- if (hooks) { +- frozenHooksCache.set(target, hooks) +- } +-} +- + function definePropertyOnTarget(target: ProxyTarget, proxy: unknown, hooks: HooksSlot): void { + Object.defineProperty(target, PROXY, { + configurable: true, + enumerable: false, + value: proxy, + writable: false, + }) + if (hooks) { + Object.defineProperty(target, HOOKS, { + configurable: true, +@@ -934,23 +928,22 @@ function definePropertyOnTarget(target: ProxyTarget, proxy: unknown, hooks: Hook + } + } + + // Returns whether the target took the [PROXY] symbol; only a target that did not needs the proxyCache entry + function defineProxyProperties(target: ProxyTarget, proxy: unknown, hooks: HooksSlot): boolean { + try { + if (Object.isExtensible(target)) { + definePropertyOnTarget(target, proxy, hooks) + return true + } +- storeHooksForFrozenTarget(target, hooks) + } catch { +- storeHooksForFrozenTarget(target, hooks) ++ // a Proxy target that rejects symbol defines + } + return false + } + + const HOOKLESS_HANDLER: ProxyHandler = { + deleteProperty: createDeleteHandler(undefined), + get: createGetHandler(undefined, undefined), + has: createHasHandler(undefined), + ownKeys: createOwnKeysHandler(undefined), + set: createSetHandler(undefined), +diff --git a/tests/AGENTS.md b/tests/AGENTS.md +index 98de6ab..4dae5d6 100644 +--- a/tests/AGENTS.md ++++ b/tests/AGENTS.md +@@ -85,17 +85,17 @@ ## Coverage Gate + - Lines: 90% + - Includes: `src/**/*.ts` + - Excludes: `scripts/**/*.ts` + + ## Hooks Tests + + Test hooks instrumentation for each primitive. Each file verifies backward compatibility (works without hooks), individual hook firing, and error isolation (hook errors never break core). + + | File | Tests | Covers | + |------|-------|--------| +-| `state-hooks.test.ts` | 18 | `onRead`, `onWrite`, batched `onWrite`, `onDelete`, `onHas`, `onOwnKeys`, nested propagation, nested proxy wrapping, array methods, multiple hooks | ++| `state-hooks.test.ts` | 19 | `onRead`, `onWrite`, batched `onWrite`, `onDelete`, `onHas`, `onOwnKeys`, nested propagation, nested proxy wrapping, array methods, multiple hooks, hooks symbol on array targets only | + | `effect-hooks.test.ts` | 11 | `onRun`, `onDispose`, `onError`, `onDependencyAdd`, `onSchedule` | + | `derive-hooks.test.ts` | 12 | `onCompute`, `onCacheHit`, `onDispose`, `onError`, `onDependencyChange` | + | `batch-hooks.test.ts` | 7 | `onBatchStart`, `onBatchEnd`, `onBatchError`, nested depth | + | `hooks-compose.test.ts` | 7 | `composeHook` utility — undefined, single, array, forwarding, error isolation | + + [Tests Index]|root: ./tests|IMPORTANT: Follow naming conventions and always dispose effects/derives in tests|.:{*.test.ts,PROPERTY_BASED_TESTING.md,biome.json} +diff --git a/tests/array-push-pop.test.ts b/tests/array-push-pop.test.ts +index 793fc01..12d6abf 100644 +--- a/tests/array-push-pop.test.ts ++++ b/tests/array-push-pop.test.ts +@@ -418,21 +418,21 @@ describe('Array push and pop', { concurrency: true, timeout: 1000 }, (): void => + assert.strictEqual(method.call(rawList, ...args), result, `${name} ${kind}`) + assert.deepStrictEqual(rawList, after, `${name} ${kind}`) + assert.deepStrictEqual(calls, [], `${name} ${kind}`) + assert.deepStrictEqual(subscription.woken(), [], `${name} ${kind}`) + subscription.dispose() + } + } + }) + + it('runs push and pop called with another receiver on that receiver', (): void => { +- // A hooked object state is the receiver that carries hooks where an array carries its direct context ++ // A hooked object state has no slot where an array carries its direct context, so it takes the native path + const calls: HookCall[] = [] + const rawObject: Record = { length: 0 } + const $object = state(rawObject, recordingHooks(calls, rawObject as unknown as unknown[])) + for (const [kind, $list] of bothKinds([0])) { + const subscription = subscribe($list) + const plain = [9] + + assert.strictEqual($list.push.call(plain, 1), 2, kind) + assert.strictEqual($list.pop.call(plain), 1, kind) + assert.strictEqual($list.push.call($object, 1), 1, kind) +diff --git a/tests/property-frozen-sealed.test.ts b/tests/property-frozen-sealed.test.ts +index e666236..3f853c0 100644 +--- a/tests/property-frozen-sealed.test.ts ++++ b/tests/property-frozen-sealed.test.ts +@@ -1,22 +1,22 @@ + import assert from 'node:assert/strict' + import { describe, it } from 'node:test' + import fc from 'fast-check' + import { batch, derive, effect, state } from '../src/index.ts' + + /** + * Property-based tests for frozen and sealed object reactivity. + * + * When Object.isExtensible(target) is false, Beacon falls back to WeakMap + * storage for subscribers (proxyCacheSubs). Array method wrappers always +- * live in arrayMethodCache, and frozenHooksCache is written but never read; +- * hooks fire through the handler closure. These tests verify that the ++ * live in arrayMethodCache, and hooks fire through the handler closure, so ++ * a non-extensible target stores nothing else. These tests verify that the + * subscriber fallback path works correctly. + * + * Key behavior notes: + * - Every write path resolves subscribers by checking rawTarget[SUBSCRIBERS] + * first and proxyCacheSubs second. So direct writes, batched writes and + * cross-effect writes to sealed objects all notify the effects that read + * the written property. + * - Frozen objects cannot be written to (properties are non-writable). + * + * @see tests/PROPERTY_BASED_TESTING.md — Opportunity #11 +diff --git a/tests/state-hooks.test.ts b/tests/state-hooks.test.ts +index 1239abf..e2153d4 100644 +--- a/tests/state-hooks.test.ts ++++ b/tests/state-hooks.test.ts +@@ -361,11 +361,26 @@ describe('State Hooks', { + { + onRead: (prop: PropertyKey) => { + reads.push(prop) + }, + } + ) + + assert.strictEqual($frozen.limit, 10) + assert.deepStrictEqual(reads, ['limit']) + }) ++ ++ it('defines the hooks symbol on an array target only', () => { ++ // Counted before any read or subscription: the subscribers symbol is added lazily on first subscription ++ const hooks = { onRead: (): void => {} } ++ const hookedObject = { count: 0 } ++ const hookedArray = [0] ++ const hooklessObject = { count: 0 } ++ state(hookedObject, hooks) ++ state(hookedArray, hooks) ++ state(hooklessObject) ++ ++ assert.strictEqual(Object.getOwnPropertySymbols(hookedObject).length, 1) ++ assert.strictEqual(Object.getOwnPropertySymbols(hookedArray).length, 2) ++ assert.strictEqual(Object.getOwnPropertySymbols(hooklessObject).length, 1) ++ }) + }) diff --git a/.do/review/02-3be0a77..73c6e7d.diff b/.do/review/02-3be0a77..73c6e7d.diff new file mode 100644 index 0000000..3ebccfc --- /dev/null +++ b/.do/review/02-3be0a77..73c6e7d.diff @@ -0,0 +1,419 @@ +# Review package: ticket 02, 3be0a77..73c6e7d + +## Commits +### 73c6e7d +perf(state): drop the unread hooks slot on objects and the frozen hooks cache + +A hooked non-array target kept its StateHooks under the HOOKS symbol, and +a hooked non-extensible target was written into frozenHooksCache, but +nothing read either: every trap reaches the hooks through its handler +closure. Only an array target needs the slot, for the DirectArrayContext +its direct push and pop read through the Proxy receiver. + +hooksSlotFor now returns undefined for a non-array target, so the symbol +is defined exactly on extensible arrays, and the cache with its writer is +gone. The HooksSlot type says so. No consumer-visible change: hooks fire +as before on every kind of target, direct push and pop on hooked arrays +behave as before, and a non-extensible target still gets its Proxy +registered in proxyCache. + +Closes HOOKS_TODO.md item C1. + + +## Files changed + HOOKS_TODO.md | 2 +- + src/AGENTS.md | 12 +++++------- + src/hooks/AGENTS.md | 3 +-- + src/index.ts | 21 +++++++-------------- + tests/AGENTS.md | 2 +- + tests/array-push-pop.test.ts | 2 +- + tests/property-frozen-sealed.test.ts | 4 ++-- + tests/state-hooks.test.ts | 15 +++++++++++++++ + 8 files changed, 33 insertions(+), 28 deletions(-) + +## Diff +diff --git a/HOOKS_TODO.md b/HOOKS_TODO.md +index 8f70812..a5afd7c 100644 +--- a/HOOKS_TODO.md ++++ b/HOOKS_TODO.md +@@ -34,21 +34,21 @@ ## Hook contract + 6. `onCompute` fires before `computeFn` with the previous value. The new value is only visible through `onCacheHit`. + 7. Hooks bind by closure at `state()` / `effect()` / `derive()` / `batch()` call time. Nested objects wrapped later inherit the parent's hooks through that closure, not through any per-target storage. + 8. The presence of a hook never changes which subscribers wake or when. Hooked and hookless state share one set trap, so a write notifies the same subscribers at the same moment on every path: outside a batch, inside a batch from outside any effect, and inside an effect. + + Anything that needs veto, throw-through, delay, or post-run timing is `blocked` until a new slot or a return-value contract exists. + + ## Open core items + + | # | Item | Status | Evidence | + |---|------|--------|----------| +-| C1 | `frozenHooksCache` is written (`storeHooksForFrozenTarget`) and never read. The `HOOKS` symbol is defined on extensible targets and never read. Hooks on frozen targets fire through the closure either way. | `decide` | `grep -n frozenHooksCache src/index.ts` shows the declaration and one `set`; probe: `state(raw, {onRead})` then `Object.freeze(raw)`, a read fires the hook once | ++| C1 | The `HOOKS` symbol is defined only on extensible array targets, holding the `DirectArrayContext` their direct `push`/`pop` read; a non-array target gets no slot and no cache entry, since hooks fire through the handler closure on every target. | `shipped` | `tests/state-hooks.test.ts`, "defines the hooks symbol on an array target only" | + | C2 | `state(obj, hooksB)` on an already-proxied `obj` returns the cached proxy and drops `hooksB` silently. A nested object proxied before its parent keeps its own (possibly empty) hooks. | `decide` | probe: parent `onRead` sees `["inner"]` only; a second `state()` call fires hooksA, never hooksB | + | C3 | Reading a nested object through a frozen proxy throws a Proxy invariant `TypeError`, with or without hooks. | `open` | probe: `Object.freeze(state({nested:{b:2}}))` then `.nested.b` | + | C4 | File layout for built-in hooks: one file per hook under `src/hooks/` (original Phase 4 plan) or a single `src/hooks/builtins.ts`. | `decide` | none yet | + + ## Catalog + + Import path for anything built here is `@nerdalytics/beacon/hooks`. Per-hook subpaths such as `@nerdalytics/beacon/hooks/logRead` are not in the exports map and are not planned. + + ### Debugging + +diff --git a/src/AGENTS.md b/src/AGENTS.md +index 39f0baf..7d366f1 100644 +--- a/src/AGENTS.md ++++ b/src/AGENTS.md +@@ -52,30 +52,29 @@ ## EffectFunction Properties + | `__schedule` | `EffectSchedule \| undefined` | `'microtask'` routes scheduling through `pendingMicroEffects` | + | `effectName` | `string \| undefined` | Optional name for debugging/hooks | + + ## Internal Data Structures (WeakMap-based) + + | WeakMap | Type | Purpose | + |---------|------|---------| + | `proxyCache` | `object → object` | The Proxy of a target that could not take the `[PROXY]` symbol (non-extensible, or a define that threw); an extensible target carries its Proxy in `[PROXY]` and gets no entry, since an entry per `state()` cost 2.6x on creation (GC ephemeron work) | + | `proxyCacheSubs` | `object → Set` | Fallback subscriber storage for frozen/sealed objects | + | `arrayMethodCache` | `ArrayMethod → ArrayMethod` | One wrapper per native mutating array method; every wrapper acts on its call receiver, so no per-target entry is needed (`push` and `pop` map to `directPushWrapper`/`directPopWrapper`) | +-| `frozenHooksCache` | `object → StateHooks \| DirectArrayContext` | What the `[HOOKS]` slot would have held, for non-extensible targets; written, never read (see `HOOKS_TODO.md` C1) | + + ## Symbols + + | Symbol | Purpose | + |--------|---------| + | `OWN_KEYS_SYMBOL` | Tracks key enumeration reads (ownKeys trap) | + | `SUBSCRIBERS` | Stores the subscriber Set on target objects; absent until the first subscription and on targets that cannot take it (those keep the set in `proxyCacheSubs`) | + | `PROXY` | Stores Proxy reference on target objects | +-| `HOOKS` | On an extensible array target: its `DirectArrayContext` (raw target, raw hooks, composed `onDelete`/`onRead`/`onWrite`), which the direct `push`/`pop` read through the Proxy receiver (`proxy[HOOKS]` returns the raw slot; the raw target as receiver runs the native method silently, Q71 b). On any other extensible hooked target: its StateHooks, never read (nested propagation is by closure). A WeakMap keyed by the Proxy cost 2.7x on array state creation | ++| `HOOKS` | Only on an extensible array target: its `DirectArrayContext` (raw target, raw hooks, composed `onDelete`/`onRead`/`onWrite`), which the direct `push`/`pop` read through the Proxy receiver (`proxy[HOOKS]` returns the raw slot; the raw target as receiver runs the native method silently, Q71 b). A non-array target gets no slot: its hooks reach the traps by closure, and nested propagation is by closure too. A WeakMap keyed by the Proxy cost 2.7x on array state creation | + + ## Internal Function Architecture + + ### Proxy Handlers + + Five handler factories, each returns a ProxyHandler method. `state()` without hooks shares one prebuilt `HOOKLESS_HANDLER`; the get factory returns a leaner variant when no hooks are passed, the set factory returns one trap for every state object. + + | Handler | Traps | Key behavior | + |---------|-------|-------------| + | `createGetHandler()` | `get` | One keyed load of the raw target, then the internal-symbol early return (a separate load site for the symbols saw only array maps, lost its feedback when those arrays were collected, and deoptimized the trap on the next direct push/pop); delegates to `trackDependency` for reads and `wrapNestedObject` for nested wrapping; both variants route mutating array methods through `getWrappedArrayMethod` before the non-object early return | +@@ -91,22 +90,22 @@ ### Proxy Handlers + | `trackDependency` | First run: adds `currentEffect` to the target's subscribers, then `recordEffectRead`. Re-run (`isTrackingOnly`): `recordEffectRead` in silent mode | + | `recordEffectRead` | Non-silent: records into `__deps`/`__reads` + fires `onDependencyAdd`. Silent: advances the read-list replay while it matches, else replays the prefix into temp collections and falls back to `recordSilentRead` | + | `recordSilentRead` | Records into `rerunTempDeps`/`rerunTempReads` (and `buildingReadList`) without hooks or subscriber registration | + | `readListMatchesRead` | Checks the next `[target, prop]` pair of the replayed read list | + | `replayReadListToTempCollections` | Copies the matched read-list prefix into fresh temp collections when the fast path breaks | + | `wrapNestedObject` | Returns the nested object's `[PROXY]`, else `state()` (which finds a non-extensible object's Proxy in `proxyCache` or wraps it) | + | `isInternalSymbol` | Guards access to `SUBSCRIBERS`, `PROXY`, `HOOKS` symbols | + | `getWrappedArrayMethod` | Returns the wrapper for a mutating array method read from an array target (`Array.isArray` first, so plain-object reads pay one call and one check; then the `arrayMethodCache` hit, before the method-name check). Its body is a read-path hot spot: any change to it, even code a plain-object read never reaches, cost 2 to 5% on the read benchmark, so the direct `push`/`pop` dispatch lives in the cached wrappers | + | `applyInBatch` | Runs a method on a receiver inside the hookless batch bookkeeping (`batchDepth++`, then `abortBatch` or `endBatch`, without the `batch(fn)` closure); shared by the Proxy-receiver wrappers and the direct `push`/`pop` | + | `wrapArrayMethod` | Wraps a native mutating array method once, cached in `arrayMethodCache`: the native `push` and `pop` get `directPushWrapper`/`directPopWrapper`; the other five get a wrapper that applies them via `applyInBatch` to the call receiver (the Proxy), so every read and write passes the traps (seen by `onRead`/`onWrite`, subject to the loop check inside an effect) and each subscriber wakes at most once per call | +-| `directPushWrapper` / `directPopWrapper` | Read the receiver's `[HOOKS]` slot (optional chaining, so a nullish receiver reads no slot): a `DirectArrayContext` (checked with `instanceof`, since a hooked object receiver keeps its hooks there) on a receiver that is not the context's own raw target runs `directPush`/`directPop` on it inside `applyInBatch`; any other receiver (such as `push.call(otherArray)`, the raw array target, `null` or `undefined`) gets the native method inside `applyInBatch`: the raw target never enters the traps, so the call is silent (no hook call, no wake, Q71 b), and a nullish receiver raises the native TypeError | +-| `hooksSlotFor` | What `state()` stores in the `[HOOKS]` slot: for an array target a new `DirectArrayContext`, otherwise the hooks | ++| `directPushWrapper` / `directPopWrapper` | Read the receiver's `[HOOKS]` slot (optional chaining, so a nullish receiver reads no slot): a `DirectArrayContext` (checked with `instanceof`, so only a reactive array's slot qualifies) on a receiver that is not the context's own raw target runs `directPush`/`directPop` on it inside `applyInBatch`; any other receiver (such as `push.call(otherArray)`, the raw array target, `null` or `undefined`) gets the native method inside `applyInBatch`: the raw target never enters the traps, so the call is silent (no hook call, no wake, Q71 b), and a nullish receiver raises the native TypeError | ++| `hooksSlotFor` | What `state()` stores in the `[HOOKS]` slot: for an array target a new `DirectArrayContext`, otherwise `undefined` (no slot) | + | `directPush` / `directPop` | Run the native method on the raw target inside `applyInBatch` and replay by hand what the traps would have done: `push` reads `length`, then sets each new index and `length`; `pop` reads `length` and the last index, deletes that index (`onDelete`, readers of the index scheduled, no loop or equal check) and sets `length`; `pop` returns the nested wrapper the get trap would | + | `recordDirectRead` | The get trap's bookkeeping for a property the native method read: `trackDependency` inside an effect, then `onRead` | + | `recordDirectWrite` | The set trap's bookkeeping for a write the native method already performed: the loop check (inside an effect only), the equal skip, then `onWrite`, shared by both paths; only the scheduling branches: per property inside an effect (plus `length` for an index write that grew the array), one `ARRAY_DIRTY_PROPS` entry per batch outside. A write that grew the array went to an absent index, so it is never skipped as equal (Q65 a) | + | `isUnchangedWrite` | The equal skip of every set path: `Object.is` against the current read, and for an equal `undefined` a `prop in rawTarget` check, so a write of `undefined` to an absent property counts as a change (Q65 a); a read that is not `undefined` proves the property exists | + | `performUnbatchedWrite` | Outside any effect and batch: skips an unchanged write (`isUnchangedWrite`), skips scheduling when the target has no subscribers, else notifies per property | + | `performWrite` | Checks infinite loop, skips an unchanged write (`isUnchangedWrite`), notifies subscribers | + | `checkInfiniteLoop` | Throws if effect writes to a property it read (exempt: child effects, microtask effects) | + | `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty | + | `getArrayLengthBeforeMutation` | Captures array length before index assignment | + | `notifyLengthChangeIfNeeded` | Notifies `length` subscribers if array grew/shrank | +@@ -203,23 +202,22 @@ ### Settle Observation + | `onIdle()` | Public API — adds a callback to `idleCallbacks`, returns an unsubscribe | + | `notifyIfIdle` | Calls idle callbacks (errors swallowed) when the graph is idle (no effect queued, no microtask flush armed) and work happened since the last call; a pending async derive does not block it | + | `notifyIdleOutsideFlush` | Notify entry outside `flushEffects`: sets `isNotifying` around `notifyIfIdle` so callback writes queue, then `flushEffects` | + | `until()` | Public API — throws inside a reactive scope, rejects at once on an aborted signal, else wraps `runUntil` in a Promise | + | `runUntil` | Runs the predicate in an effect; resolves on a microtask once it is truthy and no read derive was pending; `timeout`/`signal` reject; a throwing predicate rejects with its error on a microtask instead of propagating to the writer; disposes its effect, timer and abort listener on resolve or reject | + + ### Proxy Setup + + | Function | Purpose | + |----------|---------| +-| `defineProxyProperties` | Stores `[PROXY]` and `[HOOKS]` symbols on extensible targets and returns `true`; falls back to `storeHooksForFrozenTarget` and returns `false` otherwise, which makes `state()` register the Proxy in `proxyCache` | +-| `definePropertyOnTarget` | `Object.defineProperty` for the `PROXY` symbol, and for the `HOOKS` symbol when the slot has a value | +-| `storeHooksForFrozenTarget` | Writes the `HOOKS` slot value to `frozenHooksCache` for non-extensible targets | ++| `defineProxyProperties` | Stores the `[PROXY]` symbol, and `[HOOKS]` on an array, on an extensible target and returns `true`; returns `false` for a non-extensible target or a define that threw, which makes `state()` register the Proxy in `proxyCache` | ++| `definePropertyOnTarget` | `Object.defineProperty` for the `PROXY` symbol, and for the `HOOKS` symbol when the slot has a value (array targets only) | + + ### Utilities + + | Function | Purpose | + |----------|---------| + | `callHookSafe` | Invokes hook in try-catch — hook errors never propagate to core | + | `didEffectReadProp` | Checks if effect read a specific property (for infinite loop detection) | + | `depsMatch` | Compares two dep sets by size + per-target property sets | + + ## Critical Invariants +diff --git a/src/hooks/AGENTS.md b/src/hooks/AGENTS.md +index 429bff0..b66e44a 100644 +--- a/src/hooks/AGENTS.md ++++ b/src/hooks/AGENTS.md +@@ -44,16 +44,15 @@ ## Integration with Core + - `state(initial, hooks?: StateHooks)` + - `effect(fn, name?, hooks?: EffectHooks, options?: EffectOptions)` + - `derive(computeFn, hooks?: DeriveHooks)` + - `batch(fn, hooks?: BatchHooks)` + + The core `index.ts` imports `composeHook` from `./hooks/compose.ts`. Published as the `./hooks` subpath export (`package.json`, `jsr.json`). + + ## Design Constraints + + - All hook invocations wrapped in try-catch — never propagate to core +-- State hooks propagate to nested objects by closure: the get handler passes the parent's hooks to `state(value, hooks)` when wrapping a nested object. The `HOOKS` symbol is defined on extensible targets but never read +-- Non-extensible targets get their hooks stored in `frozenHooksCache`; nothing reads it (see `HOOKS_TODO.md` item C1) ++- State hooks propagate to nested objects by closure: the get handler passes the parent's hooks to `state(value, hooks)` when wrapping a nested object. The `HOOKS` symbol is defined only on extensible array targets, holding the `DirectArrayContext` their direct `push`/`pop` read; no target stores its hooks object anywhere + - All hooks fire synchronously during their respective operations + - Types defined in `../types.ts`, not in this module + + [Hooks Module Index]|root: ./src/hooks|IMPORTANT: Hook types live in ../types.ts. This module only re-exports and composes.|.:{index.ts,compose.ts} +diff --git a/src/index.ts b/src/index.ts +index dfdcc7b..e103daa 100644 +--- a/src/index.ts ++++ b/src/index.ts +@@ -102,21 +102,21 @@ type ArrayMethod = (...args: unknown[]) => unknown + const arrayMethodCache: WeakMap = new WeakMap() + // Only these two take the direct path (Q52 a); any other function under their names keeps the Proxy receiver + const NATIVE_PUSH: ArrayMethod = Array.prototype.push as ArrayMethod + const NATIVE_POP: ArrayMethod = Array.prototype.pop as ArrayMethod + + type ArrayTarget = ProxyTarget & unknown[] + + // What the direct push and pop of one array act on: its raw target and its composed hooks. An array target + // carries it in its [HOOKS] slot, where the shared wrappers read it through the Proxy receiver's internal-symbol + // guard: a WeakMap keyed by the Proxy cost 2.7x on array state creation (GC ephemeron work). A class, so the +-// wrappers can tell it from the hooks object a non-array target keeps in the same slot ++// wrappers can tell a reactive array's slot from whatever another receiver holds under the same symbol + class DirectArrayContext { + readonly hooks: StateHooks | undefined + readonly onDelete: HookFunction<[PropertyKey, boolean, object]> | undefined + readonly onRead: HookFunction<[PropertyKey, unknown, object]> | undefined + readonly onWrite: HookFunction<[PropertyKey, unknown, unknown, object]> | undefined + readonly target: ArrayTarget + + constructor( + target: ArrayTarget, + hooks: StateHooks | undefined, +@@ -125,24 +125,24 @@ class DirectArrayContext { + onWrite: HookFunction<[PropertyKey, unknown, unknown, object]> | undefined + ) { + this.hooks = hooks + this.onDelete = onDelete + this.onRead = onRead + this.onWrite = onWrite + this.target = target + } + } + +-// What the [HOOKS] slot of a target holds: the hooks of a non-array target, the direct context of an array +-type HooksSlot = StateHooks | DirectArrayContext | undefined ++// What the [HOOKS] slot holds: the direct context of an array target; a non-array target has no slot, since ++// its hooks reach every trap by closure ++type HooksSlot = DirectArrayContext | undefined + +-const frozenHooksCache: WeakMap = new WeakMap() + const MAX_IDLE_CYCLES = 100 + const MAX_MICROTASK_FLUSH_CHAIN = 100 + // Up to this many subscribers a scan is cheaper than the per-property index lookups + const INDEXED_SCHEDULE_MIN_SUBSCRIBERS = 4 + + // Hooks consulted on the hot path; stored on the effect only when at least one is set + type EffectInternalHooks = { + onDependencyAdd?: HookFunction<[object, PropertyKey, string | undefined]> | undefined + onDependencyChange?: HookFunction<[object, PropertyKey]> | undefined + onSchedule?: HookFunction<[string | undefined]> | undefined +@@ -736,29 +736,29 @@ function createGetHandler( + callHookSafe(onRead, prop, value, rawTarget as T) + + const wrapped = getWrappedArrayMethod(rawTarget, prop, value) + if (wrapped) return wrapped + + if (value === null || typeof value !== 'object') return value + return wrapNestedObject(value as object, hooks as StateHooks | undefined) + } + } + +-// An array target's [HOOKS] slot holds the context its direct push and pop find through the Proxy receiver ++// Only an array target gets a [HOOKS] slot: the context its direct push and pop find through the Proxy receiver + function hooksSlotFor( + target: ProxyTarget, + hooks: StateHooks | undefined, + onDelete: HookFunction<[PropertyKey, boolean, T]> | undefined, + onRead: HookFunction<[PropertyKey, unknown, T]> | undefined, + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined + ): HooksSlot { +- if (!Array.isArray(target)) return hooks as StateHooks | undefined ++ if (!Array.isArray(target)) return undefined + return new DirectArrayContext( + target as ArrayTarget, + hooks as StateHooks | undefined, + onDelete as DirectArrayContext['onDelete'], + onRead as DirectArrayContext['onRead'], + onWrite as DirectArrayContext['onWrite'] + ) + } + + function createHasHandler( +@@ -904,26 +904,20 @@ function performWrite( + rawTarget[prop] = value + + callHookSafe(onWrite, prop, oldValue, value, rawTarget as T) + + scheduleSubscribersForTarget(rawTarget, prop) + notifyLengthChangeIfNeeded(rawTarget, oldLength) + + return true + } + +-function storeHooksForFrozenTarget(target: ProxyTarget, hooks: HooksSlot): void { +- if (hooks) { +- frozenHooksCache.set(target, hooks) +- } +-} +- + function definePropertyOnTarget(target: ProxyTarget, proxy: unknown, hooks: HooksSlot): void { + Object.defineProperty(target, PROXY, { + configurable: true, + enumerable: false, + value: proxy, + writable: false, + }) + if (hooks) { + Object.defineProperty(target, HOOKS, { + configurable: true, +@@ -934,23 +928,22 @@ function definePropertyOnTarget(target: ProxyTarget, proxy: unknown, hooks: Hook + } + } + + // Returns whether the target took the [PROXY] symbol; only a target that did not needs the proxyCache entry + function defineProxyProperties(target: ProxyTarget, proxy: unknown, hooks: HooksSlot): boolean { + try { + if (Object.isExtensible(target)) { + definePropertyOnTarget(target, proxy, hooks) + return true + } +- storeHooksForFrozenTarget(target, hooks) + } catch { +- storeHooksForFrozenTarget(target, hooks) ++ // a Proxy target that rejects symbol defines + } + return false + } + + const HOOKLESS_HANDLER: ProxyHandler = { + deleteProperty: createDeleteHandler(undefined), + get: createGetHandler(undefined, undefined), + has: createHasHandler(undefined), + ownKeys: createOwnKeysHandler(undefined), + set: createSetHandler(undefined), +diff --git a/tests/AGENTS.md b/tests/AGENTS.md +index 98de6ab..4dae5d6 100644 +--- a/tests/AGENTS.md ++++ b/tests/AGENTS.md +@@ -85,17 +85,17 @@ ## Coverage Gate + - Lines: 90% + - Includes: `src/**/*.ts` + - Excludes: `scripts/**/*.ts` + + ## Hooks Tests + + Test hooks instrumentation for each primitive. Each file verifies backward compatibility (works without hooks), individual hook firing, and error isolation (hook errors never break core). + + | File | Tests | Covers | + |------|-------|--------| +-| `state-hooks.test.ts` | 18 | `onRead`, `onWrite`, batched `onWrite`, `onDelete`, `onHas`, `onOwnKeys`, nested propagation, nested proxy wrapping, array methods, multiple hooks | ++| `state-hooks.test.ts` | 19 | `onRead`, `onWrite`, batched `onWrite`, `onDelete`, `onHas`, `onOwnKeys`, nested propagation, nested proxy wrapping, array methods, multiple hooks, hooks symbol on array targets only | + | `effect-hooks.test.ts` | 11 | `onRun`, `onDispose`, `onError`, `onDependencyAdd`, `onSchedule` | + | `derive-hooks.test.ts` | 12 | `onCompute`, `onCacheHit`, `onDispose`, `onError`, `onDependencyChange` | + | `batch-hooks.test.ts` | 7 | `onBatchStart`, `onBatchEnd`, `onBatchError`, nested depth | + | `hooks-compose.test.ts` | 7 | `composeHook` utility — undefined, single, array, forwarding, error isolation | + + [Tests Index]|root: ./tests|IMPORTANT: Follow naming conventions and always dispose effects/derives in tests|.:{*.test.ts,PROPERTY_BASED_TESTING.md,biome.json} +diff --git a/tests/array-push-pop.test.ts b/tests/array-push-pop.test.ts +index 793fc01..12d6abf 100644 +--- a/tests/array-push-pop.test.ts ++++ b/tests/array-push-pop.test.ts +@@ -418,21 +418,21 @@ describe('Array push and pop', { concurrency: true, timeout: 1000 }, (): void => + assert.strictEqual(method.call(rawList, ...args), result, `${name} ${kind}`) + assert.deepStrictEqual(rawList, after, `${name} ${kind}`) + assert.deepStrictEqual(calls, [], `${name} ${kind}`) + assert.deepStrictEqual(subscription.woken(), [], `${name} ${kind}`) + subscription.dispose() + } + } + }) + + it('runs push and pop called with another receiver on that receiver', (): void => { +- // A hooked object state is the receiver that carries hooks where an array carries its direct context ++ // A hooked object state has no slot where an array carries its direct context, so it takes the native path + const calls: HookCall[] = [] + const rawObject: Record = { length: 0 } + const $object = state(rawObject, recordingHooks(calls, rawObject as unknown as unknown[])) + for (const [kind, $list] of bothKinds([0])) { + const subscription = subscribe($list) + const plain = [9] + + assert.strictEqual($list.push.call(plain, 1), 2, kind) + assert.strictEqual($list.pop.call(plain), 1, kind) + assert.strictEqual($list.push.call($object, 1), 1, kind) +diff --git a/tests/property-frozen-sealed.test.ts b/tests/property-frozen-sealed.test.ts +index e666236..3f853c0 100644 +--- a/tests/property-frozen-sealed.test.ts ++++ b/tests/property-frozen-sealed.test.ts +@@ -1,22 +1,22 @@ + import assert from 'node:assert/strict' + import { describe, it } from 'node:test' + import fc from 'fast-check' + import { batch, derive, effect, state } from '../src/index.ts' + + /** + * Property-based tests for frozen and sealed object reactivity. + * + * When Object.isExtensible(target) is false, Beacon falls back to WeakMap + * storage for subscribers (proxyCacheSubs). Array method wrappers always +- * live in arrayMethodCache, and frozenHooksCache is written but never read; +- * hooks fire through the handler closure. These tests verify that the ++ * live in arrayMethodCache, and hooks fire through the handler closure, so ++ * a non-extensible target stores nothing else. These tests verify that the + * subscriber fallback path works correctly. + * + * Key behavior notes: + * - Every write path resolves subscribers by checking rawTarget[SUBSCRIBERS] + * first and proxyCacheSubs second. So direct writes, batched writes and + * cross-effect writes to sealed objects all notify the effects that read + * the written property. + * - Frozen objects cannot be written to (properties are non-writable). + * + * @see tests/PROPERTY_BASED_TESTING.md — Opportunity #11 +diff --git a/tests/state-hooks.test.ts b/tests/state-hooks.test.ts +index 1239abf..e2153d4 100644 +--- a/tests/state-hooks.test.ts ++++ b/tests/state-hooks.test.ts +@@ -361,11 +361,26 @@ describe('State Hooks', { + { + onRead: (prop: PropertyKey) => { + reads.push(prop) + }, + } + ) + + assert.strictEqual($frozen.limit, 10) + assert.deepStrictEqual(reads, ['limit']) + }) ++ ++ it('defines the hooks symbol on an array target only', () => { ++ // Counted before any read or subscription: the subscribers symbol is added lazily on first subscription ++ const hooks = { onRead: (): void => {} } ++ const hookedObject = { count: 0 } ++ const hookedArray = [0] ++ const hooklessObject = { count: 0 } ++ state(hookedObject, hooks) ++ state(hookedArray, hooks) ++ state(hooklessObject) ++ ++ assert.strictEqual(Object.getOwnPropertySymbols(hookedObject).length, 1) ++ assert.strictEqual(Object.getOwnPropertySymbols(hookedArray).length, 2) ++ assert.strictEqual(Object.getOwnPropertySymbols(hooklessObject).length, 1) ++ }) + }) diff --git a/AGENTS.md b/AGENTS.md index cee1f32..38443eb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ Single-file core: `src/index.ts` (~1400 lines). Zero external dependencies. Prox - Nested objects automatically wrapped in Proxies (deep reactivity) - Infinite loop detection: throws if effect writes to a state property it reads from (child effects and microtask effects are exempt; microtask re-runs stop after 100 chained microtask flushes with a `console.error`) - Cyclic dependencies between different effects: allowed via queue-based non-recursive `flushEffects` -- Array mutating methods intercepted on state with and without hooks: `push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse` +- Array mutating methods intercepted on state with and without hooks: `push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`, `fill`, `copyWithin` - Each runs through the Proxy inside a batch, so it wakes each subscriber of the array at most once per call, is subject to the loop check inside an effect, and is seen by the read and write hooks - Frozen/sealed objects: fallback to WeakMap storage - `derive()` must be disposed to prevent memory leaks (creates internal state + effect pair) diff --git a/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/.openspec.yaml b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/.openspec.yaml new file mode 100644 index 0000000..f5d81df --- /dev/null +++ b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/.openspec.yaml @@ -0,0 +1,3 @@ +schema: do +created: 2026-10-25 +skip_specs: true diff --git a/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/design.md b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/design.md new file mode 100644 index 0000000..96857b8 --- /dev/null +++ b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/design.md @@ -0,0 +1,45 @@ +# Design + +## Context + +See proposal.md, Why. Facts at commit `3376d26` in `src/index.ts`: + +- `state()` builds a slot value through `hooksSlotFor`: a new `DirectArrayContext` for an array target, the hooks object itself for any other target, `undefined` without hooks. `defineProxyProperties` passes it to `definePropertyOnTarget`, which defines the `PROXY` symbol on every extensible target and the `HOOKS` symbol whenever the slot is truthy. For a non-extensible target, or when the define throws, it hands the slot to `storeHooksForFrozenTarget`, which writes it into `frozenHooksCache`, and returns `false` so `state()` puts the Proxy in `proxyCache`. +- The `HOOKS` slot has two readers, `directPushWrapper` and `directPopWrapper`. Both read `this?.[HOOKS]`, act only when the value is an `instanceof DirectArrayContext` and the receiver is not the context's own raw target, and otherwise run the native method inside a batch. A hooks object in the slot is never acted on. `frozenHooksCache` has no reader at all. +- Hooks reach the traps by closure: `createGetHandler(onRead, hooks)` and the other factories capture them at `state()` time, and `wrapNestedObject` passes the parent's hooks object into the nested `state()` call. Hook contract fact 7 in `HOOKS_TODO.md` states this. The frozen-object hook test in `tests/state-hooks.test.ts` passes on this path today. +- The traps' internal-symbol guard (`isInternalSymbol`) still lists `HOOKS`; it stays, because array targets keep the slot. +- Coverage thresholds are 100% branches and 100% functions (`node.config.json`), so every branch that survives must still be reached by a test. +- The benchmark suite's creation cases (`state creation`, `state creation (array)`, `state creation (sealed)`) all call `state()` without hooks; the "Blind spots" header comment already names "creation of a target that takes a second symbol" as a gap. + +## Goals / Non-Goals + +**Goals:** +- Remove the two writes that nothing reads, so hooked object creation does the same symbol work as hookless creation. +- Make the win measurable: a hooked creation case in the suite with a number from the unchanged library. +- Leave every observable behavior as it is: hooks fire the same on extensible and non-extensible targets, direct `push` and `pop` on hooked arrays behave the same, `proxyCache` registration for non-extensible targets is unchanged. +- Prose that describes the removed storage says the same as the code afterwards. + +**Non-Goals:** +- Moving the array context out of the `HOOKS` slot. A WeakMap keyed by the Proxy was measured at 2.7x on array creation when the slot was introduced; the slot stays for arrays. +- Any change to the trap handlers or to `HOOKLESS_HANDLER`; step 3 of the chain touches those. +- A version bump or handbook change. + +## Decisions + +- **`hooksSlotFor` returns `undefined` for a non-array target; the define call keeps its truthiness check.** The `HOOKS` define then happens exactly for array targets, hooked or not, since the hookless path builds a context for an array too, through the existing `if (hooks)` arm, which stays a two-way branch (array, everything else) and stays covered. Alternative: a second `Array.isArray` inside `definePropertyOnTarget`. Rejected: `hooksSlotFor` already answers that question, and one place deciding what the slot holds is easier to keep true. +- **`HooksSlot` narrows to `DirectArrayContext | undefined`.** The type then says what the slot can hold, and the `ProxyObject` helper type follows. The `instanceof` check in the two direct wrappers stays: it is what rejects a nullish receiver, a foreign receiver with no slot, and (defensively) any non-context value. Only its comment changes, since there is no hooks object to tell apart any more. +- **Delete `frozenHooksCache` and `storeHooksForFrozenTarget` outright.** Both the non-extensible arm and the `catch` arm of `defineProxyProperties` reduce to returning `false`. The `catch` arm stays, because a define can throw for a non-extensible target seen after the `isExtensible` check and the existing branch coverage reaches it. Alternative: keep an empty function as a seam. Rejected: it is the dead code the maintainer asked to remove. +- **One new benchmark case, `state creation (hooks)`, under the "Blind spots" header, shaped like `state creation` with one hooks object built once outside the loop.** Any non-empty hooks object triggers the slot write, so a single no-op hook is enough and keeps the hooked trap work minimal, which makes the symbol define the dominant difference from the hookless case. It lands in its own ticket before the library change so its baseline is the unchanged library (Q66 d). +- **Gate: every benchmark case's median on the final commit at most 1% above the same case on the baseline run from ticket 01, on the same machine and Node version.** This is the per-case rule the inbox note names. The change removes work from the hooked creation path and touches no other path, so no case has a reason to move up; the hooked creation case is expected to move down. +- **One test asserts the storage shape:** the raw target of a hooked plain object owns exactly one symbol, the raw target of a hooked array owns two, and a hookless object owns one. It is red on trunk for the object case and green after, which gives the implementer the red-then-green cycle for the removal. Behavior tests for frozen hooked objects and for direct `push`/`pop` already exist and must stay green. + +## Risks / Trade-offs + +- [Benchmark noise: creation cases show a standard deviation of 15 to 20% of the median across cycles in the last recorded run] → compare medians from full runs at the default run count on the same machine; on a disagreement with the expected direction, rerun both commits back to back before concluding, and record both outputs in the ticket report. +- [A branch that only the deleted cache reached drops coverage below 100%] → the deleted branches disappear with the code; the surviving `catch` arm and the `if (hooks)` arm keep their existing tests. The coverage run in the ticket catches the rest. +- [Prose drift: five documents name the removed storage] → the ticket lists each one as an acceptance criterion; the implementer greps for the two deleted identifiers before finishing and finds none outside the archived change folders. +- [Step 3 of the chain expects the hooked creation case with a trunk number] → ticket 01's report carries the full output of the baseline run; nothing else from this change is a dependency of step 3. + +## Migration Plan + +Two commits on one change branch, each green on its own: the benchmark case, then the removal with its prose updates. No data, API shape, or manifest changes. Rollback: revert the squash commit. Merging to `trunk` publishes nothing because the publish workflow is gated by a version change. diff --git a/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/proposal.md b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/proposal.md new file mode 100644 index 0000000..d414073 --- /dev/null +++ b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/proposal.md @@ -0,0 +1,35 @@ +# Proposal + +## Why + +Every `state(obj, hooks)` call on a non-array target, and every nested object wrapped through its hooked parent, pays one `Object.defineProperty` for the module-private `HOOKS` symbol that nothing ever reads: the only readers of that slot are the direct `push` and `pop` wrappers, and both act only on the `DirectArrayContext` an array target holds. A hooked non-extensible target is written into `frozenHooksCache`, a WeakMap with one declaration, one `set` and no `get`. The hooks keep firing through the handler closures either way, as `HOOKS_TODO.md` item C1 records. The maintainer ruled on 2026-10-25 (Q80 a, `.do/inbox/2026-10-25-drop-unread-hooks-slot-on-objects.md`) to remove the dead work and close C1 with it. A micro probe in that note measured the second symbol define at 77 to 100 ns per fresh object on top of the 102 ns the `PROXY` symbol costs, and the benchmark suite cannot show it because none of its three creation cases passes hooks. + +## What Changes + +- A hooked non-array target no longer takes the `HOOKS` symbol. Only an array target keeps the slot, holding the `DirectArrayContext` its direct `push` and `pop` read. The slot type narrows accordingly. +- `frozenHooksCache` and the function that writes it are deleted. The non-extensible and define-threw branches keep returning `false`, so `state()` still registers the Proxy in `proxyCache` for those targets. +- The benchmark suite gains one creation case that passes hooks, run on the unchanged library first so the win has a trunk number (Q66 d). Gate: the usual 1% per benchmark case against the base commit; a removal of work is expected to come in at or below it. +- Stale prose that names the removed storage is updated: the test file header that says `frozenHooksCache` is written but never read, the source comment that says the array context shares its slot with a hooks object, the `src/AGENTS.md` rows for the two WeakMaps, `hooksSlotFor`, `definePropertyOnTarget`, `storeHooksForFrozenTarget` and the direct wrappers, and `HOOKS_TODO.md` C1 moves from `decide` to `shipped`. +- No consumer-visible behavior changes. The symbol is module-private, hooks fire exactly as before on extensible and non-extensible targets, and the internal-symbol guard in the traps is unchanged. Commit type `perf(state)`, not breaking. +- No version bump in this change (assumption, recorded here: the inbox note says the step can release on its own, not that it must; the trunk publish workflow is gated by a version change, so merging publishes nothing until the maintainer bumps). + +Out of scope: steps 2 and 3 of the object-side chain the inbox note lists (`fill` and `copyWithin` batching; `defineProperty` and prototype writes bypassing the traps). Step 3 reuses the hooked creation benchmark case this change adds. + +## Capabilities + +### New Capabilities + +None. No externally observable behavior is introduced. + +### Modified Capabilities + +None. No requirement in `async-derive`, `effect-scheduling`, `handbook-docs` or `settle-observation` describes the internal symbol or the cache, and no consumer-visible behavior changes. The change sets `skip_specs: true`. + +## Impact + +- `src/index.ts`: `hooksSlotFor`, `definePropertyOnTarget`, `defineProxyProperties`, the `HooksSlot` type, the `frozenHooksCache` declaration and `storeHooksForFrozenTarget`; one comment on `DirectArrayContext`. +- `tests/`: one new assertion that a hooked object carries only the Proxy symbol and a hooked array carries both; the header comment of `property-frozen-sealed.test.ts`. Existing frozen-with-hooks and array direct push/pop tests must stay green; coverage thresholds (100% branches, 100% functions) must hold after the deleted branches. +- `scripts/benchmark.ts`: one new case. +- `src/AGENTS.md`, `HOOKS_TODO.md`: the rows named above. +- `package.json`, `jsr.json`, handbook: unchanged. +- Consumers: none. A hooked object has one own symbol instead of two, which only code enumerating own symbols of the raw target could tell, and no public API returns the raw target. diff --git a/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/tickets/01-hooked-state-creation-benchmark-and-baseline.md b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/tickets/01-hooked-state-creation-benchmark-and-baseline.md new file mode 100644 index 0000000..a3ffb67 --- /dev/null +++ b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/tickets/01-hooked-state-creation-benchmark-and-baseline.md @@ -0,0 +1,62 @@ +# 01: Hooked state creation benchmark case and the trunk baseline + +**Area:** state + +**Blocked by:** None (can start immediately) + +**What to build:** The benchmark suite gains one case that times what this change speeds up: creating state objects with hooks. It is shaped like the existing hookless `state creation` case, with one hooks object holding a single no-op hook, built once outside the loop and passed to every call. It sits in the blind-spots group, whose header comment already names a creation that takes a second symbol as a gap. Nothing else changes in this ticket, so the numbers it records are the baseline of the unchanged library. The full suite is then run with the package's benchmark script at its default run count, and the complete output, including every case line and the grand total line, goes into the ticket report. Ticket 02 compares its final run against this output case by case, and step 3 of the inbox chain reads the same number later, so the report holds the exact figures, not a summary. + +- [x] The benchmark suite lists one new case that creates the same number of state objects as the hookless creation case, each with the same hooks object, and reports through the same result printer as the other cases +- [x] The library source, tests, docs and manifests are unchanged in this ticket (`git diff --stat` against the branch base lists only the benchmark script) +- [x] The ticket report contains the full output of the package's benchmark script from a run on this ticket's commit, with the Node version and machine named + +## Report + +Commit `3be0a77`, `test(state): add hooked state creation benchmark case`, made by the ticket's implementer in worktree `.claude/worktrees/agent-ae24f764692c76f84` on branch `worktree-agent-ae24f764692c76f84`, started at BASE `3376d26`. `git diff --stat 3376d26 3be0a77` lists only `scripts/benchmark.ts | 22 ++++++++++++++++++++++`. The implementer reported `npm run check` exit 0 and `npm test` exit 0 with 307 passing tests. Run with `npm run benchmark` on that commit, Node v24.21.0, Apple M4 Pro (arm64), Darwin 25.6.0. Full output as the implementer reported it: + +``` +> @nerdalytics/beacon@2002.0.0 benchmark +> node --expose-gc scripts/benchmark.ts -R 3 + +=== Beacon Benchmark (1,000,000 iterations, 3 cycles × 7 samples = 21 total) === + +classic loop : med=1.78ms mean=1.76ms min=1.54ms max=1.96ms sd=0.13ms heap=11kb total=36.90ms avg/cycle=12.30ms total mem=238kb avg mem/cycle=79kb +state no subs : med=39.65ms mean=35.83ms min=25.80ms max=42.48ms sd=6.82ms heap=10kb total=752.42ms avg/cycle=250.81ms total mem=267kb avg mem/cycle=89kb +state + derive : med=170.56ms mean=161.76ms min=131.89ms max=178.50ms sd=14.78ms heap=68853kb total=3397.01ms avg/cycle=1132.34ms total mem=1048499kb avg mem/cycle=349500kb +state + derive + 2 effects: med=315.15ms mean=302.06ms min=267.43ms max=324.91ms sd=21.50ms heap=63802kb total=6343.26ms avg/cycle=2114.42ms total mem=1014664kb avg mem/cycle=338221kb +batch + derive : med=42.98ms mean=39.87ms min=32.00ms max=44.34ms sd=4.93ms heap=19kb total=837.19ms avg/cycle=279.06ms total mem=297kb avg mem/cycle=99kb +batch + derive + 2 effects: med=42.50ms mean=39.69ms min=31.77ms max=44.59ms sd=5.25ms heap=24kb total=833.52ms avg/cycle=277.84ms total mem=612kb avg mem/cycle=204kb + +--- Targeted benchmarks --- + +state creation : med=9.80ms mean=9.84ms min=9.52ms max=10.31ms sd=0.17ms heap=15680kb total=206.74ms avg/cycle=68.91ms total mem=329320kb avg mem/cycle=109773kb +state read (no effect) : med=16.64ms mean=16.63ms min=15.99ms max=17.53ms sd=0.38ms heap=7kb total=349.26ms avg/cycle=116.42ms total mem=146kb avg mem/cycle=49kb +state write 1 sub : med=12.70ms mean=12.96ms min=11.75ms max=15.07ms sd=0.86ms heap=16131kb total=272.22ms avg/cycle=90.74ms total mem=327129kb avg mem/cycle=109043kb +state write 100 subs : med=84.60ms mean=84.37ms min=81.18ms max=86.33ms sd=1.36ms heap=80098kb total=1771.78ms avg/cycle=590.59ms total mem=1363125kb avg mem/cycle=454375kb +effect triggers : med=7.30ms mean=7.48ms min=6.70ms max=9.69ms sd=0.74ms heap=10331kb total=157.02ms avg/cycle=52.34ms total mem=206588kb avg mem/cycle=68863kb +array push/pop (hookless) : med=23.45ms mean=23.69ms min=22.57ms max=26.53ms sd=0.94ms heap=48191kb total=497.53ms avg/cycle=165.84ms total mem=930687kb avg mem/cycle=310229kb +array push/pop (hooks) : med=25.37ms mean=25.57ms min=24.44ms max=27.71ms sd=0.79ms heap=47870kb total=536.95ms avg/cycle=178.98ms total mem=928129kb avg mem/cycle=309376kb +many dependencies : med=1.90ms mean=2.02ms min=1.39ms max=3.18ms sd=0.40ms heap=5950kb total=42.32ms avg/cycle=14.11ms total mem=135498kb avg mem/cycle=45166kb +derive chain depth 10 : med=2.28ms mean=2.65ms min=1.56ms max=4.56ms sd=1.12ms heap=3745kb total=55.58ms avg/cycle=18.53ms total mem=80316kb avg mem/cycle=26772kb +100 states individual : med=25.25ms mean=25.42ms min=24.59ms max=26.83ms sd=0.55ms heap=5906kb total=533.89ms avg/cycle=177.96ms total mem=130963kb avg mem/cycle=43654kb +100 states batched : med=1.80ms mean=1.87ms min=1.29ms max=2.82ms sd=0.36ms heap=5920kb total=39.34ms avg/cycle=13.11ms total mem=136456kb avg mem/cycle=45485kb +100 subs disjoint props : med=2.13ms mean=2.07ms min=1.40ms max=3.38ms sd=0.66ms heap=2561kb total=43.46ms avg/cycle=14.49ms total mem=46082kb avg mem/cycle=15361kb + +--- Non-extensible targets (subscriber set lives in the WeakMap fallback) --- + +sealed write 1 sub : med=14.25ms mean=14.39ms min=13.27ms max=16.10ms sd=0.85ms heap=15833kb total=302.29ms avg/cycle=100.76ms total mem=316251kb avg mem/cycle=105417kb +state no subs after seal : med=40.71ms mean=40.74ms min=39.21ms max=42.47ms sd=0.82ms heap=22kb total=855.60ms avg/cycle=285.20ms total mem=383kb avg mem/cycle=128kb + +--- Blind spots (creation shapes, nested read, equal and undefined writes) --- + +state creation (array) : med=19.18ms mean=19.06ms min=18.44ms max=19.62ms sd=0.35ms heap=3239kb total=400.32ms avg/cycle=133.44ms total mem=297308kb avg mem/cycle=99103kb +state creation (sealed) : med=12.36ms mean=12.65ms min=10.50ms max=16.70ms sd=1.62ms heap=6301kb total=265.58ms avg/cycle=88.53ms total mem=132332kb avg mem/cycle=44111kb +state creation (hooks) : med=21.49ms mean=21.56ms min=21.03ms max=22.30ms sd=0.31ms heap=9572kb total=452.68ms avg/cycle=150.89ms total mem=201035kb avg mem/cycle=67012kb +state read nested : med=35.95ms mean=35.97ms min=34.78ms max=37.71ms sd=0.73ms heap=8kb total=755.36ms avg/cycle=251.79ms total mem=173kb avg mem/cycle=58kb +state write equal value : med=18.20ms mean=17.70ms min=15.98ms max=18.68ms sd=0.98ms heap=7kb total=371.65ms avg/cycle=123.88ms total mem=145kb avg mem/cycle=48kb +array push/pop (undefined): med=23.68ms mean=23.97ms min=23.13ms max=26.43ms sd=0.91ms heap=48368kb total=503.47ms avg/cycle=167.82ms total mem=1063307kb avg mem/cycle=354436kb + +Total: 20.6s Avg/cycle: 6.9s Total mem: 8689948kb Avg mem/cycle: 2896649kb +``` + +The ticket's checkboxes are ticked only after review and landing on the change branch, per the apply contract. diff --git a/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/tickets/02-drop-the-slot-write-and-the-frozen-hooks-cache.md b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/tickets/02-drop-the-slot-write-and-the-frozen-hooks-cache.md new file mode 100644 index 0000000..d39b25c --- /dev/null +++ b/openspec/changes/archive/2026-10-25-drop-unread-hooks-slot-on-objects/tickets/02-drop-the-slot-write-and-the-frozen-hooks-cache.md @@ -0,0 +1,246 @@ +# 02: Drop the unread HOOKS slot on objects and the frozen hooks cache + +**Area:** state + +**Blocked by:** 01 + +**What to build:** Creating state with hooks on a non-array target no longer defines the internal hooks symbol on it, and a hooked non-extensible target is no longer written into a cache nobody reads. Only an array target keeps the slot, holding the direct-array context its `push` and `pop` wrappers read; the slot type says so. Hooks keep firing through the handler closures on every kind of target, direct `push` and `pop` on hooked arrays behave as before, and a non-extensible target still gets its Proxy registered in the fallback cache. Every document that described the removed storage says what the code now does, and the hooks roadmap closes item C1 as shipped. The final commit is measured against ticket 01's baseline. + +- [x] A new test asserts that the raw target of a hooked plain object owns exactly one own symbol, the raw target of a hooked array owns two, and the raw target of a hookless object owns one; the object case fails before the change and passes after +- [x] The source has no WeakMap that is written and never read: the frozen hooks cache and its writer are gone, the slot type admits only the direct-array context or nothing, and the non-extensible and define-threw paths still make `state()` register the Proxy in the fallback Proxy cache +- [x] The existing tests for hooks on a frozen object, for direct `push` and `pop` on hooked arrays, and the frozen-and-sealed property tests pass unchanged, and the full suite meets the configured coverage thresholds +- [x] The source comment on the direct-array context class and the header comment of the frozen-and-sealed property test no longer say that a hooks object shares the slot or that the cache is written but never read +- [x] The internals guide in the source folder drops the row for the deleted WeakMap and the row for its writer, and its rows for the slot builder, the symbol definer, the Proxy-properties function and the direct wrappers describe the slot as array-only +- [x] The hooks roadmap moves item C1 to `shipped`, with the evidence column pointing at the test from the first criterion +- [x] `npm run check` and `npm run build` pass on the final commit; the commit message follows the project's format with type `perf` and scope `state` +- [x] The ticket report contains the full benchmark output from the final commit on the same machine and Node version as ticket 01, and no case's median is more than 1% above its ticket 01 median; the hooked creation case is at or below its baseline + +## Report + +Commit `198e195`, `perf(state): drop the hooks slot on objects and the frozen hooks cache`, made by the ticket's implementer in worktree `.claude/worktrees/agent-a185ac3f1c4a26607` on branch `worktree-agent-a185ac3f1c4a26607`, started at BASE `3be0a77`. The commit was first made as `73c6e7d` and amended for message length after review round 1; the tree is identical (`git diff --stat 73c6e7d 198e195` is empty). `git diff --stat 3be0a77 198e195`: + +``` + HOOKS_TODO.md | 2 +- + src/AGENTS.md | 12 +++++------- + src/hooks/AGENTS.md | 3 +-- + src/index.ts | 21 +++++++-------------- + tests/AGENTS.md | 2 +- + tests/array-push-pop.test.ts | 2 +- + tests/property-frozen-sealed.test.ts | 4 ++-- + tests/state-hooks.test.ts | 15 +++++++++++++++ + 8 files changed, 33 insertions(+), 28 deletions(-) +``` + +Criterion 1 red-then-green, as the implementer reported: the new test `defines the hooks symbol on an array target only` in `tests/state-hooks.test.ts` failed against the unchanged source with `AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: 2 !== 1` on the hooked object case, and passed after the source change. Review round 1 reproduced the failure against `3be0a77` in a scratch tree. + +Checks on the final tree, as the implementer reported and `do-transcript-facts` corroborated (each ran after the last file change, status ok, no sandbox denials): `npm run check` exit 0 ("Checked 64 files, No fixes applied, Found 2 infos", the infos are pre-existing biome.json schema notices); `npm run build` exit 0; `npm test` exit 0, 308 passing; `npm run test:coverage` exit 0, `all files | 100.00 | 100.00 | 100.00`. A grep for `frozenHooksCache` and `storeHooksForFrozenTarget` outside `openspec/changes/archive`, `.do/` and this change's folder found no hits. The four prose sites added under Q82 (a) are in the diff. + +Benchmark, criterion 8. Node v24.21.0, Apple M4 Pro (arm64), the same machine as ticket 01. Three runs of the final tree exist: two by the implementer back to back on `73c6e7d`, and one by the steering session on `198e195` immediately after a fresh run of the baseline commit `3be0a77` in the main checkout. Medians in ms: + +| Case | 01 baseline | Run 1 | Run 2 | Fresh baseline | Run 3 | Run 3 vs fresh | +|------|------------|-------|-------|----------------|-------|----------------| +| classic loop | 1.78 | 1.70 | 1.81 | 1.72 | 1.72 | 0.0% | +| state no subs | 39.65 | 38.24 | 39.68 | 40.10 | 38.06 | -5.1% | +| state + derive | 170.56 | 169.85 | 169.05 | 169.87 | 169.58 | -0.2% | +| state + derive + 2 effects | 315.15 | 322.93 | 315.57 | 317.14 | 321.25 | +1.3% | +| batch + derive | 42.98 | 42.71 | 42.55 | 43.16 | 42.74 | -1.0% | +| batch + derive + 2 effects | 42.50 | 43.08 | 43.16 | 42.67 | 42.87 | +0.5% | +| state creation | 9.80 | 9.99 | 9.82 | 9.83 | 9.91 | +0.8% | +| state read (no effect) | 16.64 | 16.60 | 16.42 | 16.74 | 16.66 | -0.5% | +| state write 1 sub | 12.70 | 12.82 | 12.84 | 12.70 | 12.75 | +0.4% | +| state write 100 subs | 84.60 | 83.60 | 85.03 | 85.01 | 85.11 | +0.1% | +| effect triggers | 7.30 | 7.47 | 7.22 | 7.23 | 7.33 | +1.4% | +| array push/pop (hookless) | 23.45 | 23.29 | 23.43 | 23.50 | 23.65 | +0.6% | +| array push/pop (hooks) | 25.37 | 25.44 | 25.42 | 25.02 | 25.08 | +0.2% | +| many dependencies | 1.90 | 1.91 | 1.92 | 1.81 | 1.91 | +5.5% | +| derive chain depth 10 | 2.28 | 2.24 | 2.27 | 2.41 | 2.58 | +7.1% | +| 100 states individual | 25.25 | 25.32 | 25.56 | 25.15 | 25.64 | +1.9% | +| 100 states batched | 1.80 | 1.74 | 1.73 | 1.65 | 1.72 | +4.2% | +| 100 subs disjoint props | 2.13 | 2.19 | 2.02 | 2.15 | 2.08 | -3.3% | +| sealed write 1 sub | 14.25 | 14.49 | 14.71 | 14.28 | 14.34 | +0.4% | +| state no subs after seal | 40.71 | 39.54 | 40.40 | 40.79 | 39.54 | -3.1% | +| state creation (array) | 19.18 | 18.89 | 18.98 | 19.15 | 19.16 | +0.1% | +| state creation (sealed) | 12.36 | 11.51 | 12.18 | 12.09 | 12.06 | -0.2% | +| state creation (hooks) | 21.49 | 13.24 | 13.24 | 21.40 | 13.32 | -37.8% | +| state read nested | 35.95 | 35.98 | 35.33 | 35.45 | 35.50 | +0.1% | +| state write equal value | 18.20 | 16.58 | 18.07 | 18.40 | 16.81 | -8.6% | +| array push/pop (undefined) | 23.68 | 23.58 | 23.65 | 23.56 | 23.52 | -0.2% | +| Total | 20.6s | 20.7s | 20.5s | 20.7s | 20.5s | | + +The hooked creation case is 38% below its baseline in every run. No case is more than 1% above its baseline in all three runs; every case that is over 1% in one run is within 1% or below baseline in another, and each such case has a standard deviation in the output larger than its delta. The gate ruling is recorded in the ledger and repeated at the end of this report. + +Run 1 by the implementer on `73c6e7d`, complete stdout: + +``` +> @nerdalytics/beacon@2002.0.0 benchmark +> node --expose-gc scripts/benchmark.ts -R 3 + +=== Beacon Benchmark (1,000,000 iterations, 3 cycles × 7 samples = 21 total) === + +Cycle 1/3... +Cycle 2/3... +Cycle 3/3... +classic loop : med=1.70ms mean=1.71ms min=1.51ms max=1.87ms sd=0.11ms heap=11kb total=35.83ms avg/cycle=11.94ms total mem=238kb avg mem/cycle=79kb +state no subs : med=38.24ms mean=35.26ms min=25.68ms max=41.80ms sd=6.47ms heap=10kb total=740.49ms avg/cycle=246.83ms total mem=288kb avg mem/cycle=96kb +state + derive : med=169.85ms mean=163.40ms min=144.65ms max=175.67ms sd=12.19ms heap=68856kb total=3431.44ms avg/cycle=1143.81ms total mem=1056093kb avg mem/cycle=352031kb +state + derive + 2 effects: med=322.93ms mean=314.40ms min=280.85ms max=337.34ms sd=19.70ms heap=63836kb total=6602.34ms avg/cycle=2200.78ms total mem=1018316kb avg mem/cycle=339439kb +batch + derive : med=42.71ms mean=39.90ms min=31.72ms max=45.32ms sd=4.89ms heap=19kb total=837.88ms avg/cycle=279.29ms total mem=508kb avg mem/cycle=169kb +batch + derive + 2 effects: med=43.08ms mean=39.97ms min=30.77ms max=45.30ms sd=5.25ms heap=24kb total=839.44ms avg/cycle=279.81ms total mem=613kb avg mem/cycle=204kb + +--- Targeted benchmarks --- + +state creation : med=9.99ms mean=10.03ms min=9.76ms max=10.46ms sd=0.20ms heap=15681kb total=210.72ms avg/cycle=70.24ms total mem=329307kb avg mem/cycle=109769kb +state read (no effect) : med=16.60ms mean=16.75ms min=15.86ms max=19.16ms sd=0.66ms heap=7kb total=351.79ms avg/cycle=117.26ms total mem=145kb avg mem/cycle=48kb +state write 1 sub : med=12.82ms mean=13.48ms min=11.83ms max=16.46ms sd=1.32ms heap=16003kb total=283.06ms avg/cycle=94.35ms total mem=327291kb avg mem/cycle=109097kb +state write 100 subs : med=83.60ms mean=83.83ms min=80.23ms max=87.07ms sd=1.58ms heap=80083kb total=1760.44ms avg/cycle=586.81ms total mem=1362940kb avg mem/cycle=454313kb +effect triggers : med=7.47ms mean=7.66ms min=6.48ms max=10.35ms sd=0.94ms heap=10113kb total=160.81ms avg/cycle=53.60ms total mem=203997kb avg mem/cycle=67999kb +array push/pop (hookless) : med=23.29ms mean=23.42ms min=22.42ms max=25.17ms sd=0.65ms heap=48105kb total=491.91ms avg/cycle=163.97ms total mem=927270kb avg mem/cycle=309090kb +array push/pop (hooks) : med=25.44ms mean=25.69ms min=24.34ms max=27.80ms sd=0.96ms heap=47842kb total=539.41ms avg/cycle=179.80ms total mem=928841kb avg mem/cycle=309614kb +many dependencies : med=1.91ms mean=1.95ms min=1.41ms max=2.77ms sd=0.36ms heap=5950kb total=40.95ms avg/cycle=13.65ms total mem=134065kb avg mem/cycle=44688kb +derive chain depth 10 : med=2.24ms mean=2.51ms min=1.61ms max=4.28ms sd=0.96ms heap=3744kb total=52.65ms avg/cycle=17.55ms total mem=79555kb avg mem/cycle=26518kb +100 states individual : med=25.32ms mean=25.38ms min=24.25ms max=26.72ms sd=0.51ms heap=6761kb total=532.89ms avg/cycle=177.63ms total mem=137619kb avg mem/cycle=45873kb +100 states batched : med=1.74ms mean=1.84ms min=1.31ms max=2.79ms sd=0.34ms heap=5912kb total=38.55ms avg/cycle=12.85ms total mem=136665kb avg mem/cycle=45555kb +100 subs disjoint props : med=2.19ms mean=2.07ms min=1.39ms max=3.37ms sd=0.65ms heap=2561kb total=43.45ms avg/cycle=14.48ms total mem=45941kb avg mem/cycle=15314kb + +--- Non-extensible targets (subscriber set lives in the WeakMap fallback) --- + +sealed write 1 sub : med=14.49ms mean=14.61ms min=13.73ms max=16.11ms sd=0.72ms heap=15991kb total=306.82ms avg/cycle=102.27ms total mem=332243kb avg mem/cycle=110748kb +state no subs after seal : med=39.54ms mean=39.92ms min=36.72ms max=43.09ms sd=1.87ms heap=8kb total=838.33ms avg/cycle=279.44ms total mem=400kb avg mem/cycle=133kb + +--- Blind spots (creation shapes, nested read, equal and undefined writes) --- + +state creation (array) : med=18.89ms mean=18.88ms min=18.37ms max=19.34ms sd=0.28ms heap=3229kb total=396.48ms avg/cycle=132.16ms total mem=67852kb avg mem/cycle=22617kb +state creation (sealed) : med=11.51ms mean=11.32ms min=8.36ms max=15.00ms sd=1.59ms heap=6303kb total=237.71ms avg/cycle=79.24ms total mem=132339kb avg mem/cycle=44113kb +state creation (hooks) : med=13.24ms mean=13.28ms min=12.76ms max=13.87ms sd=0.28ms heap=4074kb total=278.90ms avg/cycle=92.97ms total mem=85586kb avg mem/cycle=28529kb +state read nested : med=35.98ms mean=35.92ms min=34.89ms max=36.89ms sd=0.57ms heap=8kb total=754.23ms avg/cycle=251.41ms total mem=173kb avg mem/cycle=58kb +state write equal value : med=16.58ms mean=17.03ms min=16.01ms max=18.70ms sd=0.84ms heap=7kb total=357.71ms avg/cycle=119.24ms total mem=145kb avg mem/cycle=48kb +array push/pop (undefined): med=23.58ms mean=23.90ms min=23.02ms max=26.74ms sd=0.97ms heap=48438kb total=501.92ms avg/cycle=167.31ms total mem=1062674kb avg mem/cycle=354225kb + +Total: 20.7s Avg/cycle: 6.9s Total mem: 8371105kb Avg mem/cycle: 2790368kb +``` + +Run 2 by the implementer on `73c6e7d`, back to back with run 1, complete stdout: + +``` +> @nerdalytics/beacon@2002.0.0 benchmark +> node --expose-gc scripts/benchmark.ts -R 3 + +=== Beacon Benchmark (1,000,000 iterations, 3 cycles × 7 samples = 21 total) === + +Cycle 1/3... +Cycle 2/3... +Cycle 3/3... +classic loop : med=1.81ms mean=1.79ms min=1.56ms max=2.08ms sd=0.13ms heap=11kb total=37.50ms avg/cycle=12.50ms total mem=238kb avg mem/cycle=79kb +state no subs : med=39.68ms mean=36.15ms min=26.08ms max=43.24ms sd=6.84ms heap=10kb total=759.21ms avg/cycle=253.07ms total mem=267kb avg mem/cycle=89kb +state + derive : med=169.05ms mean=160.96ms min=130.72ms max=175.94ms sd=14.60ms heap=68835kb total=3380.14ms avg/cycle=1126.71ms total mem=1041890kb avg mem/cycle=347297kb +state + derive + 2 effects: med=315.57ms mean=304.53ms min=266.26ms max=325.04ms sd=20.52ms heap=63844kb total=6395.23ms avg/cycle=2131.74ms total mem=1027264kb avg mem/cycle=342421kb +batch + derive : med=42.55ms mean=40.13ms min=31.90ms max=45.50ms sd=4.92ms heap=19kb total=842.72ms avg/cycle=280.91ms total mem=553kb avg mem/cycle=184kb +batch + derive + 2 effects: med=43.16ms mean=39.84ms min=31.64ms max=44.50ms sd=5.13ms heap=24kb total=836.65ms avg/cycle=278.88ms total mem=613kb avg mem/cycle=204kb + +--- Targeted benchmarks --- + +state creation : med=9.82ms mean=9.91ms min=9.35ms max=10.85ms sd=0.33ms heap=15680kb total=208.18ms avg/cycle=69.39ms total mem=329285kb avg mem/cycle=109762kb +state read (no effect) : med=16.42ms mean=16.44ms min=15.97ms max=16.92ms sd=0.25ms heap=7kb total=345.27ms avg/cycle=115.09ms total mem=145kb avg mem/cycle=48kb +state write 1 sub : med=12.84ms mean=13.05ms min=11.81ms max=15.58ms sd=0.96ms heap=16159kb total=273.99ms avg/cycle=91.33ms total mem=329045kb avg mem/cycle=109682kb +state write 100 subs : med=85.03ms mean=85.00ms min=82.31ms max=88.31ms sd=1.31ms heap=80057kb total=1784.94ms avg/cycle=594.98ms total mem=1362484kb avg mem/cycle=454161kb +effect triggers : med=7.22ms mean=7.39ms min=6.51ms max=9.13ms sd=0.63ms heap=10272kb total=155.14ms avg/cycle=51.71ms total mem=205448kb avg mem/cycle=68483kb +array push/pop (hookless) : med=23.43ms mean=23.59ms min=22.53ms max=25.38ms sd=0.73ms heap=48306kb total=495.49ms avg/cycle=165.16ms total mem=928706kb avg mem/cycle=309569kb +array push/pop (hooks) : med=25.42ms mean=25.47ms min=24.32ms max=28.29ms sd=0.90ms heap=47880kb total=534.97ms avg/cycle=178.32ms total mem=929842kb avg mem/cycle=309947kb +many dependencies : med=1.92ms mean=1.92ms min=1.40ms max=2.90ms sd=0.37ms heap=5947kb total=40.39ms avg/cycle=13.46ms total mem=132894kb avg mem/cycle=44298kb +derive chain depth 10 : med=2.27ms mean=2.54ms min=1.66ms max=4.36ms sd=0.93ms heap=3749kb total=53.33ms avg/cycle=17.78ms total mem=82784kb avg mem/cycle=27595kb +100 states individual : med=25.56ms mean=25.70ms min=24.53ms max=27.25ms sd=0.69ms heap=6872kb total=539.64ms avg/cycle=179.88ms total mem=140891kb avg mem/cycle=46964kb +100 states batched : med=1.73ms mean=1.78ms min=1.29ms max=2.66ms sd=0.36ms heap=5902kb total=37.30ms avg/cycle=12.43ms total mem=132818kb avg mem/cycle=44273kb +100 subs disjoint props : med=2.02ms mean=2.07ms min=1.36ms max=3.71ms sd=0.71ms heap=1941kb total=43.49ms avg/cycle=14.50ms total mem=44973kb avg mem/cycle=14991kb + +--- Non-extensible targets (subscriber set lives in the WeakMap fallback) --- + +sealed write 1 sub : med=14.71ms mean=14.85ms min=13.80ms max=16.95ms sd=0.77ms heap=16002kb total=311.76ms avg/cycle=103.92ms total mem=326668kb avg mem/cycle=108889kb +state no subs after seal : med=40.40ms mean=40.71ms min=38.72ms max=44.49ms sd=1.31ms heap=22kb total=854.99ms avg/cycle=285.00ms total mem=383kb avg mem/cycle=128kb + +--- Blind spots (creation shapes, nested read, equal and undefined writes) --- + +state creation (array) : med=18.98ms mean=18.91ms min=18.40ms max=20.08ms sd=0.37ms heap=3234kb total=397.19ms avg/cycle=132.40ms total mem=297203kb avg mem/cycle=99068kb +state creation (sealed) : med=12.18ms mean=12.17ms min=9.11ms max=14.16ms sd=1.27ms heap=6302kb total=255.55ms avg/cycle=85.18ms total mem=132331kb avg mem/cycle=44110kb +state creation (hooks) : med=13.24ms mean=13.29ms min=12.90ms max=13.97ms sd=0.25ms heap=4073kb total=279.00ms avg/cycle=93.00ms total mem=85540kb avg mem/cycle=28513kb +state read nested : med=35.33ms mean=35.39ms min=34.34ms max=36.21ms sd=0.43ms heap=8kb total=743.29ms avg/cycle=247.76ms total mem=173kb avg mem/cycle=58kb +state write equal value : med=18.07ms mean=17.72ms min=15.96ms max=19.48ms sd=1.05ms heap=7kb total=372.15ms avg/cycle=124.05ms total mem=145kb avg mem/cycle=48kb +array push/pop (undefined): med=23.65ms mean=23.89ms min=23.05ms max=27.09ms sd=0.97ms heap=48463kb total=501.76ms avg/cycle=167.25ms total mem=1062532kb avg mem/cycle=354177kb + +Total: 20.5s Avg/cycle: 6.8s Total mem: 8595114kb Avg mem/cycle: 2865038kb +``` + +Run 3 by the steering session on `198e195` (the same tree), case and total lines, immediately after a fresh run of `3be0a77` in the main checkout whose case lines are the "Fresh baseline" column above: + +``` +classic loop : med=1.72ms mean=1.72ms min=1.48ms max=1.96ms sd=0.12ms heap=11kb total=36.12ms avg/cycle=12.04ms total mem=238kb avg mem/cycle=79kb +state no subs : med=38.06ms mean=34.76ms min=25.61ms max=40.12ms sd=6.01ms heap=10kb total=729.99ms avg/cycle=243.33ms total mem=310kb avg mem/cycle=103kb +state + derive : med=169.58ms mean=161.52ms min=139.02ms max=176.05ms sd=13.89ms heap=68526kb total=3391.83ms avg/cycle=1130.61ms total mem=1021223kb avg mem/cycle=340408kb +state + derive + 2 effects: med=321.25ms mean=308.40ms min=267.57ms max=339.92ms sd=24.49ms heap=62665kb total=6476.50ms avg/cycle=2158.83ms total mem=1044630kb avg mem/cycle=348210kb +batch + derive : med=42.74ms mean=40.07ms min=31.68ms max=44.58ms sd=4.74ms heap=19kb total=841.40ms avg/cycle=280.47ms total mem=510kb avg mem/cycle=170kb +batch + derive + 2 effects: med=42.87ms mean=39.77ms min=31.06ms max=45.23ms sd=5.23ms heap=24kb total=835.27ms avg/cycle=278.42ms total mem=613kb avg mem/cycle=204kb +state creation : med=9.91ms mean=9.90ms min=9.52ms max=10.24ms sd=0.18ms heap=15681kb total=207.92ms avg/cycle=69.31ms total mem=329303kb avg mem/cycle=109768kb +state read (no effect) : med=16.66ms mean=16.69ms min=16.02ms max=18.24ms sd=0.50ms heap=7kb total=350.58ms avg/cycle=116.86ms total mem=146kb avg mem/cycle=49kb +state write 1 sub : med=12.75ms mean=12.98ms min=11.58ms max=14.98ms sd=0.86ms heap=16193kb total=272.56ms avg/cycle=90.85ms total mem=329365kb avg mem/cycle=109788kb +state write 100 subs : med=85.11ms mean=84.66ms min=80.45ms max=87.66ms sd=1.67ms heap=80078kb total=1777.77ms avg/cycle=592.59ms total mem=1359488kb avg mem/cycle=453163kb +effect triggers : med=7.33ms mean=7.49ms min=6.49ms max=9.43ms sd=0.79ms heap=10329kb total=157.21ms avg/cycle=52.40ms total mem=207261kb avg mem/cycle=69087kb +array push/pop (hookless) : med=23.65ms mean=23.95ms min=23.12ms max=26.72ms sd=0.80ms heap=48278kb total=502.96ms avg/cycle=167.65ms total mem=929122kb avg mem/cycle=309707kb +array push/pop (hooks) : med=25.08ms mean=25.33ms min=24.40ms max=27.66ms sd=0.80ms heap=47877kb total=531.90ms avg/cycle=177.30ms total mem=929396kb avg mem/cycle=309799kb +many dependencies : med=1.91ms mean=1.94ms min=1.39ms max=2.87ms sd=0.40ms heap=5947kb total=40.79ms avg/cycle=13.60ms total mem=132885kb avg mem/cycle=44295kb +derive chain depth 10 : med=2.58ms mean=2.73ms min=1.56ms max=4.58ms sd=1.00ms heap=3749kb total=57.27ms avg/cycle=19.09ms total mem=85756kb avg mem/cycle=28585kb +100 states individual : med=25.64ms mean=25.62ms min=24.44ms max=28.28ms sd=0.81ms heap=6699kb total=538.12ms avg/cycle=179.37ms total mem=137377kb avg mem/cycle=45792kb +100 states batched : med=1.72ms mean=1.84ms min=1.31ms max=2.79ms sd=0.32ms heap=5900kb total=38.58ms avg/cycle=12.86ms total mem=136990kb avg mem/cycle=45663kb +100 subs disjoint props : med=2.08ms mean=2.10ms min=1.43ms max=3.68ms sd=0.71ms heap=2563kb total=44.08ms avg/cycle=14.69ms total mem=46034kb avg mem/cycle=15345kb +sealed write 1 sub : med=14.34ms mean=14.45ms min=13.56ms max=16.45ms sd=0.74ms heap=16022kb total=303.43ms avg/cycle=101.14ms total mem=329829kb avg mem/cycle=109943kb +state no subs after seal : med=39.54ms mean=39.59ms min=37.13ms max=42.01ms sd=1.51ms heap=8kb total=831.42ms avg/cycle=277.14ms total mem=400kb avg mem/cycle=133kb +state creation (array) : med=19.16ms mean=19.12ms min=18.53ms max=19.78ms sd=0.31ms heap=3234kb total=401.56ms avg/cycle=133.85ms total mem=297210kb avg mem/cycle=99070kb +state creation (sealed) : med=12.06ms mean=11.93ms min=8.63ms max=14.14ms sd=1.54ms heap=6302kb total=250.52ms avg/cycle=83.51ms total mem=132355kb avg mem/cycle=44118kb +state creation (hooks) : med=13.32ms mean=13.36ms min=12.90ms max=13.75ms sd=0.23ms heap=4073kb total=280.63ms avg/cycle=93.54ms total mem=85549kb avg mem/cycle=28516kb +state read nested : med=35.50ms mean=35.41ms min=33.60ms max=37.41ms sd=0.88ms heap=8kb total=743.52ms avg/cycle=247.84ms total mem=173kb avg mem/cycle=58kb +state write equal value : med=16.81ms mean=17.37ms min=16.15ms max=19.87ms sd=1.07ms heap=7kb total=364.81ms avg/cycle=121.60ms total mem=145kb avg mem/cycle=48kb +array push/pop (undefined): med=23.52ms mean=23.86ms min=22.69ms max=26.40ms sd=0.95ms heap=48441kb total=501.09ms avg/cycle=167.03ms total mem=1063046kb avg mem/cycle=354349kb +Total: 20.5s Avg/cycle: 6.8s Total mem: 8599354kb Avg mem/cycle: 2866451kb +``` + +### Gate ruling (Q84) + +The maintainer ruled on Q84 that each case has a 1% ceiling on slowdowns and no limit on speedups. The 3-cycle runs above cannot resolve 1% on the small cases (identical code differs by more than 1% between runs on them), so the gate was re-measured at 10 cycles with `node --expose-gc scripts/benchmark.ts -R 10`, in the order baseline (A1), final (B1), final (B2), baseline (A2), medians in ms, final against the baseline run nearest in time: + +``` +case A1 B1 B2 A2 B1/A1 B2/A2 +classic loop 1.66 1.66 1.71 1.68 +0.0% +1.8% +state no subs 40.47 41.12 41.39 41.23 +1.6% +0.4% +state + derive 174.79 175.22 176.77 175.50 +0.2% +0.7% +state + derive + 2 effects 325.70 330.09 327.29 324.74 +1.3% +0.8% +batch + derive 43.48 43.15 43.18 43.43 -0.8% -0.6% +batch + derive + 2 effects 43.08 43.20 43.20 43.26 +0.3% -0.1% +state creation 9.82 9.96 10.02 9.91 +1.4% +1.1% +state read (no effect) 16.70 16.68 16.73 16.62 -0.1% +0.7% +state write 1 sub 13.02 12.98 13.26 13.17 -0.3% +0.7% +state write 100 subs 85.61 86.14 85.83 85.28 +0.6% +0.6% +effect triggers 7.41 7.42 7.66 7.52 +0.1% +1.9% +array push/pop (hookless) 23.46 23.46 23.68 23.66 +0.0% +0.1% +array push/pop (hooks) 25.09 25.60 25.62 25.35 +2.0% +1.1% +many dependencies 1.80 1.80 1.80 1.88 +0.0% -4.3% +derive chain depth 10 2.40 2.47 1.93 2.36 +2.9% -18.2% +100 states individual 25.45 25.51 25.50 25.55 +0.2% -0.2% +100 states batched 1.71 1.70 1.67 1.70 -0.6% -1.8% +100 subs disjoint props 2.25 2.25 2.12 2.19 +0.0% -3.2% +sealed write 1 sub 14.44 14.56 14.84 14.54 +0.8% +2.1% +state no subs after seal 40.67 41.60 41.62 41.19 +2.3% +1.0% +state creation (array) 18.86 18.78 18.92 19.05 -0.4% -0.7% +state creation (sealed) 18.41 17.35 17.41 17.88 -5.8% -2.6% +state creation (hooks) 21.53 13.37 13.39 21.69 -37.9% -38.3% +state read nested 35.88 35.69 35.77 35.69 -0.5% +0.2% +state write equal value 18.20 18.40 18.39 18.55 +1.1% -0.9% +array push/pop (undefined) 23.42 23.86 23.82 23.60 +1.9% +0.9% +Total 71.8s 71.7s 71.7s 72.2s +``` + +Two cases were over 1% in both pairings, `state creation` and `array push/pop (hooks)`, and neither path is touched by the diff. A targeted probe with the same loop bodies, 8 alternating processes per commit, 15 samples each with GC between samples, median of per-process medians: + +| Case | Baseline `3be0a77` | Final `198e195` | Change | +|------|--------------------|-----------------|--------| +| state creation | 10.134ms | 10.075ms | -0.59% | +| array push/pop (hooks) | 24.326ms | 24.236ms | -0.37% | + +The baseline's own eight processes spread from 10.00ms to 10.26ms on creation (2.6%), wider than any difference between the commits. Under the Q84 rule no case shows a slowdown above 1% that survives a measurement able to resolve 1%, and the hooked creation case is 38% faster in every run. Gate met; ruling logged in `.do/ledger/drop-unread-hooks-slot-on-objects.md`. diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/.openspec.yaml b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/.openspec.yaml similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/.openspec.yaml rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/.openspec.yaml diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/design.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/design.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/design.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/design.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/proposal.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/proposal.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/proposal.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/proposal.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/specs/array-mutation/spec.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/specs/array-mutation/spec.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/specs/array-mutation/spec.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/specs/array-mutation/spec.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/tickets/01-benchmark-case-and-baseline.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/01-benchmark-case-and-baseline.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/tickets/01-benchmark-case-and-baseline.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/01-benchmark-case-and-baseline.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/tickets/02-one-set-trap-for-all-state.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/02-one-set-trap-for-all-state.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/tickets/02-one-set-trap-for-all-state.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/02-one-set-trap-for-all-state.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/tickets/03-wrap-mutating-methods-through-the-proxy.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/03-wrap-mutating-methods-through-the-proxy.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/tickets/03-wrap-mutating-methods-through-the-proxy.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/03-wrap-mutating-methods-through-the-proxy.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/tickets/04-loop-detection-and-hook-observation.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/04-loop-detection-and-hook-observation.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/tickets/04-loop-detection-and-hook-observation.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/04-loop-detection-and-hook-observation.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/tickets/05-docs-and-benchmark-comparison.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/05-docs-and-benchmark-comparison.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/tickets/05-docs-and-benchmark-comparison.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/05-docs-and-benchmark-comparison.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/tickets/06-performance-gate.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/06-performance-gate.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/tickets/06-performance-gate.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/06-performance-gate.md diff --git a/openspec/changes/wrap-array-methods-on-hookless-state/tickets/07-direct-fast-path-for-push-and-pop.md b/openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/07-direct-fast-path-for-push-and-pop.md similarity index 100% rename from openspec/changes/wrap-array-methods-on-hookless-state/tickets/07-direct-fast-path-for-push-and-pop.md rename to openspec/changes/archive/2026-10-25-wrap-array-methods-on-hookless-state/tickets/07-direct-fast-path-for-push-and-pop.md diff --git a/openspec/changes/wrap-fill-and-copywithin/.openspec.yaml b/openspec/changes/wrap-fill-and-copywithin/.openspec.yaml new file mode 100644 index 0000000..d9520fa --- /dev/null +++ b/openspec/changes/wrap-fill-and-copywithin/.openspec.yaml @@ -0,0 +1,2 @@ +schema: do +created: 2026-10-25 diff --git a/openspec/changes/wrap-fill-and-copywithin/design.md b/openspec/changes/wrap-fill-and-copywithin/design.md new file mode 100644 index 0000000..b6b4629 --- /dev/null +++ b/openspec/changes/wrap-fill-and-copywithin/design.md @@ -0,0 +1,41 @@ +# Design + +## Context + +See proposal.md, Why. Verified on trunk `18f5a71` on 2026-10-25; the reproduction is in the triage notes of `.do/inbox/2026-10-25-fill-and-copywithin-not-batched.md`. What shapes the approach: + +- The get trap asks a lookup for a wrapper whenever an array target hands out a function. The lookup checks the method cache first, keyed by native method, and only on a miss checks the property name against the set of mutating names. A name in the set is wrapped once; the wrapper applies the native method to its call receiver, the Proxy, inside the batch depth bookkeeping, so every read and write passes the traps and the batch boundary delivers at most one wake per subscriber. `push` and `pop` are the only names with a direct path. +- `fill` reads `length` and writes the indices in its range; it reads no element. `copyWithin` reads `length`, reads the source range and writes the target range. Both already go through the traps today because the Proxy is the receiver of the unwrapped call; what they lack is the batch around the call. Hooks therefore already see them, and loop detection already applies its read-then-write rule to them, which is why `fill` alone never throws and `copyWithin` throws only on overlapping ranges. +- The lookup body is a read-path hot spot: the previous change measured 2 to 5% on the read benchmark for any change to it. Adding two names to the set changes nothing in that body; the set is consulted once per native method, after which the cache hit answers. +- The previous change's gate was re-measured under Q84: a 1% per-case ceiling on slowdowns against the baseline run, no limit on speedups, with 3-cycle runs too noisy to resolve 1% on small cases and 10-cycle alternating runs used to settle the two cases over the line. + +## Goals / Non-Goals + +**Goals:** +- `fill` and `copyWithin` behave like the seven wrapped methods under every requirement of the `array-mutation` spec, on both kinds of state. +- A trunk number for `fill` before the library changes, and no existing case slower by more than the accepted noise after it. +- Loop detection unchanged in code; the spec states what the existing rule means for the two methods. + +**Non-Goals:** +- A direct fast path for either method. +- Any change to the inside-effect wake set, to the lookup body, to the cache shape or to `applyInBatch`. +- The handbook, the version, and writes that bypass the traps. + +## Decisions + +- **Two names in the existing set, nothing else in the library.** The cached Proxy-receiver wrapper is exactly what the spec asks for and already serves five methods. Alternative: a direct path like push and pop, reproducing the trap sequence by hand; rejected because the previous change limited the direct path to the two methods whose read and write pattern is fixed, and `fill` and `copyWithin` take ranges, so their pattern depends on the arguments, which is where correctness would be guessed. Alternative: a per-call wake without the batch; rejected because the batch is the one mechanism and composes with user batches. +- **Loop detection keeps its rule; the spec gains a sentence and scenarios, not an exemption.** The rule is "a write to a property the effect read raises"; `fill` and `copyWithin` write properties their own call did not necessarily read, so the outcome depends on what else the effect read or on range overlap. Stating that in the spec keeps the unconditional scenario honest for the seven and pins the conditional behavior for the two. Alternative: force the loop error on any `fill` or `copyWithin` inside a synchronous top-level effect; rejected because it needs a new check in the trap for no observed defect and the spec forbids a new exemption or counter. +- **The main spec's Purpose line is edited by the ticket that lands the library change, not at planning time.** Archive copies delta requirements into the main spec but ignores a delta's Purpose, and the Purpose names the seven methods. Editing it now would make the main spec claim nine before the code does; editing it in the same commit as the set keeps spec and code in step. The agent docs follow the same rule. +- **Benchmark case: one subscribed effect, a loop of `fill` over a short array, hookless state, shaped like the push/pop case, committed and measured on the unchanged library first (user ruling at triage, Q66 d order).** `fill` is the cheaper of the two to time and its every call changes every element, so the case measures the batch wrapper against the unbatched trap writes it replaces; a `copyWithin` case would measure the same wrapper. One kind of state, because the change does not differ between kinds and the wrapper has no hook-dependent branch. Alternative: both kinds, as the push/pop case does; not needed here and can be added if a number is ever wanted. +- **Gate: the per-case rule of Q84, measured against ticket 01's run.** No existing case may be slower by more than 1%; the new case is reported before and after, and is expected to get faster since one wake replaces several. If a 3-cycle pairing leaves a case over the line, the gate ticket re-measures that case at 10 cycles in alternating order before judging, as the previous change did. Alternative: grand-total gate of the first array change; superseded by Q84. + +## Risks / Trade-offs + +- [A consumer relied on per-element wakes from `fill` or `copyWithin`] → undocumented; the handbook describes wakes as reactions to a method call. Called out in the notes of the release that carries the behavior, with PR 186's change of the same class. +- [`fill` on a frozen array now raises inside the batch wrapper] → `applyInBatch` aborts the batch and rethrows; the spec scenario pins the native `TypeError` and that the system stays usable, as the sealed-push scenario does. +- [The 3-cycle benchmark run cannot resolve 1% on the small cases] → the gate ticket follows the Q84 procedure for any case over the line; identical code was measured spreading 2.6% on those cases. +- [A loop-detection scenario encodes a native read pattern that a future engine could change] → the scenarios use the ranges the ECMAScript algorithms define (`fill` reads no element; `copyWithin` reads exactly its source range), checked on Node 24.21.0 at planning time. + +## Migration Plan + +No version bump (Q35 c): the wake-count change ships with the next breaking release beside PR 186. Order inside the change: the benchmark case, then the set and the notification tests, then the loop and hook assertions with the spec Purpose and agent docs, then the gate, so every commit on the branch is green. Rollback: revert the squash commit; no data or API shape changes. diff --git a/openspec/changes/wrap-fill-and-copywithin/proposal.md b/openspec/changes/wrap-fill-and-copywithin/proposal.md new file mode 100644 index 0000000..75c7292 --- /dev/null +++ b/openspec/changes/wrap-fill-and-copywithin/proposal.md @@ -0,0 +1,35 @@ +# Proposal + +## Why + +`Array.prototype.fill` and `Array.prototype.copyWithin` mutate an array in place like the seven methods the `array-mutation` spec covers, but they are not in the set of names the get trap wraps, so a reactive array hands them out unwrapped and every element they write wakes subscribers on its own, outside any batch. Reproduced on trunk `18f5a71` on 2026-10-25: an effect reading every index of a reactive `[1, 2, 3]` re-ran 3 times for `fill(9)` and 2 times for `copyWithin(0, 1)`, where `reverse()` woke it once, on both kinds of state. Triaged from `.do/inbox/2026-10-25-fill-and-copywithin-not-batched.md`; the reproduction, the loop-detection correction and the rulings Q85 and Q86 are recorded there. + +## What Changes + +- A benchmark case for `fill` on a reactive array with one subscribed effect, shaped like the existing `array push/pop` cases, committed and run on the unchanged library first so the change has a trunk number (user ruling at triage, the order Q66 d set for the previous change). +- `fill` and `copyWithin` join the set of mutating method names the get trap wraps. Both get the existing cached wrapper that applies the native method to the Proxy receiver inside a batch; no second mechanism, no direct fast path. +- Consumer-visible: a `fill` or `copyWithin` call from outside a batch wakes each subscriber of the array at most once instead of once per written element, and a call that changes no element wakes nobody. That is the same class of wake-count change PR 186 shipped, so it rides the next breaking release with it (Q35 c); no version bump here. +- Loop detection keeps its rule unchanged and the spec says what it means for the two names: `fill` writes indices it does not read, so it raises the loop error in a synchronous top-level effect only when that effect also read an index the call writes; `copyWithin` raises it only when its source and target ranges overlap. +- The `array-mutation` spec and the root `AGENTS.md` name nine methods; `src/AGENTS.md` stops counting "the other five" wrappers. +- Tests cover both names on both kinds of state: wake count, no-change silence, batch composition, the conditional loop behavior, hook observation and native-equal results. + +Out of scope, by decision in triage: a direct fast path for the two methods; the inside-effect wake semantics (`.do/inbox/2026-10-23-per-property-wake-inside-effects.md`, Q86 a); writes that bypass the traps (`.do/inbox/2026-10-25-define-property-and-prototype-writes-bypass-traps.md`); a version bump; the handbook pages, which wait for the release tree. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `array-mutation`: the purpose and every requirement that names the methods cover nine instead of seven; the loop-detection requirement gains a scenario for the two methods whose write set is not a subset of their read set; the "results equal the native method" requirement gains scenarios for `fill` and `copyWithin` argument shapes. + +## Impact + +- `src/index.ts`: two names in `MUTATING_ARRAY_METHODS`; the lookup, the cache and `applyInBatch` are unchanged. One comment that counts the Proxy-receiver wrappers. +- `scripts/benchmark.ts`: one new case. +- `tests/array-methods.test.ts`, `tests/property-array-mutations.test.ts`, `tests/infinite-loop.test.ts`, `tests/state-hooks.test.ts`: new assertions for the two names. +- `openspec/specs/array-mutation/spec.md` via the delta; `AGENTS.md` key behaviors; `src/AGENTS.md` function table. +- `package.json`, `jsr.json`, the handbook: unchanged. +- Consumers: fewer wakes per `fill` or `copyWithin` call; no other observable change, since hooks and the loop check already saw these writes. diff --git a/openspec/changes/wrap-fill-and-copywithin/specs/array-mutation/spec.md b/openspec/changes/wrap-fill-and-copywithin/specs/array-mutation/spec.md new file mode 100644 index 0000000..14e0f15 --- /dev/null +++ b/openspec/changes/wrap-fill-and-copywithin/specs/array-mutation/spec.md @@ -0,0 +1,140 @@ +# Spec Delta + +## MODIFIED Requirements + +### Requirement: Mutating methods behave the same with and without hooks +Reactive state SHALL expose the same behavior for the nine mutating array methods (`push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`, `fill`, `copyWithin`) whether the state was created with hooks or without. Hooks SHALL only observe; they SHALL NOT change which subscribers wake, how often, or whether a loop error is raised. + +#### Scenario: Same wake count on both kinds of state +- **WHEN** an effect reads index 0 and `length` of an empty reactive array, and `push(1)` is called from outside any effect, on state created without hooks and on state created with hooks +- **THEN** the effect re-runs exactly once on each + +#### Scenario: Same wake count for fill on both kinds of state +- **WHEN** an effect reads every index of a reactive array `[1, 2, 3]` and `fill(9)` is called from outside any effect, on state created without hooks and on state created with hooks +- **THEN** the effect re-runs exactly once on each + +#### Scenario: Same loop outcome on both kinds of state +- **WHEN** a synchronous top-level effect calls a mutating method on a reactive array, on state created without hooks and on state created with hooks +- **THEN** both raise the same infinite-loop error + +### Requirement: One wake per method call +A mutating array method called from outside any batch SHALL wake each subscriber of that array at most once per call, after the method has returned, regardless of which properties the subscriber read and how many properties the method changed. A call that changes no element and leaves `length` unchanged SHALL wake no subscriber. + +#### Scenario: Subscriber of an unchanged index wakes once +- **WHEN** an effect reads only index 1 of a reactive array `[0, 1, 2]` and `push(3)` is called from outside any effect +- **THEN** the effect re-runs exactly once + +#### Scenario: Subscriber of a changed index and length wakes once +- **WHEN** an effect reads index 0 and `length` of an empty reactive array and `push(1)` is called from outside any effect +- **THEN** the effect re-runs exactly once, and sees the element and the new length + +#### Scenario: Subscriber of every index wakes once after fill +- **WHEN** an effect reads every index of a reactive array `[1, 2, 3]` and `fill(9)` is called from outside any effect +- **THEN** the effect re-runs exactly once and reads `[9, 9, 9]` + +#### Scenario: Subscriber of every index wakes once after copyWithin +- **WHEN** an effect reads every index of a reactive array `[1, 2, 3]` and `copyWithin(0, 1)` is called from outside any effect +- **THEN** the effect re-runs exactly once and reads `[2, 3, 3]` + +#### Scenario: Method that changes nothing wakes nobody +- **WHEN** an effect reads every index of a sorted reactive array and `sort()` is called from outside any effect +- **THEN** the effect does not re-run + +#### Scenario: Fill with the value every element holds wakes nobody +- **WHEN** an effect reads every index of a reactive array `[9, 9, 9]` and `fill(9)` is called from outside any effect +- **THEN** the effect does not re-run + +#### Scenario: copyWithin that moves equal values wakes nobody +- **WHEN** an effect reads every index of a reactive array `[7, 7, 7]` and `copyWithin(0, 1)` is called from outside any effect +- **THEN** the effect does not re-run + +### Requirement: Mutating methods compose with batches +A mutating array method called inside a batch SHALL defer its wakes to the outermost batch boundary, where each subscriber of the array wakes at most once for the whole batch. + +#### Scenario: Two methods in one batch wake once +- **WHEN** an effect reads `length` of a reactive array and a batch calls `push(1)` then `pop()` on it +- **THEN** the effect does not re-run before the batch ends and re-runs at most once after it + +#### Scenario: Fill and copyWithin in one batch wake once +- **WHEN** an effect reads every index of a reactive array `[1, 2, 3]` and a batch calls `fill(9, 0, 1)` then `copyWithin(2, 0)` on it +- **THEN** the effect does not re-run before the batch ends and re-runs exactly once after it, reading `[9, 2, 9]` + +### Requirement: Mutating methods are subject to loop detection +A mutating array method reads the properties it writes. When a synchronous top-level effect calls one on a reactive array and the call writes a property, the system SHALL raise the same infinite-loop error it raises for an effect that writes a property it read. A call that writes nothing SHALL raise nothing. A method whose writes are not all to properties it read, `fill` and `copyWithin`, SHALL raise the error only when the call writes a property the effect read, by the same rule and with no exemption. Child effects and microtask effects SHALL keep their exemption; the system SHALL NOT add a new exemption or counter for array methods. + +#### Scenario: Each method throws in a synchronous top-level effect +- **WHEN** a synchronous top-level effect calls `push`, `pop`, `shift`, `unshift`, `splice`, `sort` or `reverse` on a reactive array it did not otherwise read, the array holding at least two elements for `sort` and `reverse` +- **THEN** the effect's first run raises the infinite-loop error, and the error names the effect when it has a name + +#### Scenario: Fill throws only when the effect read an index it writes +- **WHEN** a synchronous top-level effect calls `fill(9)` on a reactive array `[1, 2, 3]` it did not otherwise read, and another synchronous top-level effect reads index 0 of such an array and then calls `fill(9)` on it +- **THEN** the first effect raises nothing and the array reads `[9, 9, 9]`, and the second effect's first run raises the infinite-loop error + +#### Scenario: copyWithin throws only when its ranges overlap +- **WHEN** a synchronous top-level effect calls `copyWithin(2, 0, 1)` on a reactive array `[1, 2, 3]` it did not otherwise read, and another synchronous top-level effect calls `copyWithin(0, 1)` on such an array +- **THEN** the first effect raises nothing and the array reads `[1, 2, 1]`, and the second effect's first run raises the infinite-loop error + +#### Scenario: Method that writes nothing does not throw +- **WHEN** a synchronous top-level effect calls `sort` or `reverse` on a reactive array with fewer than two elements +- **THEN** no infinite-loop error is raised, because the call performs no write + +#### Scenario: Child effect does not throw +- **WHEN** a child effect calls `push` on a reactive array +- **THEN** no infinite-loop error is raised + +#### Scenario: Child and microtask effects do not throw for fill or copyWithin +- **WHEN** a child effect and a microtask effect each read index 0 of a reactive array `[1, 2, 3]` and call `fill(9)` on it, and another pair each call `copyWithin(0, 1)` on such an array +- **THEN** no infinite-loop error is raised for any of them + +#### Scenario: Microtask effect converges +- **WHEN** a microtask effect pushes onto a reactive array while its `length` is below 3 +- **THEN** no error is raised and the array ends with length 3 + +### Requirement: State hooks observe mutating methods +On state created with hooks, the `onRead` hook SHALL see the method name read and every property the method reads, and the `onWrite` hook SHALL see every property the method writes, with the same arguments a direct property access produces. + +#### Scenario: onWrite sees a push +- **WHEN** state with an `onWrite` hook holds an empty array and `push(1)` is called +- **THEN** the hook is called for index `0` with old value `undefined` and new value `1`, and for `length` only if the write changes it + +#### Scenario: onRead sees the method and its reads +- **WHEN** state with an `onRead` hook holds an array and `push(1)` is called +- **THEN** the hook is called with `push` and with `length` + +#### Scenario: Hooks see a fill +- **WHEN** state with `onRead` and `onWrite` hooks holds `[1, 2, 3]` and `fill(9, 1)` is called +- **THEN** `onRead` is called with `fill` and with `length`, and `onWrite` is called for index `1` with old value `2` and new value `9` and for index `2` with old value `3` and new value `9`, and for no other property + +#### Scenario: Hooks see a copyWithin +- **WHEN** state with `onRead` and `onWrite` hooks holds `[1, 2, 3]` and `copyWithin(0, 2)` is called +- **THEN** `onRead` is called with `copyWithin`, with `length` and with index `2`, and `onWrite` is called for index `0` with old value `1` and new value `3`, and for no other property + +### Requirement: Results equal the native method +The return value of a wrapped method and the contents of the array afterwards SHALL equal those of the same native method on a plain array with the same contents and arguments, for every argument shape the native method accepts. On a non-extensible array the native method's error SHALL propagate unchanged. + +#### Scenario: Splice with removals +- **WHEN** `splice(1, 2, 9)` is called on a reactive array `[0, 1, 2, 3]` +- **THEN** it returns `[1, 2]` and the array reads `[0, 9, 3]` + +#### Scenario: Sort with a comparator +- **WHEN** `sort((a, b) => b - a)` is called on a reactive array `[1, 3, 2]` +- **THEN** it returns the same array and the array reads `[3, 2, 1]` + +#### Scenario: Fill with negative and out-of-range bounds +- **WHEN** `fill(9, -2)` and then `fill(0, 1, 100)` are called on a reactive array `[1, 2, 3]` +- **THEN** each returns the same array, the array reads `[1, 9, 9]` after the first call and `[1, 0, 0]` after the second, and `length` stays `3` + +#### Scenario: copyWithin with negative and overlapping bounds +- **WHEN** `copyWithin(-2, 0)` is called on a reactive array `[1, 2, 3, 4]` +- **THEN** it returns the same array and the array reads `[1, 2, 1, 2]` + +#### Scenario: Push of undefined creates the element +- WHEN `push(undefined)` is called on a reactive array `[0, 1, 2]`, on state created without hooks and on state created with hooks +- THEN `length` reads `4`, index `3` is an own property as it is on a plain array, and on hooked state `onWrite` is called for index `3` with old value `undefined` and new value `undefined` +#### Scenario: Push on a sealed array +- **WHEN** `push(1)` is called on reactive state over a sealed array +- **THEN** a `TypeError` is raised, as the native method raises, and the system stays usable + +#### Scenario: Fill on a frozen array +- **WHEN** `fill(9)` is called on reactive state over a frozen array holding at least one element +- **THEN** a `TypeError` is raised, as the native method raises, and the system stays usable diff --git a/openspec/changes/wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md b/openspec/changes/wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md new file mode 100644 index 0000000..c48eda5 --- /dev/null +++ b/openspec/changes/wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md @@ -0,0 +1,11 @@ +# 01: Benchmark case for fill and the trunk baseline + +**Area:** state + +**Blocked by:** None (can start immediately) + +**What to build:** The benchmark suite gains one case that measures the pattern this change alters: one effect subscribed to a reactive array created without hooks, then a timed loop that calls `fill` on it with a value that changes every element each time (for example alternating two values), shaped like the existing array push/pop case and reporting through the same result printer. Nothing else changes: this ticket touches the benchmark script only, so the numbers it records are the baseline of the unchanged library. The full suite is then run with the package's benchmark script at its default run count (`npm run benchmark`, 3 cycles, as the previous ledgers ruled), and the complete output, including every case line and the grand total line, is pasted into the ticket report. Ticket 04 compares against this output, so the report must hold the exact figures, not a summary. + +- [ ] The benchmark suite lists one new entry that subscribes one effect to a hookless reactive array and times a loop of `fill` calls that change every element on each call; it reports through the same result printer as the existing cases +- [ ] The library source, tests, docs, specs and manifests are unchanged in this ticket (`git diff --stat` against the branch base lists only the benchmark script) +- [ ] The ticket report contains the full output of the package's benchmark script, including every case line and the grand total line, from a run on this ticket's commit, with the Node version and machine named diff --git a/openspec/changes/wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md b/openspec/changes/wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md new file mode 100644 index 0000000..3ae6774 --- /dev/null +++ b/openspec/changes/wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md @@ -0,0 +1,15 @@ +# 02: Wrap fill and copyWithin through the Proxy + +**Area:** state + +**Blocked by:** 01 + +**What to build:** A reactive array hands out `fill` and `copyWithin` the way it hands out `shift`, `unshift`, `splice`, `sort` and `reverse`: the cached wrapper that applies the native method to the Proxy receiver inside a batch. The only library change is the two names joining the set of mutating method names the get trap consults, plus any comment that counts the Proxy-receiver wrappers as five. No direct path, no change to the lookup body, the cache or the batch bookkeeping. From outside a batch a call wakes each subscriber of the array at most once and after the method returns; a call that changes no element wakes nobody; inside a user batch the wakes wait for the outermost boundary; the return value and contents equal the native method, including negative and out-of-range bounds; a frozen array raises the native `TypeError` and the system stays usable. Every assertion runs on state created without hooks and on state created with hooks, in the existing array-method test file and its property-based counterpart. + +- [ ] An effect reading every index of a reactive `[1, 2, 3]` re-runs exactly once after `fill(9)` and reads `[9, 9, 9]`, and exactly once after `copyWithin(0, 1)` and reads `[2, 3, 3]`, on both kinds of state +- [ ] `fill(9)` on `[9, 9, 9]` and `copyWithin(0, 1)` on `[7, 7, 7]` re-run such an effect zero times, on both kinds of state +- [ ] Inside a user batch that calls `fill(9, 0, 1)` then `copyWithin(2, 0)` on `[1, 2, 3]`, the effect does not re-run before the batch ends and re-runs exactly once after it, reading `[9, 2, 9]` +- [ ] `fill(9, -2)` then `fill(0, 1, 100)` on `[1, 2, 3]` each return the same array and leave `[1, 9, 9]` then `[1, 0, 0]` with `length` 3; `copyWithin(-2, 0)` on `[1, 2, 3, 4]` returns the same array and leaves `[1, 2, 1, 2]` +- [ ] The property-based array test generates `fill` and `copyWithin` calls with arbitrary bounds beside the existing methods and finds the reactive result equal to the plain-array result +- [ ] `fill(9)` on reactive state over a frozen array with at least one element raises a `TypeError` and a later write on other state still notifies +- [ ] `npm test`, `npm run test:coverage` and `npm run check` pass, and `git diff --stat` against the ticket's base shows the library source changed only in the set of method names and a comment diff --git a/openspec/changes/wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md b/openspec/changes/wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md new file mode 100644 index 0000000..e1cb4c8 --- /dev/null +++ b/openspec/changes/wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md @@ -0,0 +1,16 @@ +# 03: Loop detection and hook assertions, spec purpose and agent docs + +**Area:** state + +**Blocked by:** 02 + +**What to build:** Loop detection keeps its rule, a write to a property the effect read raises the infinite-loop error, and the tests pin what that rule means for the two methods whose writes are not all to properties they read: `fill` alone in a synchronous top-level effect raises nothing, and raises when the effect also read an index the call writes; `copyWithin` raises only when its source and target ranges overlap; child effects and microtask effects raise nothing for either. On hooked state the read hook sees the method name and every property the method reads, and the write hook sees every property it writes and no other, in the existing state-hooks test file. The main `array-mutation` spec's purpose line, which archive does not rewrite from the delta, names nine methods, as does the key-behaviors sentence of the root agent docs; the source folder agent docs stop counting "the other five" wrappers and say seven. The handbook is not touched. + +- [ ] A synchronous top-level effect that calls `fill(9)` on a reactive `[1, 2, 3]` it did not otherwise read raises nothing and the array reads `[9, 9, 9]`; one that reads index 0 first and then calls `fill(9)` raises the infinite-loop error on its first run, on both kinds of state +- [ ] A synchronous top-level effect that calls `copyWithin(2, 0, 1)` on a reactive `[1, 2, 3]` raises nothing and the array reads `[1, 2, 1]`; one that calls `copyWithin(0, 1)` raises the infinite-loop error on its first run, on both kinds of state +- [ ] A child effect and a microtask effect that each read index 0 and call `fill(9)`, and a pair that each call `copyWithin(0, 1)`, raise nothing, and the arrays end with the native contents +- [ ] On state with `onRead` and `onWrite` hooks holding `[1, 2, 3]`, `fill(9, 1)` calls `onRead` with `fill` and with `length`, and calls `onWrite` for index `1` (old `2`, new `9`) and index `2` (old `3`, new `9`) and for no other property +- [ ] On such state, `copyWithin(0, 2)` calls `onRead` with `copyWithin`, with `length` and with index `2`, and calls `onWrite` for index `0` (old `1`, new `3`) and for no other property +- [ ] The main spec's purpose line and the root agent docs' key-behaviors sentence name nine methods including `fill` and `copyWithin`, and no sentence in the source folder agent docs counts the Proxy-receiver wrappers as five +- [ ] No file under the handbook folder changes in this ticket (`git diff --stat` against the ticket's base lists none) +- [ ] `npm test`, `npm run test:coverage` and `npm run check` pass diff --git a/openspec/changes/wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md b/openspec/changes/wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md new file mode 100644 index 0000000..13ea69e --- /dev/null +++ b/openspec/changes/wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md @@ -0,0 +1,13 @@ +# 04: Benchmark comparison and performance gate + +**Area:** state + +**Blocked by:** 03 + +**What to build:** The change is complete only when it made Beacon no slower. The full benchmark suite is run on the final commit with the package's benchmark script at its default run count, on the same machine and under the same conditions as ticket 01's run, and the complete output is pasted into the ticket report beside ticket 01's figures. The gate is the per-case rule of Q84: no existing case's total may exceed ticket 01's by more than 1%, speedups are unlimited. If a 3-cycle pairing leaves a case over the line, that case is re-measured at 10 cycles in alternating order (baseline, final, final, baseline) and judged on that, as the previous change's ledger records. The new `fill` case is reported before and after; it is expected to be faster, since one wake replaces several, and the report states its medians from both runs. If the gate fails after re-measurement, this ticket iterates on the implementation with every behavior from tickets 02 and 03 kept green, and lists each attempt. This ticket bumps no version and touches no handbook file. + +- [ ] The ticket report contains the full benchmark output from the final commit, every case line and the grand total line, with ticket 01's total and the percentage difference stated per case +- [ ] No existing case is slower than ticket 01's run by more than 1%, either in the 3-cycle pairing or, for any case over the line there, in the 10-cycle alternating re-measurement whose figures are in the report +- [ ] The report states the `fill` case's median and total from ticket 01 and from the final run +- [ ] If any iteration was needed, each attempt is listed with what changed and its figures, and `npm test`, `npm run test:coverage` and `npm run check` pass on the final commit +- [ ] Both package manifests still read version `2002.0.0` and no handbook file differs from the branch base diff --git a/openspec/specs/array-mutation/spec.md b/openspec/specs/array-mutation/spec.md new file mode 100644 index 0000000..8ed4f3b --- /dev/null +++ b/openspec/specs/array-mutation/spec.md @@ -0,0 +1,99 @@ +# array-mutation Specification + +## Purpose + +Defines how a mutating array method (`push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`, `fill`, `copyWithin`) called on reactive state notifies subscribers, composes with batches, is checked for self-feeding loops, and is observed by state hooks, identically for state created with and without hooks; and that the presence of a hook never changes which subscribers a write wakes or when. + +## Requirements + +### Requirement: Mutating methods behave the same with and without hooks +Reactive state SHALL expose the same behavior for the nine mutating array methods (`push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`, `fill`, `copyWithin`) whether the state was created with hooks or without. Hooks SHALL only observe; they SHALL NOT change which subscribers wake, how often, or whether a loop error is raised. + +#### Scenario: Same wake count on both kinds of state +- **WHEN** an effect reads index 0 and `length` of an empty reactive array, and `push(1)` is called from outside any effect, on state created without hooks and on state created with hooks +- **THEN** the effect re-runs exactly once on each + +#### Scenario: Same loop outcome on both kinds of state +- **WHEN** a synchronous top-level effect calls a mutating method on a reactive array, on state created without hooks and on state created with hooks +- **THEN** both raise the same infinite-loop error + +### Requirement: Hook presence does not change notification +Reactive state SHALL notify the same subscribers at the same moment whether the state was created with hooks or without, for every write path: outside a batch, inside a batch from outside any effect, and inside an effect. The `onWrite` hook SHALL be called after the raw write on every path on which a write changes a value, including a write inside a batch. A write of a value equal to the value of an existing property SHALL stay silent on every path and SHALL NOT call `onWrite`. A write of `undefined` to an absent property is a change: it SHALL create the property, notify, and call `onWrite` with old value `undefined`. + +#### Scenario: Batched write wakes the same subscribers on every kind of state +- **WHEN** an effect reads only index 1 of a reactive array `[0, 1, 2]` and a batch assigns index 3 from outside any effect, on state created without hooks, with `onRead` only, with `onWrite` only, and with both +- **THEN** the effect re-runs exactly once on each + +#### Scenario: onWrite sees a batched write +- **WHEN** state with an `onWrite` hook holds `{ a: 1 }` and a batch assigns `a = 2` from outside any effect +- **THEN** the hook is called once with property `a`, old value `1`, new value `2` and the raw target, and a batched assignment of `a = 2` again calls it no further time + +### Requirement: One wake per method call +A mutating array method called from outside any batch SHALL wake each subscriber of that array at most once per call, after the method has returned, regardless of which properties the subscriber read and how many properties the method changed. A call that changes no element and leaves `length` unchanged SHALL wake no subscriber. + +#### Scenario: Subscriber of an unchanged index wakes once +- **WHEN** an effect reads only index 1 of a reactive array `[0, 1, 2]` and `push(3)` is called from outside any effect +- **THEN** the effect re-runs exactly once + +#### Scenario: Subscriber of a changed index and length wakes once +- **WHEN** an effect reads index 0 and `length` of an empty reactive array and `push(1)` is called from outside any effect +- **THEN** the effect re-runs exactly once, and sees the element and the new length + +#### Scenario: Method that changes nothing wakes nobody +- **WHEN** an effect reads every index of a sorted reactive array and `sort()` is called from outside any effect +- **THEN** the effect does not re-run + +### Requirement: Mutating methods compose with batches +A mutating array method called inside a batch SHALL defer its wakes to the outermost batch boundary, where each subscriber of the array wakes at most once for the whole batch. + +#### Scenario: Two methods in one batch wake once +- **WHEN** an effect reads `length` of a reactive array and a batch calls `push(1)` then `pop()` on it +- **THEN** the effect does not re-run before the batch ends and re-runs at most once after it + +### Requirement: Mutating methods are subject to loop detection +A mutating array method reads the properties it writes. When a synchronous top-level effect calls one on a reactive array and the call writes a property, the system SHALL raise the same infinite-loop error it raises for an effect that writes a property it read. A call that writes nothing SHALL raise nothing. Child effects and microtask effects SHALL keep their exemption; the system SHALL NOT add a new exemption or counter for array methods. + +#### Scenario: Each method throws in a synchronous top-level effect +- **WHEN** a synchronous top-level effect calls `push`, `pop`, `shift`, `unshift`, `splice`, `sort` or `reverse` on a reactive array it did not otherwise read, the array holding at least two elements for `sort` and `reverse` +- **THEN** the effect's first run raises the infinite-loop error, and the error names the effect when it has a name + +#### Scenario: Method that writes nothing does not throw +- **WHEN** a synchronous top-level effect calls `sort` or `reverse` on a reactive array with fewer than two elements +- **THEN** no infinite-loop error is raised, because the call performs no write + +#### Scenario: Child effect does not throw +- **WHEN** a child effect calls `push` on a reactive array +- **THEN** no infinite-loop error is raised + +#### Scenario: Microtask effect converges +- **WHEN** a microtask effect pushes onto a reactive array while its `length` is below 3 +- **THEN** no error is raised and the array ends with length 3 + +### Requirement: State hooks observe mutating methods +On state created with hooks, the `onRead` hook SHALL see the method name read and every property the method reads, and the `onWrite` hook SHALL see every property the method writes, with the same arguments a direct property access produces. + +#### Scenario: onWrite sees a push +- **WHEN** state with an `onWrite` hook holds an empty array and `push(1)` is called +- **THEN** the hook is called for index `0` with old value `undefined` and new value `1`, and for `length` only if the write changes it + +#### Scenario: onRead sees the method and its reads +- **WHEN** state with an `onRead` hook holds an array and `push(1)` is called +- **THEN** the hook is called with `push` and with `length` + +### Requirement: Results equal the native method +The return value of a wrapped method and the contents of the array afterwards SHALL equal those of the same native method on a plain array with the same contents and arguments, for every argument shape the native method accepts. On a non-extensible array the native method's error SHALL propagate unchanged. + +#### Scenario: Splice with removals +- **WHEN** `splice(1, 2, 9)` is called on a reactive array `[0, 1, 2, 3]` +- **THEN** it returns `[1, 2]` and the array reads `[0, 9, 3]` + +#### Scenario: Sort with a comparator +- **WHEN** `sort((a, b) => b - a)` is called on a reactive array `[1, 3, 2]` +- **THEN** it returns the same array and the array reads `[3, 2, 1]` + +#### Scenario: Push of undefined creates the element +- WHEN `push(undefined)` is called on a reactive array `[0, 1, 2]`, on state created without hooks and on state created with hooks +- THEN `length` reads `4`, index `3` is an own property as it is on a plain array, and on hooked state `onWrite` is called for index `3` with old value `undefined` and new value `undefined` +#### Scenario: Push on a sealed array +- **WHEN** `push(1)` is called on reactive state over a sealed array +- **THEN** a `TypeError` is raised, as the native method raises, and the system stays usable diff --git a/scripts/benchmark.ts b/scripts/benchmark.ts index 9685a7b..30a3d81 100644 --- a/scripts/benchmark.ts +++ b/scripts/benchmark.ts @@ -12,6 +12,8 @@ const BENCH_RUNS = 7 const EFFECT_TRIGGER_WRITES = 50_000 const ARRAY_PUSH_POP_CALLS = 50_000 +const ARRAY_FILL_CALLS = 50_000 +const ARRAY_FILL_LENGTH = 3 const MULTI_SOURCE_COUNT = 100 const MULTI_SOURCE_ITERATIONS = 100 const DERIVE_CHAIN_DEPTH = 10 @@ -317,6 +319,20 @@ const arrayPushPopHookless = (): number => arrayPushPop(state([])) const arrayPushPopHooks = (): number => arrayPushPop(state([], noopStateHooks)) +// The effect reads every index because fill never changes length: a length-only reader would not wake at all. +// Alternating the fill value changes every element on every call +function arrayFillHookless(): number { + const items = state(new Array(ARRAY_FILL_LENGTH).fill(0)) + const cleanup: Unsubscribe = effect((): void => { + for (let i = 0; i < items.length; i++) void items[i] + }) + const start = performance.now() + for (let i = 0; i < ARRAY_FILL_CALLS; i++) items.fill((i % 2) + 1) + const end = performance.now() + cleanup() + return end - start +} + function manyDependencies(): number { const sources = Array.from( { @@ -607,6 +623,10 @@ const suite: SuiteEntry[] = [ fn: arrayPushPopHooks, name: 'array push/pop (hooks) ', }, + { + fn: arrayFillHookless, + name: 'array fill (hookless) ', + }, { fn: manyDependencies, name: 'many dependencies ', diff --git a/src/AGENTS.md b/src/AGENTS.md index 7d366f1..0a445c9 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -73,12 +73,13 @@ Effect tracking data stored directly on the function object (O(1) property acces ### Proxy Handlers -Five handler factories, each returns a ProxyHandler method. `state()` without hooks shares one prebuilt `HOOKLESS_HANDLER`; the get factory returns a leaner variant when no hooks are passed, the set factory returns one trap for every state object. +Five handler factories, each returns a ProxyHandler method, and a sixth for the array set trap. `state()` without hooks shares one prebuilt `HOOKLESS_HANDLER`, or `HOOKLESS_ARRAY_HANDLER` for an array target; the get factory returns a leaner variant when no hooks are passed, the set factory returns one trap for every state object that is not an array. | Handler | Traps | Key behavior | |---------|-------|-------------| | `createGetHandler()` | `get` | One keyed load of the raw target, then the internal-symbol early return (a separate load site for the symbols saw only array maps, lost its feedback when those arrays were collected, and deoptimized the trap on the next direct push/pop); delegates to `trackDependency` for reads and `wrapNestedObject` for nested wrapping; both variants route mutating array methods through `getWrappedArrayMethod` before the non-object early return | -| `createSetHandler()` | `set` | One trap for hooked and hookless state: `handleBatchFastPath` during batch from outside any effect, `performUnbatchedWrite` outside any effect, `performWrite` inside an effect; `onWrite` only observes after the raw write on every path | +| `createSetHandler()` | `set` | One trap for hooked and hookless state of a non-array target: `handleBatchFastPath` during batch from outside any effect, `performUnbatchedWrite` outside any effect, `performWrite` inside an effect; `onWrite` only observes after the raw write on every path | +| `createArraySetHandler()` | `set` | The same trap for an array target, `state()` picks it by `Array.isArray` once per call; its batch branch is `handleBatchArrayWrite`, so no set trap closure and no batch write function sees both arrays and objects | | `createDeleteHandler()` | `deleteProperty` | Deletion with subscriber notification | | `createHasHandler()` | `has` | `in` operator tracking via `trackDependency` | | `createOwnKeysHandler()` | `ownKeys` | Key enumeration tracking via `OWN_KEYS_SYMBOL` | @@ -96,7 +97,7 @@ Handler helpers: | `isInternalSymbol` | Guards access to `SUBSCRIBERS`, `PROXY`, `HOOKS` symbols | | `getWrappedArrayMethod` | Returns the wrapper for a mutating array method read from an array target (`Array.isArray` first, so plain-object reads pay one call and one check; then the `arrayMethodCache` hit, before the method-name check). Its body is a read-path hot spot: any change to it, even code a plain-object read never reaches, cost 2 to 5% on the read benchmark, so the direct `push`/`pop` dispatch lives in the cached wrappers | | `applyInBatch` | Runs a method on a receiver inside the hookless batch bookkeeping (`batchDepth++`, then `abortBatch` or `endBatch`, without the `batch(fn)` closure); shared by the Proxy-receiver wrappers and the direct `push`/`pop` | -| `wrapArrayMethod` | Wraps a native mutating array method once, cached in `arrayMethodCache`: the native `push` and `pop` get `directPushWrapper`/`directPopWrapper`; the other five get a wrapper that applies them via `applyInBatch` to the call receiver (the Proxy), so every read and write passes the traps (seen by `onRead`/`onWrite`, subject to the loop check inside an effect) and each subscriber wakes at most once per call | +| `wrapArrayMethod` | Wraps a native mutating array method once, cached in `arrayMethodCache`: the native `push` and `pop` get `directPushWrapper`/`directPopWrapper`; the other seven get a wrapper that applies them via `applyInBatch` to the call receiver (the Proxy), so every read and write passes the traps (seen by `onRead`/`onWrite`, subject to the loop check inside an effect) and each subscriber wakes at most once per call | | `directPushWrapper` / `directPopWrapper` | Read the receiver's `[HOOKS]` slot (optional chaining, so a nullish receiver reads no slot): a `DirectArrayContext` (checked with `instanceof`, so only a reactive array's slot qualifies) on a receiver that is not the context's own raw target runs `directPush`/`directPop` on it inside `applyInBatch`; any other receiver (such as `push.call(otherArray)`, the raw array target, `null` or `undefined`) gets the native method inside `applyInBatch`: the raw target never enters the traps, so the call is silent (no hook call, no wake, Q71 b), and a nullish receiver raises the native TypeError | | `hooksSlotFor` | What `state()` stores in the `[HOOKS]` slot: for an array target a new `DirectArrayContext`, otherwise `undefined` (no slot) | | `directPush` / `directPop` | Run the native method on the raw target inside `applyInBatch` and replay by hand what the traps would have done: `push` reads `length`, then sets each new index and `length`; `pop` reads `length` and the last index, deletes that index (`onDelete`, readers of the index scheduled, no loop or equal check) and sets `length`; `pop` returns the nested wrapper the get trap would | @@ -106,7 +107,8 @@ Handler helpers: | `performUnbatchedWrite` | Outside any effect and batch: skips an unchanged write (`isUnchangedWrite`), skips scheduling when the target has no subscribers, else notifies per property | | `performWrite` | Checks infinite loop, skips an unchanged write (`isUnchangedWrite`), notifies subscribers | | `checkInfiniteLoop` | Throws if effect writes to a property it read (exempt: child effects, microtask effects) | -| `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty | +| `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty; object targets only | +| `handleBatchArrayWrite` | The array twin of `handleBatchFastPath`: the same write, equal skip and `onWrite`, then one `ARRAY_DIRTY_PROPS` entry for the array. Kept apart because arrays written through the shared path (once `fill` and `copyWithin` went through it) made its `[SUBSCRIBERS]` load deoptimize on every benchmark cycle and slowed `many dependencies` by 10.6% on medians and 6.8% on totals (interleaved 10-cycle A/B against 87c8823) | | `getArrayLengthBeforeMutation` | Captures array length before index assignment | | `notifyLengthChangeIfNeeded` | Notifies `length` subscribers if array grew/shrank | diff --git a/src/index.ts b/src/index.ts index e103daa..4d82aa6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -94,7 +94,17 @@ const subscribersByProp: WeakMap, Map> >() -const MUTATING_ARRAY_METHODS: Set = new Set(['push', 'pop', 'shift', 'unshift', 'splice', 'sort', 'reverse']) +const MUTATING_ARRAY_METHODS: Set = new Set([ + 'push', + 'pop', + 'shift', + 'unshift', + 'splice', + 'sort', + 'reverse', + 'fill', + 'copyWithin', +]) // A native mutating array method, or the wrapper the Proxy hands out in its place type ArrayMethod = (...args: unknown[]) => unknown @@ -211,7 +221,7 @@ function wrapArrayMethod(nativeMethod: ArrayMethod): ArrayMethod { // context is a reactive array and takes the direct path; any other receiver, such as push.call(otherArray), runs // the native method in a batch. A nullish receiver has no slot to read, so the native method raises its own // TypeError. The raw target carries the same slot but never enters the traps, so a call on it runs the native -// method silently, as the other five wrappers and a direct write on the raw object do (Q71 b) +// method silently, as the other seven wrappers and a direct write on the raw object do (Q71 b) function directPushWrapper(this: unknown, ...items: unknown[]): unknown { const context = (this as ProxyObject | null | undefined)?.[HOOKS] if (!(context instanceof DirectArrayContext) || this === context.target) { @@ -746,12 +756,13 @@ function createGetHandler( // Only an array target gets a [HOOKS] slot: the context its direct push and pop find through the Proxy receiver function hooksSlotFor( target: ProxyTarget, + isArray: boolean, hooks: StateHooks | undefined, onDelete: HookFunction<[PropertyKey, boolean, T]> | undefined, onRead: HookFunction<[PropertyKey, unknown, T]> | undefined, onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined ): HooksSlot { - if (!Array.isArray(target)) return undefined + if (!isArray) return undefined return new DirectArrayContext( target as ArrayTarget, hooks as StateHooks | undefined, @@ -810,7 +821,6 @@ function handleBatchFastPath( // A target already in dirtyTargets had subscribers at its first write, so only the first write looks them up const props = dirtyTargets.get(rawTarget) - if (props === ARRAY_DIRTY_PROPS) return true if (props) { props.add(prop) return true @@ -823,11 +833,34 @@ function handleBatchFastPath( } if (!subs?.size) return true - if (Array.isArray(rawTarget)) { - dirtyTargets.set(rawTarget, ARRAY_DIRTY_PROPS) - } else { - dirtyTargets.set(rawTarget, new Set([prop])) + dirtyTargets.set(rawTarget, new Set([prop])) + return true +} + +// The array twin of handleBatchFastPath, a separate function so that the object path never sees an array: once +// fill and copyWithin wrote arrays through the shared path, its [SUBSCRIBERS] load deoptimized on every benchmark +// cycle and `many dependencies` ran 10.6% slower on medians and 6.8% on totals (interleaved 10-cycle A/B against +// 87c8823). An array goes dirty once and wakes all its subscribers +function handleBatchArrayWrite( + rawTarget: ProxyTarget, + prop: PropertyKey, + value: unknown, + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined +): boolean { + const oldValue = rawTarget[prop] + if (isUnchangedWrite(rawTarget, prop, oldValue, value)) return true + + rawTarget[prop] = value + if (onWrite) callHookSafe(onWrite, prop, oldValue, value, rawTarget as T) + + if (dirtyTargets.get(rawTarget) === ARRAY_DIRTY_PROPS) return true + + // inlined, not findSubscribers: the call cost 2.2% on the fill benchmark + let subs = (rawTarget as SubscribersObject)[SUBSCRIBERS] + if (subs === undefined && (hasFallbackSubs || (rawTarget as ProxyObject)[PROXY] === undefined)) { + subs = proxyCacheSubs.get(rawTarget) } + if (subs?.size) dirtyTargets.set(rawTarget, ARRAY_DIRTY_PROPS) return true } @@ -866,6 +899,22 @@ function createSetHandler( } } +// The set trap of an array target: createSetHandler with the batch branch on handleBatchArrayWrite, a literal of +// its own so that no trap closure calls both batch paths +function createArraySetHandler( + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined +): NonNullable['set']> { + return (rawTarget: ProxyTarget, prop: PropertyKey, value: unknown): boolean => { + if (batchDepth > 0 && !currentEffect) { + return handleBatchArrayWrite(rawTarget, prop, value, onWrite) + } + if (!currentEffect) { + return performUnbatchedWrite(rawTarget, prop, value, onWrite) + } + return performWrite(rawTarget, prop, value, onWrite) + } +} + // Outside any effect and batch: no loop check, and a target without subscribers skips scheduling function performUnbatchedWrite( rawTarget: ProxyTarget, @@ -949,6 +998,11 @@ const HOOKLESS_HANDLER: ProxyHandler = { set: createSetHandler(undefined), } +const HOOKLESS_ARRAY_HANDLER: ProxyHandler = { + ...HOOKLESS_HANDLER, + set: createArraySetHandler(undefined), +} + /** Wrap an object in a reactive Proxy. Property reads are tracked, writes notify subscribers. */ export function state(initial: T, hooks?: StateHooks): T { if (initial === null || initial === undefined || typeof initial !== 'object') return initial @@ -961,9 +1015,12 @@ export function state(initial: T, hooks?: StateHooks): T { const cached = proxyCache.get(target) if (cached) return cached as T + const isArray = Array.isArray(target) if (!hooks) { - const proxy = new Proxy(target, HOOKLESS_HANDLER) as T - if (!defineProxyProperties(target, proxy, hooksSlotFor(target, undefined, undefined, undefined, undefined))) { + const proxy = new Proxy(target, isArray ? HOOKLESS_ARRAY_HANDLER : HOOKLESS_HANDLER) as T + if ( + !defineProxyProperties(target, proxy, hooksSlotFor(target, isArray, undefined, undefined, undefined, undefined)) + ) { proxyCache.set(target, proxy) } return proxy @@ -980,11 +1037,11 @@ export function state(initial: T, hooks?: StateHooks): T { get: createGetHandler(onRead, hooks), has: createHasHandler(onHas), ownKeys: createOwnKeysHandler(onOwnKeys), - set: createSetHandler(onWrite), + set: isArray ? createArraySetHandler(onWrite) : createSetHandler(onWrite), } as ProxyHandler const proxy = new Proxy(target, handler) as T - if (!defineProxyProperties(target, proxy, hooksSlotFor(target, hooks, onDelete, onRead, onWrite))) { + if (!defineProxyProperties(target, proxy, hooksSlotFor(target, isArray, hooks, onDelete, onRead, onWrite))) { proxyCache.set(target, proxy) } return proxy diff --git a/tests/array-methods.test.ts b/tests/array-methods.test.ts index 84fc79d..fd9205e 100644 --- a/tests/array-methods.test.ts +++ b/tests/array-methods.test.ts @@ -107,6 +107,8 @@ describe('Array Mutating Methods', { ['splice', (list: number[]): unknown => list.splice(1, 2, 9)], ['sort', (list: number[]): unknown => list.sort(descending)], ['reverse', (list: number[]): unknown => list.reverse()], + ['fill', (list: number[]): unknown => list.fill(9, -3, 3)], + ['copyWithin', (list: number[]): unknown => list.copyWithin(1, 2)], ] for (const [name, call] of cases) { const plain = [0, 1, 2, 3] @@ -123,6 +125,103 @@ describe('Array Mutating Methods', { } }) + it('wakes an effect reading every index once after fill and once after copyWithin', (): void => { + const cases: [string, (list: number[]) => unknown, number[]][] = [ + ['fill', (list: number[]): unknown => list.fill(9), [9, 9, 9]], + ['copyWithin', (list: number[]): unknown => list.copyWithin(0, 1), [2, 3, 3]], + ] + for (const [name, call, expected] of cases) { + for (const $list of bothKinds([1, 2, 3])) { + const seen: number[][] = [] + const dispose = effect((): void => { + seen.push([0, 1, 2].map((i: number): number => $list[i] as number)) + }) + + call($list) + + assert.deepStrictEqual(seen, [[1, 2, 3], expected], name) + dispose() + } + } + }) + + it('does not wake an effect when fill or copyWithin changes no element', (): void => { + const cases: [string, (list: number[]) => unknown, number[]][] = [ + ['fill', (list: number[]): unknown => list.fill(9), [9, 9, 9]], + ['copyWithin', (list: number[]): unknown => list.copyWithin(0, 1), [7, 7, 7]], + ] + for (const [name, call, initial] of cases) { + for (const $list of bothKinds(initial)) { + let runs = 0 + const dispose = effect((): void => { + runs++ + for (let i = 0; i < 3; i++) void $list[i] + }) + + call($list) + + assert.strictEqual(runs, 1, name) + dispose() + } + } + }) + + it('defers wakes from fill and copyWithin to the end of a user batch', (): void => { + for (const $list of bothKinds([1, 2, 3])) { + const seen: number[][] = [] + const dispose = effect((): void => { + seen.push([0, 1, 2].map((i: number): number => $list[i] as number)) + }) + + batch((): void => { + $list.fill(9, 0, 1) + $list.copyWithin(2, 0) + assert.strictEqual(seen.length, 1) + }) + + assert.deepStrictEqual(seen, [ + [1, 2, 3], + [9, 2, 9], + ]) + dispose() + } + }) + + it('resolves negative and out-of-range bounds in fill and copyWithin like the native method', (): void => { + for (const $list of bothKinds([1, 2, 3])) { + assert.strictEqual($list.fill(9, -2), $list) + assert.deepStrictEqual([...$list], [1, 9, 9]) + assert.strictEqual($list.fill(0, 1, 100), $list) + assert.deepStrictEqual([...$list], [1, 0, 0]) + assert.strictEqual($list.length, 3) + } + + for (const $list of bothKinds([1, 2, 3, 4])) { + assert.strictEqual($list.copyWithin(-2, 0), $list) + assert.deepStrictEqual([...$list], [1, 2, 1, 2]) + } + }) + + it('propagates the TypeError from fill on a frozen array and stays usable', (): void => { + const frozen = (): number[] => Object.freeze([1, 2, 3]) as number[] + for (const $list of [state(frozen()), state(frozen(), observingHooks)]) { + const $other = state({ n: 0 }) + let runs = 0 + const dispose = effect((): void => { + runs++ + void $other.n + }) + + assert.throws((): void => { + $list.fill(9) + }, TypeError) + + $other.n = 1 + assert.strictEqual(runs, 2) + dispose() + } + }) + it('propagates the TypeError from push on a sealed array and stays usable', (): void => { for (const $list of [state(Object.seal([1])), state(Object.seal([1]), observingHooks)]) { let runs = 0 diff --git a/tests/infinite-loop.test.ts b/tests/infinite-loop.test.ts index c7bf841..249d985 100644 --- a/tests/infinite-loop.test.ts +++ b/tests/infinite-loop.test.ts @@ -371,6 +371,123 @@ describe('Infinite Loop Detection', { } }) + it('raises nothing when fill alone in a sync effect writes indices it did not read', (): void => { + for (const [, $list] of bothKinds([1, 2, 3])) { + unsubscribes.push( + effect((): void => { + $list.fill(9) + }) + ) + assert.deepStrictEqual([...$list], [9, 9, 9]) + } + }) + + it('raises the loop error when a sync effect reads an index before fill writes it on both kinds of state', (): void => { + for (const [kind, $list] of bothKinds([1, 2, 3])) { + assert.throws( + (): void => { + unsubscribes.push( + effect((): void => { + void $list[0] + $list.fill(9) + }, 'filler') + ) + }, + /Infinite loop detected: effect "filler" cannot update property "/, + kind + ) + } + }) + + it('raises nothing when copyWithin in a sync effect has source and target ranges that do not overlap', (): void => { + for (const [, $list] of bothKinds([1, 2, 3])) { + unsubscribes.push( + effect((): void => { + $list.copyWithin(2, 0, 1) + }) + ) + assert.deepStrictEqual([...$list], [1, 2, 1]) + } + }) + + it('raises the loop error when copyWithin in a sync effect has overlapping ranges on both kinds of state', (): void => { + for (const [kind, $list] of bothKinds([1, 2, 3])) { + assert.throws( + (): void => { + unsubscribes.push( + effect((): void => { + $list.copyWithin(0, 1) + }, 'copier') + ) + }, + /Infinite loop detected: effect "copier" cannot update property "/, + kind + ) + } + }) + + it('lets a child effect call fill and copyWithin on indices it read without raising', (): void => { + for (const [, $list] of bothKinds([1, 2, 3])) { + unsubscribes.push( + effect((): void => { + effect((): void => { + void $list[0] + $list.fill(9) + }) + }) + ) + assert.deepStrictEqual([...$list], [9, 9, 9]) + } + for (const [, $list] of bothKinds([1, 2, 3])) { + unsubscribes.push( + effect((): void => { + effect((): void => { + $list.copyWithin(0, 1) + }) + }) + ) + assert.deepStrictEqual([...$list], [2, 3, 3]) + } + }) + + it('lets a microtask effect call fill and copyWithin on indices it read without raising', async (): Promise => { + const settle = async (): Promise => { + // A few turns cover the one re-run each call causes, plus a spare to prove it stops + for (let turn = 0; turn < 4; turn++) { + await new Promise((resolve): void => queueMicrotask(resolve)) + } + } + for (const [, $list] of bothKinds([1, 2, 3])) { + unsubscribes.push( + effect( + (): void => { + void $list[0] + $list.fill(9) + }, + undefined, + undefined, + { schedule: 'microtask' } + ) + ) + await settle() + assert.deepStrictEqual([...$list], [9, 9, 9]) + } + for (const [, $list] of bothKinds([1, 2, 3])) { + unsubscribes.push( + effect( + (): void => { + $list.copyWithin(0, 1) + }, + undefined, + undefined, + { schedule: 'microtask' } + ) + ) + await settle() + assert.deepStrictEqual([...$list], [3, 3, 3]) + } + }) + it('lets a child effect push onto a reactive array without raising', (): void => { for (const [, $list] of bothKinds([])) { unsubscribes.push( diff --git a/tests/property-array-mutations.test.ts b/tests/property-array-mutations.test.ts index af965c6..e2d8c71 100644 --- a/tests/property-array-mutations.test.ts +++ b/tests/property-array-mutations.test.ts @@ -15,6 +15,20 @@ import { batch, effect, state } from '../src/index.ts' // --- Operation types (keys sorted per Biome useSortedKeys) --- +interface CopyWithinOp { + end: number | undefined + op: 'copyWithin' + start: number + target: number +} + +interface FillOp { + end: number | undefined + op: 'fill' + start: number | undefined + value: number +} + interface PopOp { op: 'pop' } @@ -48,12 +62,16 @@ interface UnshiftOp { op: 'unshift' } -type ArrayOp = PopOp | PushOp | ReverseOp | ShiftOp | SortOp | SpliceOp | UnshiftOp +type ArrayOp = CopyWithinOp | FillOp | PopOp | PushOp | ReverseOp | ShiftOp | SortOp | SpliceOp | UnshiftOp // --- Operations that return a meaningful value (not the array itself) --- type ValueReturningOp = PopOp | PushOp | ShiftOp | SpliceOp | UnshiftOp +// --- Operations that return the receiver --- + +type ReceiverReturningOp = CopyWithinOp | FillOp | ReverseOp | SortOp + // --- Arbitraries --- const pushArb: fc.Arbitrary = fc.record({ @@ -118,6 +136,29 @@ const reverseArb: fc.Arbitrary = fc.record({ op: fc.constant('reverse' as const), }) +// Bounds reach past both ends of arrays of up to 20 elements, so negative and out-of-range cases occur +const boundArb: fc.Arbitrary = fc.integer({ + max: 30, + min: -30, +}) + +const fillArb: fc.Arbitrary = fc.record({ + end: fc.option(boundArb, { nil: undefined }), + op: fc.constant('fill' as const), + start: fc.option(boundArb, { nil: undefined }), + value: fc.integer({ + max: 1000, + min: -1000, + }), +}) + +const copyWithinArb: fc.Arbitrary = fc.record({ + end: fc.option(boundArb, { nil: undefined }), + op: fc.constant('copyWithin' as const), + start: boundArb, + target: boundArb, +}) + const arrayOpArb: fc.Arbitrary = fc.oneof( pushArb, popArb, @@ -125,9 +166,13 @@ const arrayOpArb: fc.Arbitrary = fc.oneof( unshiftArb, spliceArb, sortArb, - reverseArb + reverseArb, + fillArb, + copyWithinArb ) +const receiverReturningOpArb: fc.Arbitrary = fc.oneof(sortArb, reverseArb, fillArb, copyWithinArb) + const valueReturningOpArb: fc.Arbitrary = fc.oneof(pushArb, popArb, shiftArb, unshiftArb, spliceArb) const initialArrayArb: fc.Arbitrary = fc.array( @@ -150,6 +195,12 @@ const opsArb: fc.Arbitrary = fc.array(arrayOpArb, { function applyOp(arr: number[], op: ArrayOp): void { switch (op.op) { + case 'copyWithin': + arr.copyWithin(op.target, op.start, op.end) + break + case 'fill': + arr.fill(op.value, op.start, op.end) + break case 'pop': arr.pop() break @@ -189,6 +240,19 @@ function applyOpWithReturn(arr: number[], op: ValueReturningOp): number | number } } +function applyOpWithReceiver(arr: number[], op: ReceiverReturningOp): number[] { + switch (op.op) { + case 'copyWithin': + return arr.copyWithin(op.target, op.start, op.end) + case 'fill': + return arr.fill(op.value, op.start, op.end) + case 'reverse': + return arr.reverse() + case 'sort': + return arr.sort() + } +} + // --- Tests --- describe('Property-Based: Array Mutations', { @@ -233,6 +297,24 @@ describe('Property-Based: Array Mutations', { ) }) + it('returns the receiver from fill, copyWithin, sort and reverse', (): void => { + fc.assert( + fc.property(initialArrayArb, receiverReturningOpArb, (initial: number[], op: ReceiverReturningOp): void => { + const $reactive = state([...initial]) + const plain = [...initial] + + const reactiveResult = applyOpWithReceiver($reactive, op) + applyOpWithReceiver(plain, op) + + assert.strictEqual(reactiveResult, $reactive) + assert.deepStrictEqual([...$reactive], plain) + }), + { + numRuns: 300, + } + ) + }) + it('fires effect at most once per batch of mutations', (): void => { fc.assert( fc.property(initialArrayArb, opsArb, (initial: number[], ops: ArrayOp[]): void => { diff --git a/tests/state-hooks.test.ts b/tests/state-hooks.test.ts index e2153d4..6e7efa0 100644 --- a/tests/state-hooks.test.ts +++ b/tests/state-hooks.test.ts @@ -279,6 +279,48 @@ describe('State Hooks', { assert.ok(reads.includes('length')) }) + it('fires onRead with the method name and length and onWrite only for the indices fill changes', () => { + const reads: PropertyKey[] = [] + const writes: unknown[][] = [] + const $list = state([1, 2, 3], { + onRead: (prop: PropertyKey) => { + reads.push(prop) + }, + onWrite: (prop: PropertyKey, oldValue: unknown, newValue: unknown) => { + writes.push([prop, oldValue, newValue]) + }, + }) + + $list.fill(9, 1) + + assert.ok(reads.includes('fill')) + assert.ok(reads.includes('length')) + assert.deepStrictEqual(writes, [ + ['1', 2, 9], + ['2', 3, 9], + ]) + }) + + it('fires onRead with the method name, length and source index and onWrite only for the target index of copyWithin', () => { + const reads: PropertyKey[] = [] + const writes: unknown[][] = [] + const $list = state([1, 2, 3], { + onRead: (prop: PropertyKey) => { + reads.push(prop) + }, + onWrite: (prop: PropertyKey, oldValue: unknown, newValue: unknown) => { + writes.push([prop, oldValue, newValue]) + }, + }) + + $list.copyWithin(0, 2) + + assert.ok(reads.includes('copyWithin')) + assert.ok(reads.includes('length')) + assert.ok(reads.includes('2')) + assert.deepStrictEqual(writes, [['0', 1, 3]]) + }) + it('fires onWrite for direct array index assignment', () => { const writes: string[] = [] const $s = state(