ReviewOS

stacks/stx

typecheck: a bare <script> block is never checked, so syntax errors in it ship silently

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

stx typecheck reports diagnostics for <script server> and <script client> blocks but skips a bare <script> entirely. Since a bare <script> is the server block — it is what most components in storage/framework/defaults actually use — this means the most common block form in the ecosystem is unchecked.

Repro

Three files, byte-identical except for the opening tag:

<!-- bare.stx -->
<script>
const broken = (((
</script>
<div>hi</div>
$ stx typecheck bare.stx
  Checked 0 script block(s) and 0 template expression(s) in 0 file(s) — 0 error(s), 0 warning(s)

$ stx typecheck server.stx      # same content, <script server>
  Checked 1 script block(s) and 0 template expression(s) in 1 file(s) — 1 error(s), 0 warning(s)

$ stx typecheck client.stx      # same content, <script client>
  Checked 1 script block(s) and 0 template expression(s) in 1 file(s) — 1 error(s), 0 warning(s)

On a real component the file is still counted, but the block is not — Checked 0 script block(s) and 12 template expression(s) in 1 file(s) for a file that plainly has one script block.

stx 0.2.175.

Why this matters

This is not hypothetical. storage/framework/defaults shipped 18 script blocks that could not parse, across many releases, and typecheck reported only 2 of them — precisely the two in <script client> blocks. The other 16 were bare <script> and were invisible:

// Marketing/Feature.stx, as shipped
const { className = '', feature = {, isActive = false } = defineProps<FeatureProps>()

name: '',
  summary: '',
  ...
}

Sixteen components were dead on arrival for anyone scaffolding from defaults, and the tool whose job is to catch that reported success. Fixed in stacksjs/stacks@f2fdb53, but the reason it went unnoticed is this gap.

Expected

A bare <script> should be checked exactly as <script server> is — they are the same thing. If there is a deliberate reason to skip it, the summary should say the block was skipped rather than counting it as zero, because "0 errors" over an unchecked block reads as a pass.

Related: the same "reports success while measuring nothing" family as #1906 (--lib silencing every diagnostic) and #1911 (prohibited APIs matched inside comments).

14 days ago

Fixed in f85e31a.

A bare <script> is now checked. It is checked as a client block, not a server one — and that correction matters for the fix. Only <script server> runs on the server, so client is an explicit alias for the default and the two forms are the same thing; the bare form is not the server block. It is load-bearing here because a client block also receives the server-to-client payload declarations, so checking a bare block as a server block would have invented "Cannot find name" on every value crossing the bridge.

Measured over a 42-file corpus: the newly-checked blocks produced 353 diagnostics and not one was TS2304 "Cannot find name", against 47 for the client blocks already in the run. Wrong context and that number would have dominated.

Diagnostics also name the tag you wrote — [<script>], not [<script plain>], which is this checker's internal word for a tag with no attribute.

Two things turned up once the blocks were actually checked

Both would have made the fix a net loss, so they are in the same commit.

1. One syntax error was switching off type checking for the whole run.

tsc collects semantic diagnostics only when the program has no syntactic ones — program-wide, not per file (emitFilesAndReportErrors). Every generated buffer shares one program, so a single unparseable block silently disabled type checking for every other file in the run. On that same corpus: 500 real type errors became 52 syntax errors and nothing else. Not an abort, not a warning — a smaller number that reads like progress.

That is the family you named, and the trap is specific: an unparseable bare <script> is exactly what this issue is about, so the fix for one blind spot walked straight into another. Unparseable buffers are now dropped and the rest re-checked, so both halves arrive from one run instead of the second only appearing after you fix the first.

2. A directive in a script body was reported as a syntax error.

A script can loop over server data to build a lookup table, and the directive is expanded long before the browser sees it:

const data = {
  @foreach(features as f)
    '{⁠{ f.id }}': { title: '{⁠{ f.title }}' },
  @endforeach
}

Checking bare blocks made one valid example page report 37 invented syntax errors. A checker that invents errors on working code gets muted, and a muted gate catches nothing — and per (1), an invented syntax error does not merely add noise, it takes real diagnostics down with it. Directives alone on their line are now blanked in place, so every line number below them still points at the code you wrote. Only known directive names, and only alone on a line, so a decorator and "hi@if.example.com" are untouched.

Verification

Three test files, each sabotage-checked — reverting the filter fails 5 of 6 bare-script tests, removing the retry fails the suppression test specifically, and making the directive blanking a no-op fails 3 including the line-number one.

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

One thing I left alone

packages/stx/test/streaming/templates/dashboard.stx uses <script props> as a data block holding object-literal entries. It has no type attribute, so it now reads as a script and reports 7 syntax errors. I did not add an attribute-based skip list for it — that would recreate exactly the "silently unchecked" hole this issue is about, and the real island props block is already <script type="application/json" data-island-props>, which the existing content filters handle. Happy to revisit if that form is meant to be supported.

Sign in to comment on this issue.