ReviewOS

stacks/stx

Two independent reactivity systems: state made with reactive() can never update a template

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

Summary

stx ships two independent reactivity systems that do not observe each other, and nothing in the type system, the export surface or the docs indicates which one you are using. State created with one and read inside an effect from the other silently never updates — no error, no warning, just a value frozen at first render.

The two systems

signals-api.tsreactivity.ts
createstate() :187, derived() :283ref(), reactive() :293
observeeffect() :385watch() :485, watchEffect() :664
tracking varits ownprivate currentEffect :119
reaches the client runtimeyes — signals.ts:11 imports state, derived, effect, batch, onMount, onDestroy… from itno

reactive() subscribes whatever is in reactivity.ts's module-private currentEffect (set only by that module's own watchEffect, :392). effect() from signals-api never sets it. Neither module imports the other — both have zero imports — so there is no bridge.

Why this is not theoretical

Both are re-exported from the package entry, so import { reactive, effect } from 'stx' gives you one of each, and the combination is inert:

import { effect, reactive } from 'stx'

const s = reactive({ n: 0 })
effect(() => console.log(s.n))  // logs 0
s.n = 1                          // logs nothing. ever.

The client runtime only ships signals-api's effect (signals.ts:11), so every template binding is on the signals-api side. Anything an app author builds with reactive() — the more discoverable, Vue-shaped API — cannot drive a template.

Found while fixing #1856: defineForm returned plain objects, and the obvious repair was to wrap them in reactive(). That would have looked correct in review, passed a watchEffect-based test, and still never updated a single template. The fix had to use state() from signals-api instead, and the reasoning is now a 12-line comment in forms-validation.ts because nothing else records it.

Ask

  1. Decide which system is canonical for template-facing state. Given the runtime already ships signals-api, that is presumably it.
  2. Make the other one either bridge or refuse:
    • bridge — have reactive()/ref() track the same effect context as signals-api's effect(), so the two compose; or
    • fence — keep them separate but make reactive() warn in dev when read inside a foreign effect, and say so in its docblock.
  3. Whichever is chosen, document it at both call sites. Today reactive()'s docblock (reactivity.ts:283-292) shows a plain state.count++ example with no hint that a template will not see it.
  4. Consider not exporting both under undifferentiated names from the same entry — the current surface actively invites the broken combination.

Refs #1856

10 days ago

I dug into this and the headline holds — but the mechanism described here is not the one that breaks templates, and acting on the diagnosis as written would produce a fix that changes nothing a user can see. Everything below was executed, not read; scripts and verbatim output included.

There are three reactive systems, not two

createsobservesreaches the browser?
reactivity.tsref(), reactive() — a real Proxywatch, watchEffectno
signals-api.tsstate(), derived()effect()no
the runtime string in signals.tsits own state/derived/ref/reactiveits own effectyes — this is what drives templates
window.stx.state  === signals-api state()?  false
window.stx.effect === signals-api effect()? false

So this line in the issue is wrong:

reaches the client runtime — yes — signals.ts:11 imports state, derived, effect, … from it

That import is the module/server side. signals.ts also generates a client runtime as a template literal containing its own let activeEffect and its own state/effect. signals-api.ts's own docblock already says this ("the runtime template literal in signals.ts … owns its own subscribers", "signals created in one impl are invisible to the other").

The consequence matters: state() from signals-api does not drive a template either. It only appears to, because the bundler rewrites bare identifiers in a <script client> to window.stx.*. Outside that rewrite — a composable, a store, a plain module — state() is the non-template system.

The module-side split is real (executed)

effect()watchEffect()
reactive() / ref()INERT [0]REACTIVE [0,1]
state()REACTIVE [0,1]INERT [0]

Three of the four cross-pairings are silently dead, and all eight names come from the same entry with nothing to tell them apart. That part of the issue is confirmed exactly as written.

The bug that actually breaks templates

packages/stx/src/signals.ts:5021, inside the generated runtime:

// Vue-compat aliases
var ref = state;
var reactive = state;   // <- not a proxy. an alias for state.
var computed = derived;
var watch = $watch;
var watchEffect = function(fn) { return effect(fn); };

and again at :6464-6465 (window.ref = state; window.reactive = state;).

ref survives this, because a signal carries a .value accessor — stx.ref(0) + stx.effect is genuinely REACTIVE [0,1]. reactive does not:

RUNTIME   reactive({n:0})   typeof function   .n = undefined   JSON.stringify = undefined
          Object.keys → ["set","update","subscribe","_isSignal","_effects","_setCount","_stxSignalId"]

MODULE    reactive({n:0})   typeof object     .n = 0           Object.keys → ["n"]

o.n = 5 writes a plain expando property onto a function object. No notification, no error, and renders empty forever.

And 'reactive' is in STX_RUNTIME_GLOBALS (runtime-globals.ts:48), so a <script client> writing reactive({ count: 0 }) resolves to the broken alias, not to reactivity.ts's proxy.

So the same identifier means two different types depending on which file you write it in. That, not a missing bridge, is what should lead this issue. It also means the two remedies proposed here miss: bridging reactivity.ts to signals-api would not touch the alias, and fencing reactive() would warn about the wrong system.

