ReviewOS

stacks/stx

fix(client-script): auto-import detection requires `(` immediately after the name, so `useQuery<T>(…)` in a classic client script is a bare ReferenceError

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

Summary

transformAutoImports decides which runtime globals to destructure off window.stx with new RegExp('\\b' + symbol + '\\s*\\('). A TypeScript generic argument list puts a < where that regex demands a (, so useQuery<Result>('/api/x') is not recognised as a use of useQuery, the destructuring preamble omits the name, and the page throws ReferenceError: useQuery is not defined at runtime. There is no build error.

The detection runs before TypeScript transpilation, so the generic is still present when the regex sees the source.

The same file's siblings already handle this correctly — usesReactiveRuntime (runtime-globals.ts:139-146) and SIGNAL_API_RE (signal-processing.ts:1160) both use \s*(?:<[^>]*>)?\s*\(, and the second even carries the comment "Tolerates a generic argument list, since state<Foo>(…) is valid TypeScript and common." One file away, the classic path does not.

Impact (real app)

This one bit us as a belief rather than a crash, which is arguably worse. resources/stores/session.ts:90-98 in analyticshq:

Widen the default rather than writing useLocalStorage<SessionUser | null>(...): for a client <script> block the generic form is a runtime bug, because auto-import detection matches /\b<symbol>\s*\(/ on the raw source (client-script.js:400) and a < where the ( must be means the destructuring line is never emitted.

Acting on that, we annotate the binding instead of the call at every signal declaration — ~32 of them across 8 files (account.stx:15-22, reset-password.stx:16-23, login.stx:14-17, pricing.stx:22-24, forgot-password.stx:12-14, register.stx:14-15, sites/new.stx:18-19, session.ts:99-112) — plus null as SessionUser | null as a widened default (session.ts:100) and six useStore('session') as SessionStore casts instead of useStore<SessionStore>('session').

For the signal path that caution turns out to be unnecessary (see below), so most of those annotations are ours to remove. But the underlying regex is live, and any client script that reaches for useQuery<T> / useMutation<T> without also touching a signal API hits it for real.

Evidence in stx

packages/stx/src/client-script.ts:777-781:

for (const symbol of STX_AUTO_IMPORTS) {
  if (existingImports.has(symbol) || locallyDeclared.has(symbol)) continue
  const symbolRegex = new RegExp(`\\b${symbol}\\s*\\(`, 'g')   // ← no generic tolerance
  if (symbolRegex.test(transformedCode)) usedStxImports.add(symbol)
}

Order confirms the generic is still present — packages/stx/src/client-script.ts:1486-1506:

// 1. Transform auto-imports
const autoImportResult = transformAutoImports(code)      // :1487
…
// 3. Transpile TypeScript to JavaScript
code = transpileTypeScript(code)                          // :1506

Reproduced against the shipped packages/stx/dist/client-script.js (0.2.155):

// input
const q = useQuery<Foo>('/api/x')
navigate('/a')

// emitted preamble
var { navigate } = window.stx || window;   // useQuery missing
…
const q = useQuery("/api/x");              // ReferenceError at runtime

With the generic removed, the same input emits var { navigate, useQuery } = window.stx || window;.

Scope, for triage — the signal path is not affected. packages/stx/src/signal-processing.ts:1251 calls buildRuntimeGlobalsDestructure('const', mergedExportNames) with no source argument, so runtime-globals.ts:93-100 skips the filter and emits all 70 names unconditionally. The two gates that route a script there both already tolerate generics (signal-processing.ts:1160, runtime-globals.ts:145). The component path (utils.ts:1242) filters on a bare word boundary, not on (. So the bug is specific to processClientScript — i.e. a <script client> that uses a runtime global not in SIGNAL_API_RE: useQuery, useMutation, useHead, useSeoMeta, watchMultiple, defineStore, and ~20 others.

Prior art

Vue ref<T>(), computed<T>(), useFetch<T>(); Pinia defineStore<Id, State>; React useState<T>(). In all of them the generic goes at the call site and there is no binding-side workaround.

Ask

  1. Change client-script.ts:781 to match the pattern the rest of the codebase already uses:
    const symbolRegex = new RegExp(`\\b${symbol}\\s*(?:<[^>]*>)?\\s*\\(`, 'g')
  2. Add a test asserting useQuery<T>(…) / useMutation<T>(…) in a classic <script client> produce the same destructuring preamble as the non-generic form.
  3. Consider making the auto-import guard already emitted at client-script.ts:830 (generateAutoImportGuard) a build-time error rather than a runtime console.error — the failure currently only surfaces in the browser.

Related: #1804, #1819, #1785 (the previous rounds of drift in this same detection list).

11 days ago

Fixed in d095f9f.

The regex ran BEFORE transpilation, so useQuery<Foo>('/api/x') still carried its type argument when name\s*\( was tested against it:

generic call  -> []                 (not detected)
plain call    -> ["useClipboard"]   (detected)

So the name was left out of the window.stx destructure while the body still called it — ReferenceError, no build error.

Your scope analysis was right and worth restating: this only bites a script touching none of SIGNAL_API_RE's names, because one of those triggers a blanket destructure that rescues everything else. An app whose client block is pure data fetching has no such name, which is why it took this long to surface.

runtime-globals.ts:162 and signal-processing.ts:1165 already spell the generic-tolerant form — with the same comment explaining why — so this was the third copy and the one that drifted. Now identical to them.

Your own retraction about the ~32 binding-side annotations checks out: useLocalStorage, useStore and useFetch are all in SIGNAL_API_RE, so those were indeed unnecessary.

Sign in to comment on this issue.