ReviewOS

stacks/stx

stx typecheck: x-tooltip prose is type-checked as an expression, and the implicit-bridge warning names values that never cross (34 of 36 diagnostics in a real app)

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

Two accuracy defects in stx typecheck, found turning it on for a real app. Together they account for 34 of the 36 diagnostics it reports there, and both point at correct code.

Filing against the handoff's own rule — a gate that invents errors gets muted, and a muted gate catches nothing.


1. x-tooltip prose is parsed as a TypeScript expression

x-tooltip is in RUNTIME_HANDLED_X_ATTRS, and the runtime reads it as literal text — signals.ts:6050:

var text = el.getAttribute('x-tooltip');

It is never evaluated. But typecheck treats the attribute value as a template expression and hands the prose to TypeScript.

Repro

tooltip.stx, two lines of markup, no script block:

<button x-tooltip="Toggle theme" aria-label="Toggle theme">Theme</button>
<span x-tooltip="Pending — becomes a member once they accept">Invited</span>
$ stx typecheck
tooltip.stx:1:32  error TS1128  Declaration or statement expected.  [template: Toggle theme]
tooltip.stx:2:26  error TS1127  Invalid character.                  [template: Pending — becomes a member once they accept]
tooltip.stx:2:28  error TS1434  Unexpected keyword or identifier.   [template: Pending — becomes a member once they accept]
… 12 errors total

  Checked 0 script block(s) and 2 template expression(s) in 1 file(s) — 12 error(s), 0 warning(s)

0 script blocks, 2 "template expressions." The two expressions are the tooltip strings.

Control — same prose in title=, which is not a directive:

<button title="Toggle theme" aria-label="Toggle theme">Theme</button>
  Checked 0 script block(s) and 0 template expression(s) in 0 file(s) — 0 error(s), 0 warning(s)

Zero expressions. So the attribute name alone decides it.

The em-dash producing TS1127 Invalid character is the tell: any tooltip written as a normal English sentence fails, and the more human the copy the more errors it generates.

Presumably the fix is to exclude the literal-text members of RUNTIME_HANDLED_X_ATTRS from expression checking. x-tooltip-position looks like the same case.


2. The implicit-bridge warning names values that do not cross

The warning reports a function-local server binding as crossing to the client, and the remedy it suggests is a silent no-op.

Repro

<script server>
function buildUrl() {
  const state = { level: 'all', page: 1 }
  return state.level + state.page
}
const link = buildUrl()
</script>
<script client>
const n = state(0)
function bump() { n.set(n() + 1) }
</script>
<div :text="n()">0</div><a href="">go</a>
bridge.stx:8:1  warning TS0  1 value reaches this block on the implicit bridge and is untyped: state.
                Declare them with defineClientPayload({ state }) in the server block to have them checked.

state does not reach that block. It is local to buildUrl, and the emitted client script destructures the runtime's state as normal:

const { …, state, … } = window.stx;
const n = state(0);

No bridge line for state is emitted, with or without the declaration.

The suggested fix is a no-op

Following the advice and writing defineClientPayload({ state }) changes nothing: generateServerDataBridge skips any name the client declares, and the runtime-globals destructure counts as a declaration. So the author is directed to add a line that has no effect, to fix a crossing that is not happening.

Worth noting it is not harmful — I expected it to shadow the state() primitive and checked; the skip prevents that. But it is work invented by the checker.

Two things look wrong independently: the scan does not respect function scope, and it does not exclude names the client runtime already provides. The second would suppress this case even if the first is hard — no runtime global should ever be reported as an implicit-bridge crossing, since it cannot be one.


Both were measured on current main with a freshly built dist.

10 days ago

Both halves fixed.

1. x-tooltip prose — already fixed in 1911fbc404 (filed as #1907). Your two-line repro reports 0 diagnostics. x-tooltip and x-tooltip-position are non-expression attributes now; you were right that signals.ts:6050 reads them with getAttribute and never evaluates them.

2. The implicit-bridge warning — fixed in dff7e81357. This one was mine, from #1868 ask 4, and it was wrong twice over:

  • collectBlockDeclarations matches ^\s*const … with leading whitespace allowed, so it returned bindings local to a function. Now blanked to depth-zero declarations only.
  • state is a runtime global. Every client block destructures it from window.stx, so it never crosses — and the remedy the warning printed, defineClientPayload({ state }), would have shadowed the runtime's own. Names the runtime provides, and names the client block declares itself, are both excluded now. The real bridge already skipped the second via declaresClientIdentifier; my warning did not, which is the underlying error: it was more permissive than the thing it describes.

Worth recording, because it nearly shipped

My first test for this used your repro verbatim. Its local is named state — which the runtime-globals filter excludes on its own — so the test passed whether or not the top-level scoping worked, and sabotaging that half changed nothing. Replaced with a local named something nothing else filters.

"34 of 36 diagnostics, both pointing at correct code" is the right way to have measured it, and quoting the handoff's own rule back at me was fair.

Sign in to comment on this issue.