ReviewOS

stacks/stx

feat(ssr): no typed server→client payload — the only bridge is identifier scraping, and the typed store-hydration path has zero callers

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

Summary

The only way to get a value from <script server> into <script client> is the implicit data bridge, which publishes a server binding when its name appears as a free identifier in the client source. There is no declaration, no schema, and no type. A page cannot state what it expects, so client code has to defend against the bridge not having emitted a name, and the payload's shape is verified by nothing.

stx already has the typed alternative in the tree — serializeStores() / generateHydrationScript() emit window.__STX_STORE_STATE__ and the runtime defineStore reads it — but nothing calls either function, so server-side store hydration is dead code.

Impact (real app)

In analyticshq's dashboard (a 25-query SSR page), every crossing is defensive or dead:

resources/views/dashboard.stx:788 — a runtime typeof guard, because nothing guarantees the bridge emitted the name:

const liveCount = state(typeof liveNow === 'number' ? liveNow : 0)

resources/views/dashboard.stx:720-721 — a server const that exists only to be scraped, which the linter correctly flags as unused:

// eslint-disable-next-line no-unused-vars
const activeSiteId = String(siteId)

resources/views/dashboard.stx:817-820 — the rule written out in prose because there is nowhere else to put it:

range is a server const reaching this block through stx's server->client data bridge, which inlines any server name that appears as a word in the client source and is not re-declared here (client-script.js:458-478).

types/dashboard.d.ts:9-13 and types/session.d.ts:11-15 both concede that none of it is checked:

tsc cannot see inside .stx, so an annotation written in a <script server> block is erased by the transpiler and verified by nothing.

The drift is already visible: the comment at dashboard.stx:704-706 still describes a data-export="" attribute, and grep -n 'data-export' resources/views/dashboard.stx matches only that comment — the 25-section export payload now rides the implicit bridge instead (const data = exportPayload at :956), inlined into the HTML on every request with no declared size bound.

Evidence in stx

packages/stx/src/client-script.ts:1019-1073 — the whole contract is name matching:

export function generateServerDataBridge(code: string, serverData?: Record<string, unknown>): string {
  const searchable = stripCommentsAndLiterals(code)
  for (const [name, value] of Object.entries(serverData)) {
    if (!referencesIdentifier(searchable, name)) continue      // ← the only selection rule
    if (declaresClientIdentifier(code, name)) continue
    …
    lines.push(`  var ${name} = ${json.replace(/</g, '\\u003c')};`)
  }
}

referencesIdentifier (:962-964) is new RegExp('(?<![.#\\w$])' + name + '(?![\\w$])'). Nothing types the emitted var.

The framework's own guardrails confirm the payload is unbounded and unaudited — client-script.ts:986 and :1000-1016:

const CREDENTIAL_NAME = /token|secret|passw|apikey|…/i
…
const oversized = json.length > 32_768

analyticshq trips the credential warning on every render, because shareToken (dashboard.stx:215) matches /token/.

The typed path exists and is unwired — packages/stx/src/state-management.ts:758 and :790:

export function serializeStores(): string { … }
export function generateHydrationScript(): string {
  return `<script>window.__STX_STORE_STATE__=${serializeStores()};…</script>`
}

packages/stx/src/signals.ts:5881 reads it:

var hydrationData = window.__STX_STORE_STATE__ && window.__STX_STORE_STATE__[id];

but grep -rn 'serializeStores\|generateHydrationScript' packages/stx/src returns only the two definitions above (plus an unrelated same-named function in hydration.ts:967). No render path calls either, so window.__STX_STORE_STATE__ is never emitted and the hydration branch is unreachable.

Prior art

  • Remix: useLoaderData<typeof loader>() — the loader's return type flows to the component.
  • Next.js: getServerSideProps → typed props.
  • Nuxt: useState(key, () => serverValue) + payload serialization; useAsyncData payload.
  • Inertia: page props + defineProps<PageProps>().

