ReviewOS

stacks/stx

Scriptless @click is dropped entirely — no handler, no runtime, no error, on full load as well as SPA nav

#1834
Closed glennmichael123 opened this 10 days ago ·
10 days ago

Summary

A @click (or any @event=) on a page with no client script is compiled away and never wired up. The element renders with an id and nothing else. There is no console error and no warning — the button simply does nothing, forever.

This is not SPA-specific. It happens on first full page load.

Repro

<main>
  <button @click="useStore('cart').add(1)">Add</button>
</main>

Through processDirectives:

<main><button id="__stx_evt_0">Add</button></main>

markers: {"setup":false,"scope":false,"reactive":false,"winstx":false,"auto":false,"dataStx":false,"runtime":false}

The handler is gone, no data-stx-events script is emitted anywhere in the page, and no runtime is shipped.

Cause

prepareDirectiveContext sets context.__stx_sfc_mode = true unconditionally (process.ts:1181), so processEventDirectives takes the SFC branch (events.ts:608) and parks the binding on context.__stx_event_bindings instead of emitting a handler. Those parked bindings are only re-emitted while transforming a client script (process.ts:1746-1795).

No client script ⇒ no consumer ⇒ the binding is silently discarded.

Why it matters

@click calling into a store or a global is a reasonable thing to write with no local state — it is the shape the docs encourage for store-backed actions. The failure is completely silent, which is the worst property: nothing in the page hints that the attribute was understood and then thrown away.

Suggested direction

Either emit the delegated handler script when bindings are parked but no client script exists, or refuse to park in the first place when there is no script to consume them. A dev-mode warning when __stx_event_bindings is non-empty at the end of processing would surface the whole class cheaply.

  • #1820 — the same ordering, different symptom: with a client script the page emits window.stx.mount(...) and no runtime, throwing on the first line.
  • #1827 — found while verifying that. The router-side fix there does not help here, since this is broken on full load too.
10 days ago

Fixed in 4b3d8b7.

The fix

Unconsumed bindings are now flushed through the standalone generator the non-SFC path already used. Guarded twice — only when the client-script pass did not run, and only at processing depth 1 — because a component's bindings are deliberately left for its caller to consume, and emitting them here as well would bind every handler twice.

Both forms verified by clicking, not by inspecting markup:

call form: {"called":1,"errors":[]}
ref form : {"called":1,"errors":[]}

The part worth reading

A security test was passing because of this bug.

packages/stx/test/security/security.test.ts → "should escape newlines in event handler strings" had its assertion wrapped in:

if (result.includes('__stx_execute')) {
  expect(result).not.toMatch(/\n.*line2.*'/)
}

A scriptless page emitted no binding script at all, so __stx_execute never appeared, the guard was false, and the assertion never ran. The bug was keeping its own regression test green.

The moment the script is emitted, that assertion fires — but as a false alarm: it searches the whole document for /\n.*line2.*'/, which matches the script's own formatting newline, not one inside the handler string. The escaping was correct all along (events.ts already maps a raw newline to the two-character escape).

So the test now asserts the property it names — the embedded handler string contains no raw newline and does contain the escape — and can no longer pass vacuously. Worth flagging because a conditional guard around a security assertion is a pattern that will do this again; there are a few more of that shape in the same file (if (result.includes('getElementById')), etc.).

Testing note

The new tests click the button rather than checking that a script exists. The bug produced markup that looked plausible and did nothing, which is precisely what a string assertion cannot detect — the same lesson as #1824.

Three of the six new tests fail at the parent commit. The three that pass there are the guards: no double-binding when a client script consumed the bindings, and no script manufactured for a page with no events.

Full suite 10750 pass in a clean worktree; the one failure is the pre-existing crosswind-arbitrary-values case.

Sign in to comment on this issue.