diff --git a/HOOKS_TODO.md b/HOOKS_TODO.md index 8f70812..a5afd7c 100644 --- a/HOOKS_TODO.md +++ b/HOOKS_TODO.md @@ -41,7 +41,7 @@ Anything that needs veto, throw-through, delay, or post-run timing is `blocked` | # | 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 | diff --git a/scripts/benchmark.ts b/scripts/benchmark.ts index e46ab1a..9685a7b 100644 --- a/scripts/benchmark.ts +++ b/scripts/benchmark.ts @@ -465,6 +465,24 @@ function stateCreationSealed(): number { 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: { @@ -631,6 +649,10 @@ const suite: SuiteEntry[] = [ fn: stateCreationSealed, name: 'state creation (sealed) ', }, + { + fn: stateCreationHooks, + name: 'state creation (hooks) ', + }, { fn: stateReadNested, name: 'state read nested ', diff --git a/src/AGENTS.md b/src/AGENTS.md index 39f0baf..7d366f1 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -59,7 +59,6 @@ Effect tracking data stored directly on the function object (O(1) property acces | `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 @@ -68,7 +67,7 @@ Effect tracking data stored directly on the function object (O(1) property acces | `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 @@ -98,8 +97,8 @@ Handler helpers: | `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) | @@ -210,9 +209,8 @@ addPendingEffect(subscriber) | 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 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 @@ -51,8 +51,7 @@ The core `index.ts` imports `composeHook` from `./hooks/compose.ts`. Published a ## 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 diff --git a/src/index.ts b/src/index.ts index dfdcc7b..e103daa 100644 --- a/src/index.ts +++ b/src/index.ts @@ -109,7 +109,7 @@ 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 @@ -132,10 +132,10 @@ class DirectArrayContext { } } -// 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 @@ -743,7 +743,7 @@ function createGetHandler( } } -// 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, @@ -751,7 +751,7 @@ function hooksSlotFor( 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, @@ -911,12 +911,6 @@ function performWrite( 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, @@ -941,9 +935,8 @@ function defineProxyProperties(target: ProxyTarget, proxy: unknown, hooks: Hooks definePropertyOnTarget(target, proxy, hooks) return true } - storeHooksForFrozenTarget(target, hooks) } catch { - storeHooksForFrozenTarget(target, hooks) + // a Proxy target that rejects symbol defines } return false } diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 98de6ab..4dae5d6 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -92,7 +92,7 @@ Test hooks instrumentation for each primitive. Each file verifies backward compa | 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 | 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 @@ -425,7 +425,7 @@ describe('Array push and pop', { concurrency: true, timeout: 1000 }, (): void => }) 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[])) 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 @@ -8,8 +8,8 @@ import { batch, derive, effect, state } from '../src/index.ts' * * 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: 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 @@ -368,4 +368,19 @@ describe('State Hooks', { 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) + }) })