Version 1.4.0#67
Merged
Merged
Conversation
Two coordinated fixes to effect scheduling in flush(): 1. Error containment: a throwing effect no longer skips sibling effects. Errors are collected per effect, the queue is fully drained, then a single error is rethrown as-is and multiple errors are wrapped in an AggregateError. FLAG_RUNNING is cleared in runEffect's finally, so a throwing effect recovers instead of causing a spurious CircularDependencyError on its next refresh. 2. Bounded convergence: an effect that writes a signal it depends on re-runs until the graph settles, so its last run always observes the final signal values (previously even a converging clamp effect ended one run stale). flush() drains queuedEffects in passes over snapshots, capped at 1000 passes; a graph that never settles (unconditional self-write, mutual effect writes — the latter previously looped until heap exhaustion) throws the new EffectConvergenceError. propagate() preserves FLAG_RUNNING on effects and flush() skips mid-run effects, removing a re-entrant runEffect hazard during creation-time writes; createEffect converges via the new internal scheduleEffect(). Docs updated: error-classes and non-obvious-behaviors references, debug workflow, codedocs error table, ARCHITECTURE.md flush description and Key Decisions, createEffect JSDoc, CHANGELOG. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Executable plans for four remaining defects found during the code audit: List mutation tracking leaks, missing CI workflow, package exports map, and Store proxy write guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugfix/flush error containment
List.set, List.sort, List.splice, List.update, Store.update, and createCollection's onChanges change branch read child item signals without untrack(). Calling them inside an effect leaked dependency edges into the caller, causing over-broad re-runs (persistent) or a spurious one-time re-run during setup (transient). All now wrap their internal reads in untrack(), matching the existing pattern in Store.set and List.replace.
fix(list): untrack internal reads in mutation methods
…neProperty Direct property writes on a Store proxy (store.name = 'Bob', delete store.name, Object.defineProperty, Object.assign) silently corrupted the store — the raw value shadowed the child State signal, causing store.name and store.get() to diverge. Add set, deleteProperty, and defineProperty traps that throw the new InvalidStoreMutationError, a TypeError subclass with a message naming the correct reactive alternative (store.key.set / store.set / store.add / store.remove). Routing to the child signal's .set() was considered and rejected: Store<T> types properties as signals (State<string>, not string) to preserve reactivity through destructuring, so proxy assignment is a compile error for typed stores — routing would be a runtime-only feature usable only behind 'as any'. See ADR-0017. - src/errors.ts: InvalidStoreMutationError class - src/nodes/store.ts: three proxy traps, import, JSDoc - index.ts: export InvalidStoreMutationError - test/store.test.ts: 6 new proxy write guard tests - adr/0017-store-proxy-rejects-direct-writes.md: ADR with full rationale - non-obvious-behaviors.md: store_proxy_rejects_direct_writes and store_method_names_shadow_data_keys entries
Bugfix/store proxy write guard
The repo uses single quotes, no semicolons, and no arrow-paren wrappers, but biome.json configured double quotes and relied on defaults that fought the existing style, producing ~53 format disagreements. - biome.json: quoteStyle single, semicolons asNeeded, arrowParentheses asNeeded; fix $schema URL to match installed Biome 2.4.6 - Apply biome check --write across 27 files (cosmetic only: line collapsing, quote/semicolon normalization; no semantic changes) - package.json: upgrade check/lint scripts from biome lint to biome check so CI can enforce formatting + import sorting alongside lint bun run check and bun run build both pass; bundle output byte-identical.
The repo had no CI; the only workflow (npm-publish.yml) published to npm
on GitHub release without running any tests, so a broken branch could
ship a release with provenance attached.
- ci.yml: two jobs on push/PR to main and next
- test (required): typecheck, biome check, unit tests, bundle-size
regression, build
- performance (continue-on-error): timing regression, informational
only since shared runners are too noisy for assertions
- npm-publish.yml: run bun run check (typecheck + biome check + tests +
bundle regression) before Build package
Both jobs use frozen-lockfile installs so PRs can't silently change
dependencies. After merge, mark the test job as a required status check
in GitHub branch protection for main and next.
Feature/ci workflow
…er-dep metadata Rewrite distribution metadata so the package resolves correctly and tree-shakes in all supported toolchains: - Add explicit `exports` map (`.`: types -> bun -> default, plus `./package.json`); the `bun` condition lets Bun consumers resolve TS source directly, `default` serves the bundled index.js - Remove `"module": "index.ts"` — it pointed at raw TypeScript, breaking webpack and older Rollup configs - Publish the unminified ESM bundle as index.js (~50 KB readable) instead of the minified artifact; consumers' bundlers minify anyway - Make TypeScript an optional peer dependency (`peerDependenciesMeta`) so JS-only consumers don't get an unresolvable-peer warning - Replace fragile `.npmignore` negation patterns with a `files` allowlist - Remove unused index.dev.js artifact Note: `sideEffects: false` from the original plan was dropped because it breaks `bun build` — Bun tree-shakes the barrel re-exports away during the package's own build, producing a broken 827-byte output. Tree-shaking for consumers is preserved because the published index.js is a single inlined ESM file (verified: bundleCoreGzipped stays at 2476B). This is a minor (not patch) version bump: the exports map intentionally blocks deep imports into package internals.
Feature/package exports
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.
Changed
package.json): The published entry point is now the unminified ESM bundle (index.js) instead of a minified artifact — consumers' bundlers minify anyway, and readable source improves debugging and bug reports. An explicitexportsmap (.:types→bun→default) replaces the baremain/modulepair: the"bun"condition resolves TypeScript source directly for Bun consumers, while"default"serves the bundledindex.jsto all other toolchains. The"module": "index.ts"field is removed — it pointed at raw TypeScript, which broke webpack and older Rollup configs. TypeScript is now an optional peer dependency (peerDependenciesMeta), so JS-only consumers no longer get an unresolvable-peer warning. Afilesallowlist replaces the fragile.npmignorenegation patterns; the unusedindex.dev.jsartifact is removed. Migration: This is a minor version bump. Theexportsmap blocks deep imports into package internals — code relying on these must switch to named imports from the package root. If your toolchain read themodulefield to consume TypeScript source, switch to the"bun"exports condition or import the bundledindex.js.Added
EffectConvergenceError: New error class (exported from the package root), thrown when queued effects keep re-triggering each other without settling within 1000 flush passes. Typical triggers: an effect that unconditionally writes a signal it reads (createEffect(() => count.set(count.get() + 1))), or two effects that write each other's dependencies. The error surfaces synchronously from the call that triggered the runaway; other queued effects still run before it is thrown.InvalidStoreMutationError: New error class (exported from the package root), aTypeErrorsubclass thrown when aStoreproperty is assigned, deleted, or defined directly via the proxy (store.name = 'Bob',delete store.name,Object.defineProperty(store, …),Object.assign(store, …)). The message names the correct reactive alternative (store.key.set(value),store.set(next),store.add(key, value), orstore.remove(key)). See ADR-0017.Fixed
Storeproxy silently corrupted the store (src/nodes/store.ts): Previously, the proxy defined noset,deleteProperty, ordefinePropertytraps — sostore.name = 'Bob'wrote the raw value onto the proxy target, shadowing the childStatesignal. From then onstore.namereturned the raw value whilestore.get()returned the reactive value, diverging silently. Now the three traps throwInvalidStoreMutationError, leavingstore.get()and the child signal intact. Migration: code that assigned through the proxy must use the reactive API (store.key.set(),store.set(),store.add(),store.remove()). This is a minor version bump — the throw replaces previously-undetected silent corruption. See ADR-0017.src/graph.ts): Previously, an exception from one effect propagated out offlush()immediately — effects queued after it were silently skipped, and the throwing effect was left withFLAG_RUNNINGset, causing its nextrefresh()to throw a spuriousCircularDependencyError. Nowflush()catches per-effect errors, drains the entire queue, and rethrows: a single error as-is (identity preserved), multiple errors wrapped inAggregateError.runEffect()clearsFLAG_RUNNINGin itsfinally, so a previously-throwing effect re-runs normally on the next update.src/graph.ts,src/nodes/effect.ts): Previously,runEffect()overwrote the node's flags withFLAG_CLEANafter running, clobbering the dirty re-mark set by the effect's own write — so a converging clamp effect (if (v > 10) s.set(10)) ended one run stale, having rendered the pre-clamp value. Nowflush()drainsqueuedEffectsin passes over snapshots (effects re-queued during a pass run in the next pass),propagate()preservesFLAG_RUNNINGon effects, andrunEffect()preservesFLAG_DIRTY/FLAG_CHECKfrom the effect's own writes — a self-writing effect re-runs until the graph settles and its last run observes the final signal values. Creation-time self-writes converge via a new internalscheduleEffect(), which also removes a re-entrantrunEffecthazard.src/graph.ts): Previously, two effects writing each other's dependencies re-queued each other forever inside oneflush(), growing the queue until heap exhaustion. Now the flush-pass cap (1000) converts this into anEffectConvergenceErrorwhile sibling effects still run.ListandStoremutation methods leaked dependency edges into the caller's effect (src/nodes/list.ts,src/nodes/store.ts,src/nodes/collection.ts):List.set(),List.sort(),List.splice(),List.update(),Store.update(), andcreateCollection'sonChangeschange branch read child item signals withoutuntrack(). Calling any of them inside an effect silently subscribed that effect to every item signal touched — causing over-broad re-runs on unrelated item mutations (persistent leak) or a spurious one-time re-run during setup (transient leak). These methods now wrap reads inuntrack(), matching the pattern already used byStore.set()andList.replace(). The public read APIs (get(),at(),byKey(),keys(),length, iterator) remain deliberately tracking per ADR-0015.