ReviewOS

stacks/stx

feat(typecheck): the runtime globals are declared `any`, so every client-side template expression is unchecked

#1889
Closed glennmichael123 opened this 14 days ago · 1 comment
14 days ago

Summary

stx typecheck now checks {⁠{ }} and directive expressions against the enclosing scope (#1852). Where the server declares the data, those get real types — @foreach(rows as row) makes {⁠{ row.total_vists }} an error against a typed Row.

Where the client declares it, they get nothing, because every runtime global is declared any:

// packages/stx/src/stx-virtual-ts.ts — runtimeGlobalDeclarations()
declare var state: any
declare var derived: any
declare var useFetch: any
// …66 more

So const count = state(0) gives count: any, and every template expression reading it is unchecked. {⁠{ count + 1 }}, {⁠{ user.nmae }} where user is a signal, :if="cart.totl" — all compile.

This is the same class as the server-side gap #1852 was filed for, one layer in: the annotation exists for a human and is verified by nothing.

Why it wasn't done in #1852

Two reasons, and the second is the interesting one.

1. stx.d.ts types these richly already, but including it drags in the built declarations — and dist/composables.d.ts has been syntactically invalid at least once (navigate: (path: string)) => unknown;). tsc aborts on a syntax error in a lib file — skipLibCheck only suppresses semantic ones — so the checker reported nothing at all about the user's code. Generating from STX_RUNTIME_GLOBALS keeps the tool working regardless of the state of the build output. Any fix here has to keep that property.

2. Templates auto-unwrap signals, so the same name has two types depending on where it appears:

<div :if="flag">…</div>              <!-- reads the VALUE  -->
<button @click="flag.set(true)">     <!-- reads the SIGNAL -->

A single declared type cannot satisfy both. Options, roughly in order of appeal:

  • Declare template-scope bindings as T & Unwrap<T> — an intersection that permits both the value operations and .set(). Cheap, no per-attribute logic, but produces odd types in hover.
  • Type per attribute kind: value type for :if / :show / x-text / {⁠{ }}, signal type for @-handlers. More correct, needs the extractor to thread the attribute through to the declaration.
  • Type the runtime globals precisely and leave the unwrap unmodelled, accepting false positives on the @-handler side.

The measurement bar from #1852 applies: the corpus run went 400 → 1 diagnostic over 2072 expressions, and the one survivor is a genuine bug. Any change here should be re-measured the same way (packages/stx/test/typecheck/stx-virtual-ts.test.ts documents the corpus method) — a template checker that invents errors is worse than none, because a muted CI gate catches nothing.

Ask

  1. Type STX_RUNTIME_GLOBALS for the checker without depending on dist/ being valid.
  2. Pick a model for the auto-unwrap split and encode it in the extractor.
  3. Re-measure against the corpus; the gate stays at zero false positives.

Related: #1852, #1791, #1792

14 days ago

Done in d426fb19d7.

Ask 1 — real types, no dist dependency. The globals now come from the package's own stx.d.ts, which already types them properly (state<T>(initial: T): StxSignal<T>, with the _isSignal brand).

The premise in this issue was wrong, and worth correcting: I wrote that including that file "drags in the built declarations". It doesn't. stx.d.ts has no imports and no triple-slash references — it's a self-contained ambient declaration file, so nothing about dist/ is reachable from it. It's also listed in the package's files, so it resolves for an installed package too. The dist/composables.d.ts hazard was real but belongs to the types entry, not this file. The any declarations survive as a fallback for an install layout where stx.d.ts is missing, since unresolved names on working code would be worse than untyped ones.

Ask 2 — the unwrap split is modelled as an intersection. A signal is typed T & V: it satisfies value operations and carries .set, so neither {⁠{ count + 1 }} nor @click="count.set(1)" is reported. Template expressions are wrapped in a function whose parameters shadow the block's bindings with that type — a parameter is the only way to give a name a second type without redeclaring it over the author's own code. (The aliases are hoisted; an inline (count: __StxTemplateValue<typeof count>) makes the annotation reference the parameter it annotates, which TypeScript reads as circular.)

I picked the intersection over per-attribute typing deliberately. It buys a false negative — :if="flag.set" type-checks though it's meaningless — and that's the right way round: this check earns its place by catching renamed fields, and a gate that invents errors gets muted, at which point it catches nothing.

It's keyed on the _isSignal / _isDerived brands that dual-impl-parity.test.ts already pins, so plain functions and plain values are left exactly as they are.

Ask 3 — re-measured, and the bar held. Through the real checker this time (typecheckStxFiles, not a hand-rolled tsc run), over the same 173-file corpus:

expressions checked : 2065
TEMPLATE diagnostics: 1
  TS1002: 1

Same single true positive as before — the unterminated string literal inside {⁠{ }} in examples/components/buddy/terminal.stx, which renders empty. Zero false positives after adding real types.

What it now catches that it couldn't before:

dash.stx:10:12  error TS2339  Property 'nmae' does not exist on type
                'StxSignal<{ name: string; email: string; }> & { name: string; email: string; }'

…while {⁠{ count + 1 }}, {⁠{ count.toFixed(2) }}, :if="open" and @click="open.set(!open())" all still pass. count.set('nope') is now an argument-type error.

Both halves are pinned against sabotage: stub the stx.d.ts lookup and the two catch-tests fail; drop the intersection and the "reads as both" test fails.

Still open, and now the honest remaining gap: the VS Code plugin keeps the any fallback. Its buffer is a module, so a local declare var state: any shadows the ambient typed one, and a tsserver plugin can't add stx.d.ts to the user's program the way the CLI controls its own. So CI is stricter than the editor for client-side expressions. The emission is behind one flag (runtimeGlobals), so closing it is a small change once there's a way to get the declarations into the editor's program — worth its own issue rather than a silent divergence.

Sign in to comment on this issue.