ReviewOS

stacks/stx

fix(dev): the store/composable bundle is memoised forever and their directories are not watched — every store edit needs a server restart

#1877
Closed glennmichael123 opened this 11 days ago ·
11 days ago

Summary

getStoreScript() memoises its output per storesDir and nothing ever invalidates it. clearStoreCache() exists and is exported, but it is called only from tests. The dev server also does not watch storesDir or composablesDir. The combined effect is that no edit to a store file — creating the first one, or changing any existing one — reaches the browser without restarting the dev server. Template HMR, meanwhile, is location.reload(), so every save on a stateful page throws away the state.

Impact (real app)

analyticshq documents the first half in config/ui.ts:72-73:

Caveat for the state stage: that null is memoised (store-loader.js:12+) and nothing calls clearStoreCache(), so the FIRST store file needs a dev-server restart.

In practice it is worse than the note says: the same memo holds the built bundle once stores exist, so every subsequent edit to resources/stores/session.ts also needs a restart.

The second half costs the app on every template save: resources/views/dashboard.stx is ~107 KB and holds date-range, filter-chip and drill-down state entirely in client signals. A one-character edit reloads the document and resets all of it.

Evidence in stx

The memo is permanent — packages/stx/src/store-loader.ts

:18   const _cachedStoreScripts = new Map<string, string>()
:83   const cached = _cachedStoreScripts.get(resolvedDir)
:84   if (cached !== undefined) return cached || null
…
:200  _cachedStoreScripts.set(resolvedDir, code)

The invalidator is never called — clearStoreCache() at store-loader.ts:207 and clearComposableCache() at composable-loader.ts:249. Grepping the whole workspace for callers:

