Repository navigation
fix(state): notify sealed-object subscribers from hookless write paths - #178
Merged
Merged
Conversation
The hookless set handler's out-of-effect branch and handleBatchFastPath read only the SUBSCRIBERS symbol, so writes to non-extensible targets, whose subscriber set lives in proxyCacheSubs, re-ran no effects. Both paths now fall back to proxyCacheSubs. A hasFallbackSubs flag, set when the first set is stored there, keeps the WeakMap lookup off the no-subscriber path until a non-extensible target has subscribers.
The process-wide hasFallbackSubs flag never reset, so once any sealed or frozen target had a subscriber, every hookless write to a target without subscribers paid a proxyCacheSubs miss (+10% on "state no subs"). definePropertyOnTarget now defines SUBSCRIBERS as a writable null placeholder at state() time, so an absent symbol means exactly "set lives in proxyCacheSubs"; null means "no subscribers yet". A target frozen after state() makes the placeholder non-writable, and storeSubscriberSet catches that assignment and stores in the WeakMap. Measured with two interleaved trunk-vs-change suite pairs at 10 cycles: aggregate 866.94 vs 864.48 ms and 925.17 vs 885.37 ms; "state no subs" 42.2/43.9 to 36.8/36.6 ms; "state creation" 27.1/27.6 to 36.9/36.2 ms, the cost of the extra define under GC pressure. Adds two benchmark rows with a sealed target and a test for a target frozen after state().
The eager SUBSCRIBERS placeholder from e8c7630 cost +10 ms per 100,000 state() calls against origin/trunk, and no defineProperty spelling removed it (hoisted descriptor, Reflect.defineProperty, define order); only an enumerable plain assignment matched base, which spread and Object.assign would copy. Drop the placeholder. The hookless set handler and the batch fast path now consult proxyCacheSubs when the target has no PROXY symbol, which state() defines on every extensible target, so non-extensible targets and symbol-rejecting Proxy targets need no flag. hasFallbackSubs returns, armed only when a target that carries PROXY lands in the WeakMap: sealed or frozen after state() and before its first subscription. Two interleaved base-vs-branch pairs at 10 cycles: 840.66 vs 842.10 ms and 839.85 vs 833.73 ms; "state creation" 24.0/24.3 vs 23.3/23.5 ms; "state no subs" 40.8/41.2 vs 36.7/37.1 ms. Adds a test for a target sealed after state() (direct and batched writes notify).
nerdalytics
marked this pull request as ready for review
October 19, 2026 14:25
4 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A write to a sealed reactive object made outside an effect, with no hooks installed, did not re-run the effects that read the written property. The handbook's "Frozen and sealed objects" section promises that it does. The hookless set handler and the batch fast path read only the
SUBSCRIBERSsymbol, and a non-extensible target keeps its subscriber set in theproxyCacheSubsWeakMap instead. This PR fixes both paths and lands the internal refactors from the over-engineering audit that only existed on the local trunk. On the benchmark, the branch does the same work faster than trunk by 1.3% and 2.2% in two interleaved rounds. The one row slower by more than 0.1 ms in both rounds is the sealed write, where trunk skipped the subscriber work because of the bug. With that row included, the totals differ by 0.2% and 0.7%, under the 1% noise limit.Changes Made
proxyCacheSubs, so direct and batched writes to a sealed target notify its subscribers. This is commit 63eb432.PROXYsymbol.state()defines that symbol on every extensible target, so a target without it never took symbols, and the common path needs no flag and noObject.isExtensiblecall. Three commits reach this form; the lastperf(state)commit is the final one.storeSubscriberSetarmshasFallbackSubsonly when a target that carriesPROXYlands in the WeakMap. That happens when code seals or freezes the target afterstate()and before its first subscription. Only then do writes with no subscribers pay a WeakMap miss.SUBSCRIBERSplaceholder and a later commit in this branch reverts it. The placeholder cost 10 ms per 100,000state()calls. The revert's commit message lists the fourdefinePropertyspellings measured.derive,until, effect cleanup and the batch flush. Six test commits in the same series pin the behaviour those refactors touch.Performance Impact
node --expose-gc scripts/benchmark.ts -R 10against origin/trunk, sum of the 18 medians: 840.66 vs 842.10 ms in round 1, which is 0.2% slower, and 839.85 vs 833.73 ms in round 2, which is 0.7% faster. The 1% limit is the noise floor of this benchmark, not a target; two runs of the same trunk code differed by up to 0.6% in this session.Test Coverage
tests/property-frozen-sealed.test.tsgains six cases: direct write, batched write, unchanged value, symbol-rejecting target, frozen afterstate(), and sealed afterstate(). Against origin/trunk's core, four of its 21 tests fail. On this branch all 260 tests pass.node --test tests/property-frozen-sealed.test.tswith trunk'ssrc/index.tsin place.npm run check,npm run buildandnpm testexit 0. Branch coverage onsrc/index.tsreports the same 23 pre-existing uncovered entries as trunk and no new ones.Related Issues
The local files
.do/inbox/2026-10-18-sealed-writes-skip-subscribers.mdand.do/inbox/2026-10-19-fallback-subs-flag-ceiling.mdtrack this work. They are not committed because.do/is gitignored.Checklist