Ask

  1. Add an explicit declaration in the server block, e.g. defineClientPayload({ liveNow, range, exportPayload }), and have generateServerDataBridge publish only the declared set — so a name is either declared or absent, never "present if you happened to mention it".
  2. Emit a matching ambient .d.ts for the declared payload (or a per-page generated type) so a client block gets a checked liveNow: number instead of a typeof guard.
  3. Wire generateHydrationScript() into the render path (or delete it) — right now the runtime reads a global that nothing writes.
  4. Warn on the implicit path once a declaration exists, so apps can migrate off name-scraping.

Related: #1831 (the bridge publishing names found in comments — the hardening that made the current scraping rule), #1832.

10 days ago

Ask 1 is done in 1ee1b9a. Asks 2–4 are not, and one of them has a finding worth recording.

Shipped — the explicit declaration

defineClientPayload({ liveNow, range }) in a <script server> block. A declared name is published whether or not the client mentions it; an undeclared one is not published even if it does. So a name is either declared or absent, never "present because you happened to mention it" — which is the property that lets a client block drop typeof liveNow === 'number' ? liveNow : 0.

It captures values, not names, so the payload is what was declared rather than whatever the context held by the time the bridge ran. Declarations merge rather than replace, so a layout and the page it wraps can each declare their own crossings.

Scraping is untouched when nothing is declared — every existing page depends on it — and both paths are tested side by side so the old one cannot drift.

Everything the bridge already guaranteed still applies to a declared payload, each pinned by a test: < is escaped so a value containing markup cannot break out of the tag, a name the client declares itself is never clobbered, functions are dropped, and the __ opt-out still wins.

Ask 3 — confirmed dead, and I did not guess which way to resolve it

