ReviewOS

stacks/stx

crosswind config merge: user preflights REPLACES the built-in preflight (unlike safelist/theme), and content/preflight/minify are silently discarded

#1822
Closed glennmichael123 opened this 10 days ago ·
10 days ago

Two defects in how config/crosswind.ts is merged into the generator config, in the same object literal.

packages/stx/src/dev-server/crosswind.ts:669-679:

const crosswindConfig: CrosswindConfig = {
  ...baseConfig,
  ...userConfig,
  content: [],
  output: '',
  preflight: true,
  minify: false,
  theme,
  safelist,
}

1. content, preflight and minify are dead keys

All three are hardcoded after ...userConfig, so whatever the project declared is discarded. A real config declaring five content globs plus preflight: true, minify: false has none of it take effect.

preflight and minify are then ignored a second time — generator.toCSS(true, false) is called with literal arguments at lines 693 and 704 — and the underlying crosswind package spells the flag includePreflight, not preflight, so the key was never going to be read under that name anyway.

Since these are documented, type-checked config keys that silently do nothing, the honest options are to either honour them or remove them from the public config type. Silently accepting and discarding is the worst of the three.

2. preflights is replaced rather than merged, unlike its neighbours

theme and safelist both get explicit merge logic immediately above:

const theme: Record<string, any> = { ...baseTheme }
if (Object.keys(userExtend).length > 0) theme.extend = userExtend

const safelist = [
  ...(baseConfig.safelist || []),
  ...(userConfig.safelist || []),
]

preflights (declared preflights?: unknown[] at line 31) gets none, so it falls through the plain ...userConfig spread and replaces the base preflight wholesale.

The inconsistency is the bug: two of three array/object config keys merge, the third clobbers, and nothing says so. An app adding one preflights entry — the natural way to declare shared CSS custom properties in one place — silently loses crosswind's built-in preflight, i.e. the entire reset.

That is exactly the migration this came up in: consolidating 11 duplicated :root token blocks (7 byte-identical) into a single preflights entry is the obvious fix for the duplication, and doing it the obvious way removes the reset from every page.

safelist two lines above shows the intended shape:

const preflights = [
  ...(baseConfig.preflights || []),
  ...(userConfig.preflights || []),
]

Suggested direction

Merge preflights the way safelist is merged, and either honour content/preflight/ minify or drop them from the config type. A test asserting that a user preflights entry does not remove the base preflight would cover the first; asserting that a user content glob reaches the generator would cover the second.

10 days ago

Both fixed in 4e12377d5b.

Dead keys — I took the "honour them" option of the three you laid out. preflight and minify now reach toCSS(), which as you noted was being called with literal arguments anyway, so they were ignored twice over.

content and output stay stx-owned and pinned after the spread, but now with a comment saying why: classes are collected by scanning the rendered page rather than by globbing, and the CSS is returned rather than written to disk. Honouring them would mean pretending to a contract the dev server doesn't implement — so that's the "remove from the public type" case rather than the "honour" one, and I've documented it in place instead of leaving it looking like an oversight. Say the word if you'd rather they came off the type entirely.

preflights replaced rather than merged — now merged, matching theme and safelist directly above. Your framing was the convincing part: a user preflight is additive by nature, so dropping the built-in base layer because a project added one font-face rule is not what anyone means by "I have a preflight too".

Sign in to comment on this issue.