ReviewOS

stacks/stx

bundleFrameworkComposables globs *.ts and so reads only .d.ts from dist — nothing is ever bundled in a released stx (likely the mechanism behind #1806)

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

bundleFrameworkComposables (packages/stx/src/framework-composables.ts) globs *.ts next to itself:

const dir = path.join(import.meta.dir, 'composables')      // :97
const glob = new Bun.Glob('*.ts')                          // :109
files = files.filter(f => !f.endsWith('index.ts')).sort()  // :116

When stx runs from src/ that finds the composable sources. When it runs from dist/ — which is every consumer, always — it finds only declaration files:

packages/stx/dist/composables:
  .js                    37
  .d.ts                  37
  plain .ts (non-.d.ts)   0

So every file the bundler processes is type-only and contributes no runtime code. Two things then compound:

  1. The barrel exclusion misses. 'index.d.ts'.endsWith('index.ts') is false, so index.d.ts — the one file the filter on :116 exists to skip — is the one that gets processed.

  2. The export strip corrupts re-exports. :143 does code.replace(/^export\s+(default\s+)?/gm, ''), which turns

    export {
      useStorage,
      type StorageType,
    } from './use-storage'

    into { useStorage, type StorageType, } from './use-storage' — not valid input, so transformSync throws.

Net effect: nothing is ever emitted, so every requested name lands in missing and users get the warning at :158 telling them their code will break:

[stx] framework composable index.d.ts could not be bundled: 32 |   type StorageType,
error: Expected ";" but found "StorageType"
    at input.ts:32:8
[stx] could not bundle notify from @stacksjs/stx/composables — calls to them will throw
ReferenceError in the browser.

That is from a plain buddy build of a Stacks app against a normal built stx.

Why this matters

This looks like the mechanism behind #1806. Composables that live only in @stacksjs/stx/composables and are not mirrored into the signals runtime can never be bundled from a released stx — not because of anything about the composable, but because the bundler is reading .d.ts files. Anything that does work today (useColorMode, useDark, useLocalStorage, …) works because it is in the signals runtime on window.stx, i.e. it never needed this path.

It is also invisible in the stx repo's own test runs, since running from src/ is the one configuration where the glob finds real code.

Suggested fix

  • Glob *.js when running from a built layout (or detect: prefer *.ts excluding *.d.ts, and fall back to *.js). Excluding .d.ts explicitly is worth doing regardless — a declaration file can never contribute runtime code.
  • Make the barrel exclusion extension-agnostic (path.basename(f).replace(/\.d\.ts$|\.[jt]s$/, '') !== 'index').
  • Replace the regex export-stripping with transpiler.scan() + a real transform, or at minimum skip files whose stripped form fails to parse before concluding the exports are missing — right now a parse failure and a genuinely absent export produce the same user-facing message.
  • A test that exercises this against dist/ rather than src/ would have caught it; every current path runs from source.

The missing warning does not account for a name the calling file defines itself. In my app resources/views/accounts.stx:223 has its own const notify = (message, tone, options) => …, and stx still warned that notify could not be bundled and would ReferenceError. Local definitions shadow the auto-import, so that instance is a false alarm — worth filtering out, since a confident warning about working code costs trust (cf. #1815).

10 days ago

Fixed in 68304ba. Reproduced exactly as reported, including the verbatim error, against the built package in this repo:

=== DIST (what consumers run) ===
referenced: [ "notify" ]
[stx] framework composable index.d.ts could not be bundled: 32 |   type StorageType,
error: Expected ";" but found "StorageType"
bundled: (null — NOTHING emitted)

=== SRC  (what stx tests run) ===
referenced: [ "notify" ]
bundled: 4080 chars

Same input, same function, opposite outcome — which is the whole issue in two lines.

What changed

File selection moved into one exported function that handles all three of your points together: .d.ts excluded outright, barrel exclusion extension-agnostic (index.js / index.d.ts / index.ts), glob widened to *.{ts,js} with source preferred if a directory ever holds both.

The export handling is fixed rather than worked around. Re-export statements are dropped outright — the names they mention are declared in the sibling modules the same loop visits — and the export prefix is stripped only where a declaration follows, so nothing else can be decapitated:

code = code.replace(/^export\s+\*\s+(?:as\s+\w+\s+)?from\s*['"][^'"]+['"]\s*;?\s*$/gm, '')
code = code.replace(/^export\s+(?:type\s+)?\{[\s\S]*?\}\s*(?:from\s*['"][^'"]+['"]\s*)?;?\s*$/gm, '')
code = code.replace(/^export\s+default\s+/gm, '')
code = code.replace(/^export\s+(?=(?:declare\s+)?(?:abstract\s+)?(?:async\s+)?(?:function|class|const|let|var|type|interface|enum)\b)/gm, '')

Against the real dist/composables: 74 files globbed → 36 selected, none .d.ts, no barrel, and notify bundles at 4015 chars.

On your testing point

You were right that the test gap is the bug — every existing path runs from source. The composables directory is now a parameter defaulting to the same place, so the selection is exercised against a dist-shaped fixture (compiled .js beside .d.ts, plus a declaration-only barrel with exactly the export { …, type X } from './y' shape that used to throw).

That parameter also introduced a bug, and the new tests caught it by poisoning an unrelated suite. The cache key was the requested names alone, so a bundle built from my temp fixture was served to a later test rendering a real page. The directory is part of the key now — which was always correct, but only became reachable once the directory could vary.

Fixed. A name the page declares itself (const notify = …) no longer warns. A local definition shadows the auto-import, so that was a confident claim that working code would throw — and as you say, that costs more trust than it saves.

On #1806

Consistent with your read, but I have not verified it end to end, so I have not closed it. The mechanism is certainly capable of causing it: anything that appears to work today does so because it lives in the signals runtime on window.stx and never needed this path.

Note the fix only reaches consumers once packages/stx/dist is rebuilt — worth confirming on the next release, since a partial rebuild is its own trap (#1814).

Full suite 10680 pass in a clean worktree; the one failure is the pre-existing crosswind-arbitrary-values case.

Sign in to comment on this issue.