also looking at this
fix(client-script): bind template names shadowed by inlined imports (#1767)
#1768
4 files
+97
-1
| @@ -338,6 +338,25 @@ export async function bundleClientScript( | ||
| 338 | 338 | break |
| 339 | 339 | } |
| 340 | 340 | |
| 341 | // Capture Bun's export-rename map BEFORE stripping (stacksjs/stx#1767). | |
| 342 | // When an inlined import shares a top-level name with a component-declared | |
| 343 | // const, Bun.build keeps the import under the bare name and RENAMES the | |
| 344 | // local (`foo` -> `foo2`); the catch-all export we added above then comes | |
| 345 | // back as `export { foo2 as foo }`. That `as` clause is the only record of | |
| 346 | // the rename. If we strip it blindly, the scope registrar re-harvests the | |
| 347 | // bare `foo` — now the raw import — and binds the template to the wrong | |
| 348 | // value. So collect [originalName, renamedBinding] pairs here and re-expose | |
| 349 | // them below, letting the registrar bind the original name to the correct | |
| 350 | // (renamed) identifier without touching the import the local closes over. | |
| 351 | const renamePairs: Array<[string, string]> = [] | |
| 352 | for (const exp of bundled.matchAll(/^\s*export\s*\{([^}]*)\}\s*;?\s*$/gm)) { | |
| 353 | for (const spec of exp[1].split(',')) { | |
| 354 | const asMatch = spec.trim().match(/^([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)$/) | |
| 355 | if (asMatch && asMatch[1] !== asMatch[2]) | |
| 356 | renamePairs.push([asMatch[2], asMatch[1]]) // [exported/original name, renamed local binding] | |
| 357 | } | |
| 358 | } | |
| 359 | ||
| 341 | 360 | // Strip ESM export artifacts from the output |
| 342 | 361 | // Bun.build with format: 'esm' may add `export { ... }` at the end |
| 343 | 362 | bundled = bundled |
| @@ -345,6 +364,17 @@ export async function bundleClientScript( | ||
| 345 | 364 | .replace(/^\s*export\s+default\s+.*$/gm, '') |
| 346 | 365 | .trim() |
| 347 | 366 | |
| 367 | // Re-expose renamed component declarations under their ORIGINAL names as a | |
| 368 | // plain map (originalName -> renamed binding). The scope registrar merges | |
| 369 | // this so the template binds to the component's declaration, not the | |
| 370 | // shadowing import. We do NOT reassign the bare name (`var foo = foo2`): | |
| 371 | // the renamed local's closure still reads the import under `foo`, so | |
| 372 | // clobbering it would break the binding it depends on. See #1767. | |
| 373 | if (renamePairs.length > 0) { | |
| 374 | const entries = renamePairs.map(([orig, id]) => `${JSON.stringify(orig)}: ${id}`).join(', ') | |
| 375 | bundled += `\nvar __stxScopeRenames = { ${entries} };` | |
| 376 | } | |
| 377 | ||
| 348 | 378 | // Strip any remaining external import statements (stx, @stores etc.) |
| 349 | 379 | // These will be handled by transformAutoImports and transformStoreImports |
| 350 | 380 | // Keep them as-is — the existing transforms expect to find them |
| @@ -298,6 +298,11 @@ function transformSignalScript(scriptContent: string, scopeId: string): string { | ||
| 298 | 298 | if (!window.stx._scopes) window.stx._scopes = {}; |
| 299 | 299 | var __scopeRegistration = { __destroyCallbacks: __destroyHooks }; |
| 300 | 300 | ${scopeAssign} |
| 301 | // #1767: when the bundler renamed a component const that collided with an | |
| 302 | // inlined import (foo -> foo2), rebind the ORIGINAL template name to the | |
| 303 | // renamed declaration so it resolves to the component's value, not the | |
| 304 | // shadowing import. __stxScopeRenames is emitted by bundleClientScript. | |
| 305 | try { if (typeof __stxScopeRenames !== 'undefined' && __stxScopeRenames) { for (var __stxRK in __stxScopeRenames) __scopeRegistration[__stxRK] = __stxScopeRenames[__stxRK]; } } catch (e) { /* no renames */ } | |
| 301 | 306 | window.stx._scopes['${scopeId}'] = __scopeRegistration; |
| 302 | 307 | ` |
| 303 | 308 | // Set __STX_CURRENT_ELEMENT__ to this scope's root element while the body runs, |
| @@ -1859,7 +1859,10 @@ else if (name === '@text' || name === ':text' || name === 'x-text') { | ||
| 1859 | 1859 | } |
| 1860 | 1860 | else if (name === '@html' || name === ':html' || name === 'x-html') { |
| 1861 | 1861 | effect(() => { |
| 1862 | el.innerHTML = evalAttrExpr(value); | |
| 1862 | // Coerce null/undefined to '' so a missing/unresolved binding renders | |
| 1863 | // nothing rather than the literal string "undefined" (#1767). | |
| 1864 | var __html = evalAttrExpr(value); | |
| 1865 | el.innerHTML = (__html == null) ? '' : __html; | |
| 1863 | 1866 | }); |
| 1864 | 1867 | el.removeAttribute(name); |
| 1865 | 1868 | } |
| @@ -0,0 +1,58 @@ | ||
| 1 | /** | |
| 2 | * Tests for the import/local name-shadow collision (stacksjs/stx#1767). | |
| 3 | * | |
| 4 | * When a `<script client>` imports from a module AND declares a top-level | |
| 5 | * `const` whose name equals a top-level export of that module, `Bun.build` | |
| 6 | * inlines the import under the bare name and RENAMES the component's local | |
| 7 | * (`items` -> `items2`). The catch-all export the bundler adds to defeat | |
| 8 | * tree-shaking then comes back as `export { items2 as items }`. Pre-fix, | |
| 9 | * that `as` mapping was stripped and discarded, so scope registration | |
| 10 | * re-harvested the bare `items` (now the raw import) and bound the template | |
| 11 | * to the wrong value — a silent misregistration. | |
| 12 | * | |
| 13 | * Fix: capture the rename map before stripping and re-expose it as | |
| 14 | * `var __stxScopeRenames = { "items": items2 }` so the scope registrar can | |
| 15 | * bind the ORIGINAL name to the renamed (correct) binding. | |
| 16 | */ | |
| 17 | import { afterEach, beforeEach, describe, expect, it } from 'bun:test' | |
| 18 | import fs from 'node:fs' | |
| 19 | import path from 'node:path' | |
| 20 | import { bundleClientScript } from '../../src/client-script-bundler' | |
| 21 | ||
| 22 | const TMP = path.join(import.meta.dir, 'temp-shadow') | |
| 23 | ||
| 24 | describe('client-script-bundler: import/local name shadow (#1767)', () => { | |
| 25 | let projectRoot: string | |
| 26 | let templatePath: string | |
| 27 | ||
| 28 | beforeEach(async () => { | |
| 29 | projectRoot = path.join(TMP, `project-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`) | |
| 30 | templatePath = path.join(projectRoot, 'Widget.stx') | |
| 31 | await fs.promises.mkdir(projectRoot, { recursive: true }) | |
| 32 | await Bun.write(path.join(projectRoot, 'data.ts'), `export const items = [{ id: 1, label: 'raw' }]\n`) | |
| 33 | }) | |
| 34 | ||
| 35 | afterEach(async () => { | |
| 36 | if (fs.existsSync(TMP)) | |
| 37 | await fs.promises.rm(TMP, { recursive: true, force: true }) | |
| 38 | }) | |
| 39 | ||
| 40 | it('re-exposes a rename map when a local const shadows an inlined import export', async () => { | |
| 41 | // The component aliases the import (`allItems`) yet still declares a local | |
| 42 | // `const items` that collides with data.ts's `export const items`. | |
| 43 | const script = `import { items as allItems } from './data'\nconst items = derived(() => allItems.map(x => ({ ...x, extra: 'computed' })))` | |
| 44 | const out = await bundleClientScript(script, templatePath, { projectRoot }) | |
| 45 | ||
| 46 | // Bun renamed the component's local: | |
| 47 | expect(out).toMatch(/\bitems2\b/) | |
| 48 | // ...and the fix re-exposes ORIGINAL name -> renamed binding, so the | |
| 49 | // registrar binds the template's `items` to the derived, not the import: | |
| 50 | expect(out).toMatch(/__stxScopeRenames\s*=\s*\{\s*"items":\s*items2\s*\}/) | |
| 51 | }) | |
| 52 | ||
| 53 | it('emits no rename map when there is no name collision', async () => { | |
| 54 | const script = `import { items as allItems } from './data'\nconst list = derived(() => allItems.map(x => x))` | |
| 55 | const out = await bundleClientScript(script, templatePath, { projectRoot }) | |
| 56 | expect(out).not.toContain('__stxScopeRenames') | |
| 57 | }) | |
| 58 | }) | |