$ grep -rn 'serializeStores\|generateHydrationScript' packages/stx/src   # non-test, non-dist
packages/stx/src/state-management.ts:790:export function serializeStores(): string {
packages/stx/src/state-management.ts:822:export function generateHydrationScript(): string {
packages/stx/src/state-management.ts:823:  const serialized = serializeStores()

Zero callers outside their own definitions, while packages/stx/src/signals.ts:6398 reads the global they would write:

var hydrationData = window.__STX_STORE_STATE__ && window.__STX_STORE_STATE__[id];

So the hydration branch is unreachable, exactly as reported. I left it alone because wiring vs deleting is a product call, not a mechanical one: wiring means every page ships serialized store state in its HTML on every request, which is a payload-size decision this issue is otherwise arguing against. Deleting removes an API someone may have been about to use. Worth someone stating which, since right now it is neither.

Asks 2 and 4 — not started

  • Ask 2 (generated .d.ts for the declared payload) is the part that turns this from "deterministic" into "typed", and it is the larger half. Note it now has something concrete to generate from, which it did not before.
  • Ask 4 (warn on the implicit path) is small but should wait until 2 lands — warning people off scraping before the typed replacement exists would just be noise.

11 tests, 5 failing without the change.

10 days ago

Ask 3 resolved — deleted (dc0b593907).

serializeStores / hydrateStores / generateHydrationScript in state-management.ts had zero callers and were never re-exported from index.ts, so window.__STX_STORE_STATE__ was never written and the runtime read never fired — exactly as reported. Resolved by delete, not wire: wiring would ship serialized store state in every page's HTML on every request, the payload cost this issue argues against. (Distinct from the live generateHydrationScript in hydration.ts, which is kept.)

Deliberately kept the client-side reader in signals.ts as the dormant hook a future typed payload (Ask 2) can write into on purpose — removing it would foreclose the wiring option Ask 2 wants, rather than just removing dead producer code. So the runtime still has a hydration consumer; what's gone is the untyped, unbounded producer nobody called.

Dropped the three orphaned describe blocks in state-management/server.test.ts; the live useStore block stays (3 pass). Type-clean, lint clean.

Remaining: Ask 2 (generated .d.ts for the declared payload — the typed half, larger and coupled to seeing types inside .stx) and Ask 4 (warn on the implicit path, best after Ask 2). Ask 1 shipped in 1ee1b9a746.

10 days ago

Ask 2 is in — efc13f412a

A client block now gets the declared payload typed from the server block, so the guard this issue opens with has something to be replaced by:

<script server>
const liveNow = await countLive()      // no annotation
defineClientPayload({ liveNow })
</script>
<script client>
const label: string = liveNow          // Type 'number' is not assignable to type 'string'
</script>

Projected, not annotated. The server block is re-emitted inside a wrapper function returning exactly the declared payload, and each published name is declared as a lookup into that function's return type — so const range = loadRange() arrives as whatever loadRange returns, with no second declaration that can drift from the first. Inference works through function calls, not just literals.

A name the payload does not publish is an error, which is the half that makes declaring worth doing: at runtime it is simply absent, because extractBridgeData publishes the declared set in full and nothing else.

Two things had to be true first, and neither was

defineClientPayload was not in the checker's global surface. Calling the one API the typed bridge is built on was itself Cannot find name 'defineClientPayload'.

Every bridged value was Cannot find name too. A client block is checked in its own buffer, so stx typecheck reported an error on every page that used the bridge at all. A page that declares no payload now gets its server bindings as any — the honest description of a value delivered by textual reference — so the incentive runs the right way: declare and they are checked, or don't and they're any. A name that comes from nowhere is still an error.

The blindness underneath it — worth knowing independently of this issue

const duration = inside <script client> is a supported stx pattern, and 48 of the framework's own 95 components use it. As TypeScript it is a syntax error, and a parse failure suppresses every other diagnostic in that file. So the checker was not strict on the component library — it was blind, and would have reported clean no matter what was in those files.

Interpolations are now substituted in place, padded to the original width and carrying the original newline count, so a diagnostic after one still points at the line the author wrote.

Measured over 268 .stx files

diagnostics
before438 (369 of them the component library failing to parse)
after the fix2
after fixing those 20

Both survivors were real bugs and are fixed in the same commit: a single-quoted string spanning lines in terminal.stx (which cannot evaluate, so that welcome message never rendered), and a misplaced quote in claims.stx putting a whole value where a key belonged. A gate that ships red is a gate that gets muted.

19 tests; the four sabotages fail 4 / 2 / 4 / 1 tests respectively.

Still open

Ask 4 — warning on the implicit path. It has its typed replacement now, so the warning would no longer be noise. The shape I would suggest: report the scraped names at warning severity rather than error, since they are already emitted as any and the fix is mechanical (add a defineClientPayload naming them). Happy to do it, but it changes what CI says for every existing page, so it wants your call on severity first.

Ask 1 shipped in 1ee1b9a746, ask 3 resolved by deletion in dc0b593907.

10 days ago

Ask 4 is in — 303ba7e57d. Closing.

stx typecheck now names what still crosses implicitly:

2 values reach this block on the implicit bridge and are untyped: range, siteId.
Declare them with defineClientPayload({ range, siteId }) in the server block to have them checked.

A warning, not an error. These pages work — the value does arrive — but it arrives because its name happened to appear in the client source, and untyped. Failing CI over that punishes an app for predating the feature; the reward for declaring is types, not correctness. One line per file rather than per name.

Measured over the same 268 .stx files: 0 errors, 57 warnings, each naming a real crossing.

One thing worth recording, because it nearly shipped wrong

The warning has to describe exactly the set the runtime publishes, or it is worse than nothing. The bridge decides that with stripCommentsAndLiterals, which recurses into ${…} because an interpolation is code. My first version had its own copy that blanked template literals whole — so it silently under-reported every ${name} reference, and the warning and the runtime would have described different sets.

It is now literally the same function, moved to strip-literals.ts and imported by both. Its own module because the editor-facing extractor cannot import client-script.ts without pulling the bundler in behind it, and the alternative was the second implementation that had just gone wrong. Re-exported from its old home so nothing importing it from client-script has to change.


All four asks are done

1 — explicit declaration, publish only the declared set1ee1b9a746
2 — typed payload for the client blockefc13f412a
3 — wire or delete the dead store-hydration pathdc0b593907 (deleted)
4 — warn on the implicit path303ba7e57d

Two things found along the way that were arguably worse than the issue itself, both fixed in ask 2's commit: defineClientPayload was not in the checker's global surface, so calling the API this issue asked for was itself Cannot find name; and inside a <script client> block is a TypeScript syntax error, so the checker was blind on 48 of the framework's own 95 components rather than strict — a parse failure suppresses every other diagnostic in the file.

Sign in to comment on this issue.