ReviewOS

stacks/stx

fix: crosswind bridge drops a `theme` written without `extend`, and the serve path merges it a third way

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

Summary

generateCrosswindCSS rebuilds the crosswind config instead of passing the project's through. It reads only theme.extend and discards every other theme key, then pins the reconstructed theme after the ...userConfig spread — so a project that writes theme: { colors: { … } } the Tailwind-classic way (no extend) has its entire theme vanish with no error and no warning. content is pinned the same way, which #1822 closed as intentional but left as a typed, documented key a project can spend real effort on for nothing. And bun-plugin's serve path merges theme a third, incompatible way, so the same config file means different things depending on which binary rendered the page.

Impact (real app)

analyticshq's config/crosswind.ts:6-20 declares four content globs and spends eleven lines of comment reasoning about them, including auditing which globs matched zero files:

//   // Both of these are generated, gitignored directories — they exist after the
//   // framework has run, not in a fresh checkout. Verified to resolve: 11 view
//   // templates and 194 component templates.
//   …
//   // A glob that silently matches zero files is worse than no glob: it reads as
//   // coverage. If error views ever ship utility classes again, add the real path
//   // back rather than restoring this one.

None of it runs on any stx render path. It is maintained, reasoned-about configuration with no effect — and it is the first file anyone opens when a utility class fails to generate.

The theme half is worse because it is invisible until it bites. The app was spared purely by accident: config/crosswind.ts:27-39 happens to spell its eight design tokens under theme.extend.colors. Written as theme: { colors: { panel: 'var(--panel)', … } } — which is valid against crosswind's own Theme type, where extend is optional (crosswind packages/crosswind/src/types.ts:123-136) — all eight tokens would be dropped on the floor and every bg-panel / text-text-2 / border-border in the app would generate nothing, with no diagnostic anywhere.

Evidence in stx

packages/stx/src/dev-server/crosswind.ts (v0.2.155, origin/main 1c053f2):

653  const baseTheme = baseConfig.theme || {}
654  const userTheme = userConfig.theme || {}
655  const userExtend = (userTheme as any).extend || {}
656
657  // Build theme with extend preserved — CSSGenerator's constructor deep-merges extend
658  const theme: Record<string, any> = { ...baseTheme }
659  if (Object.keys(userExtend).length > 0) {
660    theme.extend = userExtend
661  }
…
692  const crosswindConfig: CrosswindConfig = {
693    ...baseConfig,
694    ...userConfig,
695    content: [],
696    output: '',
697    theme,
698    safelist,
699    ...(preflights.length > 0 && { preflights }),
700  }

Two separate losses in one object literal:

  1. theme (:658-661) is baseTheme plus userExtend and nothing else. userTheme.colors, .screens, .fontFamily, .spacing, .borderRadius, .boxShadow never make it in.
  2. theme is then pinned at :697, after ...userConfig at :694, so the user's own theme object cannot survive the spread either. The two together make a non-extend theme unreachable by any route.

This is the module every render path uses: packages/stx/src/production-builder.ts:275, packages/stx/src/ssg.ts:687, packages/stx/src/render.ts:409, packages/stx/src/site-builder/build.ts:144, packages/stx/src/process.ts:650.

Meanwhile packages/bun-plugin/src/serve.ts:1717-1719 does something different again:

if (userConfig.theme) {
  generatorConfig.theme = { ...(generatorConfig.theme || {}), ...userConfig.theme }
}

A shallow spread, so here a user theme.colors replaces the entire stock palette rather than being ignored — the opposite failure mode from the dev-server path, from the same config file. serve.ts:1698-1719 also reads only safelist, shortcuts and theme, so darkMode, preflights, cssVariables, rules, blocklist and variants are dropped on that path entirely.

On content: #1822 closed with the rationale now at :680-691 ("classes are collected by scanning the rendered page, not by globbing … They are pinned AFTER the user spread on purpose"). That reasoning is sound. What is left over is that content remains a required, type-checked key on CrosswindConfig (crosswind packages/crosswind/src/types.ts:89) that stx accepts and silently ignores — which #1822 itself named as "the worst of the three" options.