One correction to my own framing above: I verified ref and reactive. I did not verify computed = derived, watch = $watch or watchEffect in the runtime, so treat those three as unaudited rather than fine.

On the proposed fixes

A bridge is not achievable as described. The template-facing implementation lives inside a generated template-literal string; no TypeScript module can import it or share a tracking variable with it. Any real bridge means restructuring how the runtime is emitted, which is a much larger change than the issue implies.

Suggested order instead:

  1. Fix var reactive = state. Either give the runtime a real proxy, or delete the alias so reactive() raises a loud ReferenceError instead of yielding silent undefined. Deleting is a one-line change and strictly better than today.
  2. Stop exporting reactive/ref/watch/watchEffect from the package entry beside state/derived/effect, or the inert combinations stay one autocomplete away. This is ask 4 of the issue and I think it is the highest-value item after (1).
  3. Keep ask 3 (document at both call sites) — still correct.

Unrelated bug found while doing this, worth its own issue

effect() calls its callback's return value as a cleanup with no type check, in both implementations — signals-api.ts:395 and the runtime at signals.ts:730:

first run ok, textContent = a
CRASH on second run: cleanup is not a function. (In 'cleanup()', 'cleanup' is "a")

Trigger is effect(() => el.textContent = name()). A concise arrow returns the assigned value, if (cleanup) is truthy for any non-empty string, and it gets invoked on the next run. Braced bodies are unaffected, which is presumably why it has survived. It is the same missing typeof cleanup === 'function' guard that #1857 just fixed for mount callbacks. Happy to file and fix separately.

Caveat on all of the above: my six independent verification agents died on a session limit, so this is one pass of direct execution rather than corroborated by separate probes. The scripts are small and I can attach them if useful.

10 days ago

There's a third wrinkle that makes this sharper, and it explains how two people auditing the same package can reach opposite conclusions about what reactive() is.

reactive names two different functions depending on how you reach it.

Reached viaResolves toReturnsRead as
import { reactive } from 'stx'reactivity.ts:293 — the Vue-shaped proxy (index.ts:77 re-exports the module wholesale)Tobj.n
the client-runtime globalsignals.ts:5031 var reactive = state / :6473 window.reactive = statea signalsig()

So it isn't only that the two systems don't observe each other — the same identifier has a different call convention in each context. Code written against one is not merely inert when moved to the other; it's a type error that fails silently, because obj.n on a signal function is undefined rather than a throw.

That matters most for the migration path this issue implies. "Use state() instead of reactive()" is right for a <script client> block, where the runtime global already is state — so the advice is a no-op there and the code was never broken. But the same advice applied to a module that imports from 'stx' is a real behavioural change, because that reactive is the proxy.

I hit the other end of this while filing #1883 (about x-model's write path). My verification concluded "reactive is a bare alias of state, not a Vue-style deep proxy" — true of the runtime global at signals.ts:5031, and wrong about the import. Your repro is correct for the import path. Neither of us was looking at the wrong file; there are just two reactives.

Adding to your ask #4 — "consider not exporting both under undifferentiated names" — I'd suggest the runtime global is the more urgent half. An import can at least be traced to a module; a global cannot, and a <script client> author has no way to know which reactive they got. If the alias at signals.ts:5031 exists for Vue familiarity, it's actively working against that: it hands you something Vue-shaped in name that behaves nothing like Vue's.

Refs #1883

10 days ago

The root cause from my audit above is fixed in 21c2131 — reactive() in the client runtime is now a real proxy instead of var reactive = state.

Every measurement from that audit flips:

beforeafter
typeof reactive({n:0})functionobject
.nundefined0
Object.keys(...)["set","update","subscribe","_isSignal",…]["n","label"]
JSON.stringify(...)undefined{"n":0,"label":"hi"}
reactive + effectINERTREACTIVE

All five runtime pairings (state/ref/reactive × effect/watchEffect) are now live, so the 2×2 dead-zone inside the runtime is gone.

Backed by one signal per property, created on first touch, so an effect that reads a does not re-run when b changes. Nested objects are wrapped on read and cached in a WeakMap so proxy identity is stable — a fresh proxy per read would break === and defeat keyed list reuse in x-for.

Boundaries I chose deliberately, each with a test:

  • A primitive is returned unchanged, so reactive(0) does not silently become an object.
  • Symbols and inherited members pass straight through. Tracking Symbol.iterator or toJSON would make every effect depend on the whole object and mint a signal per method lookup.
  • ref is untouched and stays state — a signal already carries a .value accessor, so the Vue shape always worked there. Only reactive was wrong.

This is a behaviour change to a public name and should be reviewed as one. Code that treated reactive(x) as a signal — calling it, or .set() — breaks. That usage could only ever have been a synonym for state(), since as a Vue-style reactive object it did not function at all, so I judged the blast radius near zero. Say the word if you would rather have had the deletion.

