ReviewOS

stacks/stx

fix(config): two loaders resolve directory keys differently — `loadStxConfig` prefixes them with `root` a second time, `serve()` never does

#1851
Closed glennmichael123 opened this 14 days ago · 1 comment
14 days ago

Summary

stx.config / config/ui.ts is consumed by two independent loaders that disagree about what a relative directory key means. loadStxConfig() runs resolveStxRoot() and then prefixes componentsDir / layoutsDir / partialsDir with root a second time; bun-plugin-stx's serve() reads the same file through bunfig and uses the literal strings. Because root is inferred from the filesystem when it is not set, a perfectly ordinary Stacks layout resolves partialsDir: 'resources/partials' to resources/resources/partials on the build path and to resources/partials on the serve path. The failure is silent at 200 OK — the include just fails and the error text is rendered into the page.

Impact (real app)

analyticshq (config/ui.ts:9-31) carries a 23-line comment block titled "THIS FILE IS READ BY TWO LOADERS THAT DO NOT AGREE", and config/ui.ts:37 pins root: '.' for one reason only:

Because Loader A prefixes and Loader B does not, a relative directory value can only be correct in both when root is '.'. That is why root is pinned below and every path is written project-root-relative.

Before it was pinned, root was inferred as resources from the mere existence of resources/layouts, and every template dir resolved to resources/resources/*. The damage is still visible in the committed build output — dist/index.html (and 5 sibling pages) ship this as page content:

include error: Error loading include file site-nav: ENOENT: no such file or directory,
open '/…/resources/resources/partials/site-nav.stx'

That was 54 failed @include('site-nav'|'site-footer') calls across the marketing pages. Every remaining key in config/ui.ts now carries a note about which of the two loaders it is actually for (Dev/prod-INERT, Render-inert on the live paths, LIVE on every top-level render).

Evidence in stx

Loader A — packages/stx/src/config.ts

  • :407 resolveStxRoot(configRoot, configPagesDir, cwd)
  • :423-425 auto-detects { root: 'resources', pagesDir: 'views' } whenever resources/views and resources/layouts both exist — i.e. on every Stacks app that has not set root
  • :532-534 writes that inferred root back onto the config
  • :535-547 prefixes partialsDir, componentsDir, layoutsDir with it — including values that already contain the root

Loader B — packages/bun-plugin/src/serve.ts

  • :845-853 loadConfig({ name: 'stx', alias: ['ui'], cwd: process.cwd(), defaultConfig: defaultStxConfig, … }) — bunfig's raw object; resolveStxRoot is never called
  • :890-893 const componentsDir = options.componentsDir ?? stxConfig.componentsDir ?? defaultStxConfig.componentsDir (and the same for layouts/partials/public) — the literal string

warnMissingDirs() (config.ts:449-466) is a good mitigation but only fires on Loader A, and only when the doubled path does not happen to exist.

Prior art

Vite's resolveConfig() returns a ResolvedConfig whose root is already absolute and whose directory options are resolved against it exactly once; every plugin receives that object, and no second consumer re-prefixes. Nuxt normalizes nuxt.options.dir / srcDir inside loadNuxtConfig before any module runs. Neither framework has an inference step that can silently double a path.

Ask

  1. Expose a single resolveStxConfig() whose output is the only thing any consumer sees — dev server, prod server, SSG, store loader, composable loader, crosswind — with root absolute and every *Dir normalized to an absolute path.
  2. Make bun-plugin-stx's serve() call that instead of bunfig directly, so there is one resolution pass rather than two.
  3. Until (1) lands: make the prefixing at config.ts:535-547 idempotent — do not prefix a value that already starts with root.
  4. Make the root inference at config.ts:423-425 non-silent: log the inferred value once, since it changes the meaning of every other directory key in the file.

Related: #1792

14 days ago

Fixed in ae486b4 — asks 2, 3 and 4; ask 1 partially, with the remainder called out below.

resolveStxDirectories() (packages/stx/src/config.ts) is now the single resolution pass. loadStxConfig() calls it, and bun-plugin's serve() now calls it on the object bunfig hands back instead of consuming the literal strings — so the two loaders resolve identically by construction rather than by an app pinning root: '.' to make the prefix a no-op.

Ask 3 (idempotent prefixing). A value already under root is left alone, so running the pass twice — which now genuinely happens, since both loaders call it — changes nothing. Verified on your shape:

HEAD    root=resources  partialsDir=resources/resources/partials  componentsDir=resources/resources/components
fixed   root=resources  partialsDir=resources/partials            componentsDir=resources/components

Ask 4 (non-silent inference). Now logged once per (cwd, root): No "root" in config — inferred "resources" from the project layout. Template directories are resolved under it; set root explicitly to pin this. Once per cwd, not per reload, so stx dev doesn't repeat itself.

Also fixed a second-order problem: warnMissingDirs still said the value had been "prefixed with root … a value that already includes the root gets it twice." That's no longer what happened, and a warning that misdescribes the cause sends people to the wrong line.

On ask 1 — the absolute-path half is not done, deliberately. The single-pass and agreement parts are in; normalising every *Dir to an absolute path is a contract change across 219 references in packages/stx/src + packages/bun-plugin/src. Encouragingly it looks tractable: every cross-module consumer already uses path.resolve(projectRoot, dir) (absolute-safe), and only 9 sites use path.join with the dir — all as the first argument, also safe. So it's plausibly a small diff, but it's a behavioural change to a public config contract and I'd rather it not ride along inside a correctness fix. Happy to do it as its own change if you want it — say the word and I'll open one.

For analyticshq: the root: '.' pin and the per-key "which loader is this for" annotations in config/ui.ts should now be unnecessary. Worth re-running the build before deleting the comment block, though — and the 54 include error: strings in the committed dist/ need a rebuild to clear.

Covered by packages/stx/test/config-single-resolution.test.ts (9 tests). One existing test in config-dir-resolution.test.ts asserted the doubling as then-current behaviour and explicitly said un-prefixing "belongs in its own change" — this is that change, so its expectation moved to the un-doubled path.

Sign in to comment on this issue.