ReviewOS

stacks/stx

variable-extractor drops `let x!: T` (annotation, no initializer): the binding never reaches the client scope and every template read silently renders nothing

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

A client-block declaration that has a type annotation but no initializer — let rows!: () => Row[] — is never extracted, so the binding does not exist in the client scope. Every template expression reading it silently evaluates to nothing. No console error, no diagnostic, the element just renders empty.

What it cost

Adding types to a production composer view. The declaration went from

let providerRows = null

to

let providerRows!: () => ProviderRow[]

and nothing else changed — the assignment (providerRows = derived(…)) and the template (x-for="row in providerRows()") were untouched. Rendered, in a real browser against the same API responses:

provider rows rendered
let providerRows = null2
let providerRows!: () => ProviderRow[]0

Zero console errors in both. The page looks fine; the channel picker is simply gone.

Nothing catches it

  • stx typecheck reports 0 errors on the repro below, for both forms.
  • stx codemod reports nothing.
  • tsc, lint and the unit suite are all green — the failure only exists in the rendered DOM.

This is the "answered 200 and did nothing" class from the August sweep notes, reached through the type system.

Repro

<script client>
  let withInit = null
  let noInit!: () => string[]

  withInit = () => ['a', 'b']
  noInit = () => ['a', 'b']
</script>

<template>
  <ul data-with-init><li x-for="x in withInit()" :text="x"></li></ul>
  <ul data-no-init><li x-for="x in noInit()" :text="x"></li></ul>
</template>

data-with-init renders two <li>. data-no-init renders none.

Cause

packages/stx/src/variable-extractor.ts has two declaration branches and this form falls between them:

// 129 — requires an initializer
const match = line.match(/(?:const|let|var)\s+(\w+)\s*=/)

// 136 — declaration-only, but the line must end right after the name
else if (/^\s*(?:const|let|var)\s+\w+\s*;?\s*$/.test(line)) {

let noInit!: () => string[] has no =, so 129 misses, and it does not end after the name, so 136 misses too.

Line 202 already handles the annotated-with-initializer form by stripping the annotation:

/^(\s*(?:export\s+)?(?:const|let|var)\s+)(\w+|\{[^}]+\}|\[[^\]]+\])\s*:\s*[^=]+(\s*=)/gm

so const x: T = … is fine. It is specifically annotation-without-initializer that is dropped.

Fix

Widen the declaration-only branch to allow a definite-assignment ! and a type annotation, i.e. accept let x, let x!: T, and let x: T alongside the initialized forms.

Worth considering a diagnostic as well: a template expression calling an identifier that no declaration published is currently indistinguishable from one that legitimately evaluates to nothing, and that is what makes this cost an afternoon rather than a minute. Related in spirit to #1890, which was the same extractor dropping a different declaration shape.

14 days ago

Fixed in 6e0a23c. Your repro was exact — but the file was not, and it's worth being precise about because the distinction is the reason only one side broke.

Where it actually was

extractExports in signal-processing.ts. It builds the object the setup function returns — the client scope every template expression is evaluated against — and it matched const|let|var NAME =, requiring the =. So the name never entered the scope, and a template reading it evaluated to nothing.

Reproduced as return { src, withInit } — noInit simply absent.

Where it wasn't: variable-extractor.ts

Both its branches have exactly the shape you identified, so the diagnosis is reasonable. But that extractor serves <script server> blocks, and I measured it before changing anything: it already publishes every one of these forms.

let c!: () => string[]           -> published
let d: (() => string[]) | undefined -> published
let e                            -> published
let x!: string; if (true) { x = "v" }  -> published

Its later-assignment re-sync picks up the value whether or not the declaration matched, including when the assignment is inside an if or a try. I wrote the widened regex there first, then found it changed nothing observable, and reverted it rather than ship an edit I couldn't demonstrate.

The client scope has no such re-sync. extractExports is the only thing deciding what a template can see, so a name it misses is simply gone. That asymmetry is the whole bug: the same declaration works in <script server> and vanishes in <script client>.

Also worth knowing for your original repro: with no signals in the block it takes the plain IIFE path, which emits let noInit; correctly. It's specifically the setup-function path — the one state()/derived() trigger, i.e. your real composer view — that dropped it. A minimal repro without a signal wouldn't have shown the bug.

The fix

The terminator is matched rather than the initializer required, so let x, let x!: T, let x: T, let x; and let x, y all publish.

Matching the terminator is also what makes let fn!: (a: string) => Promise<number> work: an annotation can contain =, so anything trying to consume the type stops on the arrow and loses the declaration again. =(?!=) keeps a comparison from being read as a declaration.

On the diagnostic you suggested

Agreed that it's the thing that would turn this from an afternoon into a minute, and it's a bigger change than this fix — a template expression calling an identifier no declaration published is currently indistinguishable from one that legitimately evaluates to nothing. Worth its own issue; the scope object is now the right place to check against, since it's authoritative about what a template can see.

Verification

7 tests, including the end-to-end return {...} assertion. Sabotage-checked by restoring the =-required regex: 5 of 7 fail, including the rendered-page one.

Full suite 11864 pass. The 4 failures on that run (craft binary resolution ×2, X-STX-Runtime ×2) reproduce on a clean checkout of HEAD and are unrelated.

Sign in to comment on this issue.