packages/stx/test/signal-processing/store-runtime-globals.test.ts:33,38
packages/stx/test/composables/composable-loader.test.ts:44,48,125,145,154,166,226,237
packages/stx/dist/*.d.ts                (declarations only)

Nothing in packages/stx/src or packages/bun-plugin/src calls either one.

The directories are not watched — packages/bun-plugin/src/serve.ts:1078-1088

for (const dir of [
  componentsDir,
  layoutsDir,
  partialsDir,
  publicDir,
  'resources/assets',
  ...(options.watchDirs ?? []),
]) { … startWatcher(dir) }

storesDir and composablesDir are absent, and Stacks' dev views server (storage/framework/core/actions/src/dev/views.ts:178-200) passes no watchDirs.

HMR is a reload — packages/bun-plugin/src/serve.ts:987

es.onmessage=function(e){var m=JSON.parse(e.data);if(m.type==='reload')location.reload();else if(m.type==='css')bust()}

Two actions only: reload the document, or re-fetch stylesheets via a ?v= bust.

Prior art

  • Vite HMR + Vue/React Fast Refresh swap a module and preserve component state across the edit.
  • Pinia's acceptHMRUpdate(store, import.meta.hot) hot-swaps a store definition without losing its state — the direct analogue for what is missing here.

Ask

  1. Add storesDir and composablesDir to the dev server's watch list at serve.ts:1078-1088.
  2. Call clearStoreCache() / clearComposableCache() from those watchers before firing the HMR event, so an edit is actually rebuilt.
  3. Key the memo on file mtimes (or drop it in dev entirely) so a stale bundle cannot outlive its source even when a watcher misses an event.
  4. Ship a type:'store' HMR message that re-executes the store bundle and re-runs defineStore in place, rather than reloading the document — the Pinia acceptHMRUpdate contract.
  5. Longer term: a type:'fragment' message that swaps the changed template region instead of location.reload().
11 days ago

Fixed in 4e98218 — asks 1, 2 and 3. Asks 4 and 5 are not done; reasons below.

Ask 3 (memo keyed on sources) — done first, because it's the one that holds when the others fail. sourceSignature() fingerprints path + size + mtime; the loaders scan before the cache lookup and compare. Size is in there because mtime resolution is coarse enough on some filesystems that a fast rewrite of the same length lands in the same millisecond. Statting a handful of files is nothing next to the Bun.build it guards.

Asks 1 and 2 — storesDir and the composable directories are watched (resolved under root the way the loaders resolve them, including the conventional functions/ alias the composable loader probes for), and the watcher now calls clearStoreCache() / clearComposableCache(), both exported from the package index so bun-plugin can reach them.

One extra find while in there: resolveComposablesDir memoised a negative result too, so a process that looked before any composable existed could never see the first one — the same shape as the store memo, one directory up.

Verified on a fixture:

HEAD    before any store: null | after first store: null (STALE) | after edit: STALE
fixed   before any store: null | after first store: script      | after edit: rebuilt

On asks 4 and 5. Ask 4 — the Pinia acceptHMRUpdate analogue — needs defineStore's internals inside the generated signals runtime, and that file is under concurrent edit by another workstream right now; landing a runtime change into it from here would be asking for a conflict. It's also genuinely more design than plumbing: re-running defineStore in place means deciding what happens to existing subscribers, to state that no longer has a matching key after the edit, and to derived values captured by effects. Worth its own issue rather than a rider on this one. Ask 5 you scoped as longer-term yourself, and it depends on the same fragment machinery the SPA path uses.

So the ~107KB dashboard still loses its client state on a template save. That's real and unfixed — what changed is that a store edit is now seen, where before it was invisible until restart.

Covered by packages/stx/test/store-cache-invalidation.test.ts (10 tests). Full suite: 11102 pass, 1 pre-existing unrelated failure.

11 days ago

Asks 4 and 5 are now done too — 5e00590. That closes every ask on this issue.

Ask 4 — store HMR. defineStore no longer short-circuits on the cached instance during a replacement pass, and window.stx.__hmrReplaceStores(code) seeds the new store's signals from the old values. /_stx/stores.js serves the bundle standalone so the HMR client can fetch it without a page load.

The design decision worth recording is which object survives. Patching the OLD store in place and keeping its signals is the obvious reading of "hot swap" — and it's wrong here. A setup-style store's actions close over the signals created in that same run, so the new bump() would write to a signal nothing is watching: an edit that silently does nothing, which is worse than the reload it replaces. So the new store wins, its state is seeded from the old values, and the route re-renders afterwards so components rebind. There's a test asserting the new action body runs and still writes to the signal the store exposes — that's the one that fails under the patch-in-place design.

A bundle that doesn't evaluate returns false rather than half-applying, and the client falls back to a reload.

Ask 5 — fragment swap. A page or component template edit now re-renders the current route and swaps the SPA container, using the router's existing force-refresh (which refresh()/invalidateRoute() had just exposed — that landed at a convenient moment). Every non-store signal on the page stays alive.

One deliberate limit: a layout edit still does a full reload. The swap replaces the container's contents, so chrome rendered outside it would stay stale and the page would look updated while being half-old. That's a correctness call, not an oversight — silent staleness at HTTP 200 is the worse failure.

A bug the end-to-end test caught that a unit test would not have: fs.watch reports filenames relative to the watched directory, and my first version resolved them against cwd — which put every path outside storesDir and classified every store edit as a plain reload. A unit test over the predicate would have been handed the already-correct absolute path and passed happily. The test drives a real serve() subprocess and reads the actual SSE stream:

store edit   -> store
page edit    -> fragment
layout edit  -> reload
/_stx/stores.js -> 200, contains the store, Cache-Control: no-store

So the dashboard case from the issue: a template edit no longer resets date-range/filter/drill-down state, and a store edit is applied in place. Covered by packages/stx/test/signals/store-hmr.test.ts (9 tests, verified non-vacuous — disabling the seeding fails 4 of them) and packages/bun-plugin/test/serve-hmr-granularity.test.ts (5 tests).

Full suite: 11194 pass, 1 pre-existing unrelated failure. Runtime guards (runtime-parses, runtime-no-console-log, runtime-copies-agree, dual-impl-parity) all green.

Sign in to comment on this issue.