ReviewOS

also looking at this

stacks/stx

fix(client-script): bind template names shadowed by inlined imports (#1767)

#1768
Merged glennmichael123 wants to merge fix/1767-client-script-import-shadow-collision into main
4 files +97 -1
9 days ago

Fixes #1767.

The bug

A component <script client> that imports from a module and declares a top-level const whose name equals a top-level export of that module silently loses its binding. bundleClientScript's Bun.build inlines the import under the bare name and renames the component's local (foo → foo2). The catch-all export { foo } the bundler already adds (to defeat tree-shaking) comes back as export { foo2 as foo } — the only record of the rename — and line 343's blind strip discards it. Scope registration then re-harvests the bare foo (now the raw import) and binds the template to the wrong value. Aliasing the import (import { foo as bar }) doesn't help.

Symptom in the wild: a :for over the "derived" iterated the raw import, and x-html="it.missing" rendered the literal string undefined (clipped to "defin" in a small avatar — very confusing to debug).

Minimal repro

data.ts

export const items = [{ id: 1, label: 'raw' }]

Widget.stx

<script client>
  import { items as allItems } from './data'
  const items = derived(() => allItems.map(x => ({ ...x, extra: 'computed' })))
</script>
<div :for="it in items">{{ it.extra }}</div>

Expected it.extra === 'computed'; actual undefined (binds to the raw import). The emitted bundle shows var items2 = derived(...) while the template still binds bare items.

The fix

  • bundleClientScript — capture the X as Y rename map before stripping the export, and re-expose it as var __stxScopeRenames = { "Y": X } (original name → renamed binding). It deliberately does not reassign the bare name (var foo = foo2): the renamed local's closure still reads the import under foo, so clobbering it would break the very binding it depends on. Emitted only when a rename actually happened.
  • transformSignalScript — merge __stxScopeRenames into __scopeRegistration after the normal harvest, so the template's original name resolves to the component's declaration, not the shadowing import.
  • x-html — coerce null/undefined to '' before innerHTML =, so a missing/unresolved binding renders nothing instead of the literal string undefined.

Verification

  • New regression test: test/client-script-bundler/import-shadow-collision.test.ts.
  • Full suite green — 7795 pass, no regressions from this change (the lone failure in a full run is a pre-existing flaky route-middleware throttling test that passes in isolation on both this branch and main).
  • End-to-end: compiled a real @include and evaluated the emitted scope registration — the original name now binds to the derived (extra: "computed"), not the raw import.

Scope / follow-up

This wires the fix through the per-component scope path (includes.ts transformSignalScript) — the reproduced case. The page-level merged-script path (signal-processing.ts, return { ...mergedExports }) shares the same extractExports-on-bundled-code pattern and would benefit from consuming __stxScopeRenames too; left out here to keep the change focused and low-risk (multiple merged scripts would need per-script map names). Happy to extend if preferred.

4 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.