Skip to content

fix(createRegistry): skip values/keys/entries cache when reactive#209

Merged
johnleider merged 3 commits into
masterfrom
fix/registry-reactive-values-cache
Apr 23, 2026
Merged

fix(createRegistry): skip values/keys/entries cache when reactive#209
johnleider merged 3 commits into
masterfrom
fix/registry-reactive-values-cache

Conversation

@johnleider
Copy link
Copy Markdown
Member

@johnleider johnleider commented Apr 23, 2026

Summary

Skip the keys() / values() / entries() cache in createRegistry when the registry is created with reactive: true. The cache was silently dropping Vue's iteration dependency on warm re-runs, so a computed iterating a reactive registry would lose its Map-iteration dep after any re-run that hit the cache — subsequent mutations then failed to propagate.

Background

Alternative fix for the bug in #208. That PR works around the symptom by mutating the existing ticket in `upsert` via `Object.assign` so its `shallowReactive` proxy fires — which happens to land on a different live dep path (per-ticket `.value`). The underlying iteration-dep hole stays open for any other consumer.

This PR fixes the hole at the source:

  • `reactive: true` now does what its JSDoc promises — iteration reactivity included.
  • `upsert`'s existing replace-the-object behavior is fine; no change needed there.
  • Every `reactive: true` consumer (`useNotifications`, `createInput`, `createForm`, `createBreadcrumbs`) benefits from the same fix.

Why the cache broke dep tracking

function values (): readonly E[] {
  const cached = cache.get('values')
  if (!isUndefined(cached)) return cached as readonly E[]  // no reactive read
  const values = Array.from(collection.values())           // reactive read
  cache.set('values', values)
  return values
}

Vue's computed re-tracks deps on every run. When a computed iterated `values()`:

  • Cold cache → `Array.from` on the `shallowReactive` Map forms an iteration dep.
  • Warm cache → early return, no iteration, no dep.

`select()` doesn't `invalidate()`, so any re-run triggered by a ref change after the initial render dropped the iteration dep. Subsequent `upsert`s then had nothing listening.

Reactive consumers tend to have small collections (UI state), so skipping the cache in that mode has negligible cost. Non-reactive registries keep the cache unchanged.

Follow-ups (separate PRs)

  • `useTheme` — enable `reactive: true` internally so consumers don't have to opt in.
  • `useLocale` — same.
  • `useFeatures` — same; `variation()` currently reads through a non-reactive registry.

@johnleider johnleider self-assigned this Apr 23, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 23, 2026

Open in StackBlitz

commit: ca8e4f3

@johnleider johnleider added the E: createRegistry Composable label Apr 23, 2026
@johnleider johnleider added this to the v0.2.x milestone Apr 23, 2026
When `reactive: true`, `values()`/`keys()`/`entries()` now iterate the
underlying collection on every call instead of serving a cached array.
The cache was silently dropping Vue's iteration dependency on warm
re-runs, so a computed that iterated a reactive registry would lose its
Map-iteration dep after any re-run that hit the cache — subsequent
mutations then failed to propagate.

Refs #208
@johnleider johnleider force-pushed the fix/registry-reactive-values-cache branch from 6b6e156 to aa339f7 Compare April 23, 2026 15:12
…-run

Three regression tests for values()/keys()/entries() that would have
caught #208. Each wires a computed that first runs through a
non-iteration dep (ref bump), then mutates the registry and asserts
the computed picks up the change.

Verified to fail without the cache skip in the reactive path.
…e fix

The tip on create-registry.md steered users away from `reactive: true`
toward `useProxyRegistry` because `values()` caching broke iteration
deps. That bug is fixed (see PR #209) — rewrite the tip to recommend
`reactive: true` for template/computed iteration and position
`useProxyRegistry` as the tool for event-driven snapshots, deep
tracking, or cases where wrapping tickets isn't desired.

Qualify the \"What's NOT Reactive\" table in the reactivity guide with
a pointer to the reactive-option pattern. Simplify Pattern 1 so the
example calls the public `registry.values()` / `registry.size` rather
than poking at `registry.collection` directly, now that those are
reactive too.

Refs #208 #209
@johnleider johnleider merged commit 2b7cf9d into master Apr 23, 2026
16 of 17 checks passed
@johnleider johnleider deleted the fix/registry-reactive-values-cache branch April 23, 2026 15:44
johnleider added a commit that referenced this pull request Apr 23, 2026
- `.claude/rules/composables.md`: new section "Plugins and Reactive
  Defaults" covering the primitive/plugin tiers, when to pass
  `reactive: true` to an internal registry, the two-registry
  architecture, and how `reactive: true` and `useProxyRegistry`
  complement each other.
- `PHILOSOPHY.md §4.4`: rewrite from "the reactive: true footgun" to
  "Registry reactivity". The footgun described (values() cache dropping
  Vue dep tracking) was closed in #209 — that section was actively
  teaching the wrong mental model through #210, #211, and #212. New
  text positions `reactive: true` and `useProxyRegistry` as two valid
  complementary options.
- `PHILOSOPHY.md §6.7`: revise the closing warning that said "do not
  substitute reactive: true on the registry itself" — same reason.

Refs #208 #209 #210 #211 #212
johnleider added a commit that referenced this pull request Apr 23, 2026
#213)

- `.claude/rules/composables.md`: new section "Plugins and Reactive
  Defaults" covering the primitive/plugin tiers, when to pass
  `reactive: true` to an internal registry, the two-registry
  architecture, and how `reactive: true` and `useProxyRegistry`
  complement each other.
- `PHILOSOPHY.md §4.4`: rewrite from "the reactive: true footgun" to
  "Registry reactivity". The footgun described (values() cache dropping
  Vue dep tracking) was closed in #209 — that section was actively
  teaching the wrong mental model through #210, #211, and #212. New
  text positions `reactive: true` and `useProxyRegistry` as two valid
  complementary options.
- `PHILOSOPHY.md §6.7`: revise the closing warning that said "do not
  substitute reactive: true on the registry itself" — same reason.

Refs #208 #209 #210 #211 #212
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant