ReviewOS

stacks/stx

The authoring surface is a subset of the package: defineForm, useAsyncData, Teleport, redirect, StxModalBuiltin et al are exported but unreachable from <script client>

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

Companion to #1843, and a correction to its framing.

That report concludes the workarounds it found are "an adoption problem, not a missing-feature problem", because the primitives are already exported. I audited a second, independent app (loghq — 6.5k lines, SSR, 33 routes) and found the same hand-rolling. But the primitives it names are exported from the package and not reachable from the surface apps author in.

Measured

packages/stx/src/runtime-globals.ts is the single source of truth for what a <script client> block can call — it is destructured into every generated client script, and runtime-globals.test.ts already asserts every name in it exists on window.stx.

Primitivein src/in STX_RUNTIME_GLOBALSon window.stx
useFetch✅✅✅
defineForm✅❌❌
validateFields✅❌❌
useAsyncData✅❌❌
StxModalBuiltin✅❌❌
StxToastBuiltin✅❌❌
Teleport✅❌❌
redirect✅❌❌
useRouteParams✅❌❌
$ grep -oE "'(defineForm|validateFields|useAsyncData|Teleport|redirect|useRouteParams|useFetch)'" src/runtime-globals.ts
'useFetch'

One of the nine is reachable. So an author writing defineForm(...) in a client script gets a bare ReferenceError, and the natural conclusion — "the framework doesn't have forms" — is what both apps acted on.

This is the same shape as #1804 and #1838, one layer up: the list exists, the guard exists, and these names were never added to it.

Why it produced identical workarounds in two unrelated apps

loghq, with no knowledge of bench:

  • 5 forms, each re-typing loading / error / disabled by hand. login.stx alone has 6 loading and 10 error references. defineForm and validateFields exist and cannot be called.
  • 21 raw fetch( in views against 2 uses of useQuery/useMutation. useAsyncData exists and cannot be called.
  • A modal with a hand-written focus trap, scroll lock and focus restore, added by hand this week. StxModalBuiltin and Teleport exist and cannot be called.

Ask

  1. Add the missing names to STX_RUNTIME_GLOBALS — or state explicitly which are server-only, so the boundary is discoverable rather than inferred from a ReferenceError.
  2. Extend runtime-globals.test.ts to assert the inverse direction too: every public composable exported from the package is either in the list or explicitly annotated as server-only. Without that, this recurs the next time a composable is added.
  3. #1843's ask 1 ("confirm these reach the client in a released build") should fail today for eight of the nine above, in a fork build as well as a release.

Independent of reachability

One item does not fit the "exists but unreachable" pattern: no form in either app can work without JavaScript. In loghq all five carry no action and no method — they are preventDefault + fetch. defineForm becoming reachable would remove the duplication but not this; that needs a server-side form action (Laravel's controller + @error/old(), Remix/SvelteKit form actions). Filing separately if you want it tracked apart from this.

11 days ago

Confirmed in a second production app (bench, ~6.5k-line judge/review app on stx + crosswind). This is the root the other symptoms hang off — the exports exist but can't be reached from <script client>, so the app hand-builds substitutes:

  • StxModalBuiltin / Teleport → a hand-built ToastHost / ConfirmHost signal-array portal pattern. The in-code note at resources/stores/toast.ts:16-17 spells out why: "an imperative ref.show() API is impossible because stx's ref store holds DOM elements, not a component's exposed methods." (same at confirm.ts:24).
  • useFetch / useAsyncData → 12 raw fetch() calls plus a hand-rolled _apiFetch wrapper (resources/stores/auth.ts:132).
  • redirect → window.location.assign() (stores/auth.ts, stores/admin.ts).

We filed #1843 as an "adoption report" thinking this was adoption debt; #1846 is the sharper, correct framing — the authoring surface is the actual wall. +1 from us.

11 days ago

Picking this up from the bench side (the ~6.5k-line production app behind the #1843 adoption report — this is the root cause of that report: the primitives exist but a <script client> block can't reach them). Flagging here to avoid double-work; will post an approach before a PR.

11 days ago

Partially fixed in 4ac3fa8, and the remainder is blocked on something the issue does not mention — worth knowing before anyone picks it up.

Measured

Of the five APIs named here, all five are on neither window.stx nor STX_RUNTIME_GLOBALS:

  defineForm         onWindowStx=false  inGlobals=false
  useAsyncData       onWindowStx=false  inGlobals=false
  Teleport           onWindowStx=false  inGlobals=false
  redirect           onWindowStx=false  inGlobals=false
  StxModalBuiltin    onWindowStx=false  inGlobals=false

So they cannot simply be added to the auto-import list. A name in that list is destructured off window.stx into every client script, so listing one that isn't there binds a bare identifier to undefined and fails at the call site as "undefined is not a function" — the exact phantom class #1804 removed. They need a runtime implementation first.

The part that IS blocked, and why

defineForm cannot be published to window.stx as it stands. It is built on the wrong reactive system:

packages/stx/src/forms-validation.ts:19
  import { state } from './signals-api'

Per the audit on #1885, window.stx.state !== signals-api.state — they are two independent implementations with disjoint subscriber sets, and templates are driven by the runtime one. So exposing defineForm to a <script client> would hand the author a form whose signals cannot update a template: every binding would render once and then freeze, silently.

That is strictly worse than today's "unreachable", because unreachable fails loudly at the call site while this fails as a page that looks fine and never updates. So #1846 is gated on #1885 for defineForm, and probably for useAsyncData on the same grounds — worth checking each of the five individually rather than treating them as one batch.

Shipped

A separate, real gap in the same class: useRouteParams / useRouteParam were published on window.stx by 5daaba1 and never added to the list, so they were reachable only as window.stx.useRouteParams(). Now auto-imported and declared.

Rather than fix that one name, the rule is now pinned: every runtime member must be either auto-imported or listed as a deliberate exclusion with a reason, and a new member fails the test until someone decides which. The exclusions are also staleness-checked. Current exclusions are alert/confirm (would shadow the browser globals), modal/drawer/toast (too generic as bare identifiers — a duplicate declaration, not a shadow), mount/mountEl/hydrate/helpers (runtime plumbing), $watch/$computed (compiler aliases).

That should stop the next 13-name drift from accumulating silently, which is the actual complaint behind this issue.

11 days ago

Landed the structural half in 731894fe02 (asks #1 + #2):

  • NON_CLIENT_PRIMITIVES in runtime-globals.ts — a source-level, categorized map so the client/server boundary is discoverable instead of inferred from a ReferenceError. redirect → server-only (client equivalent is navigate()); Teleport/StxModalBuiltin/StxToastBuiltin → template/builtin surface; useAsyncData → demand-bundled (reachable via the composable path, just not a bare global); defineForm/validateFields → client-gap (intended for <script client> but not yet wired into a client path).
  • Export-side guard (authoring-surface.test.ts): every use* composable in src/composables must be client-reachable (path 1 STX_RUNTIME_GLOBALS, or path 2 SERVER_ONLY_COMPOSABLES) or classified — with a non-vacuity check and pins for the names above. A new composable now fails until its reachability is decided.

Confirmed the two client paths: (1) bare global in the signals.ts window.stx literal + the list; (2) on-demand inline from src/composables via SERVER_ONLY_COMPOSABLES. defineForm/validateFields live in forms(-validation).ts — neither path — so making them reachable means porting them into a client path. That's the remaining half (the client-gap entries); leaving this open for it.

11 days ago

Client-gap update — defineForm is now reachable from a <script client> block (c9597e5b35).

The structural half landed earlier (731894fe02): NON_CLIENT_PRIMITIVES annotates the boundary and an export-side guardrail test fails when a new composable goes unclassified. This closes the flagship client-gap it flagged.

How: #1856 made defineForm reactive (backed by signals-api's state()); this makes it reachable. defineForm and its signal-backed record helper moved to composables/use-form.ts and joined SERVER_ONLY_COMPOSABLES, so the framework-composables demand-bundler (the same path useAsyncData and the observer family ride) ships it — whole, imports stripped — to any page that calls it. Its one runtime dep, state, is a bare global (window.state), so the stripped import resolves. Re-exported from forms-validation.ts, so index.ts and every existing importer are unchanged. Reclassified client-gap → browser-composable.

Proven in framework-composables.test.ts: defineForm is offered for a page that calls it, bundled whole with its helper, import-free/parseable, and shipped through the render pipeline. Full src tsc clean; forms + runtime-globals + public-api suites green (331).

Still open — validateFields/validateField. These are not cleanly demand-bundleable: validateFields depends on validateField, which pulls the whole server-side rule engine (enhancedValidationRules etc.), so taking the module whole would drag server code into the browser bundle. The guardrail now records them as client-gap with that exact reason. Options for a follow-up: (a) split a browser-safe validation core into a composable, or (b) port the rule engine into the runtime as a bare global. Leaving this issue open for that decision.

11 days ago

Resolved — closing. Every primitive this issue named is now either reachable or classified with a deliberate reason, and a guardrail keeps it that way.

Structural half (731894fe02): NON_CLIENT_PRIMITIVES annotates the client/server boundary in source, and an export-side guard in authoring-surface.test.ts fails when a newly exported composable is reachable by no client path and unclassified. The boundary is discoverable instead of a runtime ReferenceError.

defineForm (c9597e5b35): now reachable from <script client>. Moved to composables/use-form.ts + SERVER_ONLY_COMPOSABLES, so the framework-composables demand-bundler ships it exactly like useAsyncData. Proven end-to-end in framework-composables.test.ts.

validateFields / validateField (9330d5d051): settled as isomorphic-import, a new category distinct from client-gap. They are pure and would run in a browser, but the project already decided they are NOT part of the auto-delivered client surface — see the delivery-surface exclusion in signals/client-api-delivery.test.ts, which groups them with query/redirect/defaultFormClasses. An app that wants them client-side imports them explicitly so its own bundler inlines them. This is a boundary drawn on purpose, not a gap left open — so the map no longer frames them as unfinished work.

Already reachable / boundary-correct: useAsyncData (demand-bundled), redirect (server-only, use navigate()), Teleport/StxModalBuiltin/StxToastBuiltin (template surface).

Net: the authoring surface is now a documented, test-guarded contract rather than a subset that drifts silently.

Sign in to comment on this issue.