ReviewOS

stacks/stx

x-model's write path doesn't unwrap signals: object paths never notify, and a store property is silently overwritten with a raw value

#1883
Closed glennmichael123 opened this 11 days ago · {{ comments.length }} {{ comments.length === 1 ? 'comment' : 'comments' }}
11 days ago

x-model's read path unwraps signals through a proxy; its write path does not. The two disagree about what the scope contains, which produces three different failures depending on what you bind to — none of which say anything.

The asymmetry

src/signals.ts, same bindModel call, ~20 lines apart:

// getValue — unwraps signals
var unwrapScope = createExpressionAutoUnwrapProxy(capturedScope, expr)
var fn = new Function('__scope__', 'with(__scope__) { return (' + expr + ') }')
return fn(unwrapScope)

// setValue — does NOT
const setValue = (val) => {
  if (capturedScope[expr] && capturedScope[expr]._isSignal) {
    capturedScope[expr].set(val)                                    // bare name: fine
  } else {
    const fn = new Function(...Object.keys(capturedScope), 'v', expr + ' = v')
    fn(...Object.values(capturedScope), val)                        // raw values, no proxy
  }
}

The else branch receives raw Object.values(capturedScope) rather than the auto-unwrap proxy getValue uses. Everything below follows from that.

Three outcomes, all silent

1. x-model="draft.title" over a plain object — writes, but nothing notices. The object reaches the generated Function by reference, so draft.title really does become the new value. But no signal is set, so nothing re-runs. A sibling :text="draft.title" stays frozen at its initial value while the input updates. The binding looks half-alive.

2. x-model="variants().bluesky" over state({}) — same. The write lands on the object inside the signal; the signal is never notified.

3. x-model="store.title" against a defineStore property — silent corruption. store.title is a signal function. The raw assignment replaces the signal with a plain string: typeof store.title goes 'function' → 'string'. Every other consumer of that store property breaks from that point on, and the catch around setValue swallows the resulting TypeErrors.

Note the failure is invisible in all three: setValue's catch only warns for non-ReferenceError/TypeError, and these all fall inside that exclusion.

Documented, but the doc understates it

docs/api/bindings.md:125 says x-model="user.name" does not work and to use a dedicated signal. That's right for case 1, but it reads like "no effect" — case 3 is not "no effect", it's destroying a store's reactivity.

Also worth flagging: reactive is a bare alias of state (signals.ts:4854, var reactive = state), not a Vue-style deep proxy. Anyone arriving from Vue will assume reactive({...}) + x-model="obj.field" works, and will hit case 1 or 3.

Why it matters

Every editable field needs its own top-level signal with a hand-written name, plus hand-written fan-out in both directions. In our app:

  • composer.stx: 9 signals (vTextBluesky … vTextBlog) plus a 9-entry lookup map to recover keyed access, plus 9 duplicated markup blocks.
  • campaigns.stx:304-319: one record draft exploded into 14 signals, with an 8-line manual fan-out at :549 where Object.assign(draft, item) would do.
  • listening.stx:218-233: 6 signals and two 6-line fan-out functions — one to load the form, one to reset it.

Ask

Two things, in priority order:

  1. Make case 3 loud. A write that would overwrite a signal should throw or warn rather than corrupt it. That's a small change and it's the dangerous one.
  2. Resolve lvalue paths against signal-backed state — walk the path and clone-and-set the owning signal (or give reactive() a real write-through proxy), so x-model="draft.title" and x-model="variants[row.provider]" notify. createExpressionAutoUnwrapProxy already has a set trap; setValue just never uses it.

Prior art: Vue v-model="draft.title" on any lvalue plus reactive(); Svelte bind:value={item.text}; Angular [(ngModel)].

Related: #1799 (caret) was a different x-model defect on the write path.

Environment

@stacksjs/stx 0.2.153, Bun 1.3.1.

10 days ago

Correction to one line in the issue body, so this doesn't mislead anyone reading it alongside #1885.

I wrote that reactive is a bare alias of state (signals.ts:4854, now :5031). That is true of the client-runtime global only. The package export is a different function — reactivity.ts:293, a Vue-shaped proxy, re-exported from the entry via index.ts:77 — so import { reactive } from 'stx' gives you the proxy, not state.

Two reactives, different call conventions (obj.n vs sig()). Details and the reconciliation are in #1885.

Nothing else in this issue changes: the x-model findings were all measured against the client runtime, which is the only path a template binding takes. The three outcomes — object paths writing without notifying, and a defineStore property being overwritten with a raw value — stand as described.

10 days ago

Claiming this one — working on it now.

10 days ago

Fixed in 860a6867e1 ("fix(signals): x-model writes through a signal, never over it").

setValue now writes through createModelWriteProxy (signals.ts:450) — the same auto-unwrap path the read side already used — instead of destructuring the scope into plain parameters. The proxy's set trap calls .set on a signal-valued property, so all three reported outcomes are covered, including case 3: store.title stays a signal rather than being replaced by a raw string.

packages/stx/test/signals/model-lvalue.test.ts pins it (8 tests), and asserts the store case specifically:

expect(typeof store.title).toBe('function')
expect(store.title._isSignal).toBe(true)
expect(store.title).toBe(title)

Verified passing on current main.

Sign in to comment on this issue.