Still open here

  1. The MODULE side is unchanged. reactivity.ts and signals-api.ts remain separate systems, and import { reactive, effect } from 'stx' is still the inert combination measured in my audit. That is asks 1 and 4 of this issue — decide which is canonical, and stop exporting both under undifferentiated names. A bridge is still not achievable as described, for the reason given above.
  2. computed, watch and watchEffect in the runtime are unaudited. I verified ref and reactive only. Given one of the five "Vue-compat aliases" was silently wrong, the other three deserve the same check before anyone trusts them.
  3. Ask 3 — documenting it at both call sites — is not done.
10 days ago

The module side is fixed — ef1fbba1d4

Ask 1, as it applies to the two TypeScript modules. All six pairings are live:

beforeafter
reactive() + effect()INERTREACTIVE
ref() + effect()INERTREACTIVE
state() + watchEffect()INERTREACTIVE
reactive()/ref() + watchEffect()REACTIVEREACTIVE
state() + effect()REACTIVEREACTIVE

The whole cause was a variable name. reactivity.ts had currentEffect, signals-api.ts had activeEffect — same shape, same purpose, two modules, one package entry. A read recorded itself against whichever one happened to be set, so which system a value belonged to was decided by which module declared the variable rather than by anything about the value. reactive-tracking.ts now owns it for both: a third module, because either importing the other is a cycle. It stays deliberately small — how a value is stored, when it counts as changed, and what it exposes all stay where they were.

Worth noting against the earlier reading in this thread that a bridge was not achievable: that is true of the runtime, and only of the runtime. The runtime is a template literal, so no module can import it. Two ordinary TypeScript modules can share a variable, and that turned out to be the entire fix.

ref() + effect() was inert too, which this thread had not recorded — the ref clearance earlier was about the runtime alias.

batch() had the same seam, on the write side

It lived in signals-api and deferred only signals-api's notifications, so a ref written inside a batch() notified immediately and the effect ran twice — once eagerly, once at the flush. ref and reactive now notify through the shared queue, and a nested batch flushes once at the outermost exit rather than at the inner one.

Ask 2 — the three unaudited runtime names

Checked, and none is the reactive = state shape:

computed     bare-alias=[derived]   window.computed=[derived]
watch        bare-alias=[$watch]    window.watch=[$watch]
watchEffect  own function           window.watchEffect=[function]

computed is derived, which is a real computed; watch is $watch; watchEffect is its own implementation. So the audit that flagged them can be closed — the one genuinely wrong alias was reactive, already fixed in 21c2131f89.

One correction to my own working notes

I first measured watch() on a state() as broken and was about to report it. It is not: watch defaults to flush: 'post', and I read the result before the flush. A Signal is callable, so it takes watch's function-source branch and works. Pinned as a test so the alternative reading — source.value on a Signal, which is undefined and compares equal to itself forever — cannot creep back.

17 tests. Sabotaging the shared tracker fails 6 of them, un-batching ref writes fails 2, and clearing rather than restoring the subscriber on a nested effect fails 1.

Still open here: ask 4 — whether ref/reactive/watch/watchEffect should keep sitting beside state/derived/effect under undifferentiated names. That is now a naming and documentation decision rather than a correctness one, since the combinations all work; it needs your call on which set is canonical. Ask 3 (document at both call sites) follows from whatever you decide.

10 days ago

Asks 3 and 4 are in — b7195c7644. Closing.

Ask 4 — keeping both families, and saying which is which. The argument for removing the Vue-shaped names was that they trapped people in dead pairings: ref() with effect() did nothing, and so did three other combinations. ef1fbba1d4 fixed that with one shared tracker, so what remains is a familiarity choice rather than a correctness one — and breaking working imports for a familiarity choice is a bad trade.

state / derived / effect is canonical. It is what the client runtime puts on window.stx, what a <script client> block sees as bare identifiers, and what stx's own composables and stores are written against — the only set that reads the same in a module, in a component and in a template. ref / reactive / computed / watch / watchEffect stay exported and supported, for code ported from Vue.

Ask 3 — documented at both call sites, in reactivity.ts and signals-api.ts, each naming the canonical set and the one trap that survives the fix: ref() returns a .value object and state() returns a callable signal, so the two are not interchangeable at the call site even though both are now reactive to everything.


The whole issue

ask
1 — one system, or make them observe each otheref1fbba1d4 — one shared tracker; all six pairings live
2 — audit computed / watch / watchEffect in the runtimedone: computed is derived, watch is $watch, watchEffect is its own function. None is the wrong-shape alias reactive = state had been
3 — document at both call sitesb7195c7644
4 — undifferentiated namesb7195c7644 — kept, canonical set named

Plus the root cause fixed earlier in 21c2131f89 (reactive() in the runtime is a real proxy).

Two corrections to the record made along the way, both mine: ref() + effect() was inert too, which this thread had not recorded — the earlier ref clearance was about the runtime alias, not the module. And "a bridge is not achievable as described" holds for the runtime only, because it is a template literal no module can import; two ordinary TypeScript modules can share a variable, and that turned out to be the entire fix.

batch() had the same seam on the write side and is fixed with it: it deferred only signals-api's notifications, so a ref written inside a batch notified immediately and the effect ran twice.

Sign in to comment on this issue.