Prior art

  • Tailwind warns on unrecognised top-level config keys, and its Vite/PostCSS plugins fail loudly on conflicting options rather than silently winning.
  • Nuxt merges module options and surfaces the effective result through nuxi info / nuxt.config inspection; a module that overrides a user option logs it.
  • Laravel php artisan config:show <ns> prints the merged config that is actually in force, so "what I wrote" vs "what is running" is one command away.
  • Vite errors on define/resolve.alias conflicts between a plugin and user config instead of silently preferring one.

Ask

  1. Deep-merge the user's theme rather than reading only theme.extend, so theme: { colors: { … } } written without extend reaches CSSGenerator. Preserve the existing extend semantics on top.
  2. Factor the merge into one shared function used by both packages/stx/src/dev-server/crosswind.ts and packages/bun-plugin/src/serve.ts, so a config cannot mean two different things depending on which path rendered the page. Today one ignores a non-extend theme and the other lets it clobber the stock palette.
  3. Carry the remaining CrosswindConfig keys (darkMode, preflights, cssVariables, rules, blocklist, variants) through the bun-plugin serve path, which currently reads only three.
  4. Warn once at config load for every key stx deliberately overrides (content, output) when the project actually set it — naming the key and the reason — so a documented option cannot silently do nothing.
  5. Or honour content by unioning globbed classes with the classes scanned from the rendered page. That would also pick up classes that only exist in code, which the rendered-page scan cannot see.

Related: #1822

14 days ago

Fixed in 1514556648 — both asks.

mergeCrosswindConfig() (packages/stx/src/crosswind-config.ts) is now the only merge, used by dev-server/crosswind.ts and bun-plugin/src/serve.ts. The serve path also stops dropping darkMode, preflights, cssVariables, rules, blocklist and variants, stops pinning preflight/minify literals (matching what #1822 fixed on the other path), and now honours a base-config safelist it previously ignored.

On ask 1 — one deliberate deviation. Theme keys deep-merge rather than replace, so a non-extend theme is additive. That's not Tailwind's semantic, and the reasoning is worth stating: Tailwind's replace-on-non-extend exists because generation is driven by globbing source files, where an unused default color costs real bytes. Here generation is driven by scanning the rendered page — a color nothing references generates nothing — so keeping the base palette costs nothing, while replacing would mean adding one token to theme.colors silently kills bg-red-500 across the app. Given the two failure modes this issue documents, additive is the one that fails safe. extend is passed through untouched for CSSGenerator to apply on top, so existing configs are unaffected.

Verified against exactly the shape you flagged as the near-miss — theme: { colors: { panel: 'var(--panel)', 'text-2': 'var(--text-2)' } } with no extend. One note for anyone reproducing this: the module has a disk cache keyed on classes + config fingerprint, and here the code differs while the config doesn't, so a before/after comparison silently reads the other run's output unless you clear .stx/ between them. First attempt at this repro showed 'works at HEAD' for that reason.

HEAD    .bg-panel  ABSENT                                    (3029 bytes)
fixed   .bg-panel { background-color: var(--panel) }         (3120 bytes)
        .text-text-2 present, .bg-red-500 still present

On content: left pinned, per #1822's rationale. Your point that it remains a typed key a project can spend real effort on for nothing still stands — I'd rather address that as its own change (warn when a non-empty content is supplied, so the eleven lines of glob reasoning in config/crosswind.ts get told they're inert) than fold a diagnostic into this merge. Happy to do that under a separate issue if you want it.

Covered by packages/stx/test/crosswind-config-merge.test.ts (14 tests). Full suite: 11059 pass, 1 fail — crosswind-arbitrary-values, which is pre-existing and unrelated (an arbitrary slash-opacity color, rgb(242 242 244 / 0.9), that the installed engine doesn't emit).

Sign in to comment on this issue.