ReviewOS

stacks/stx

generateDocumentShell drops htmlAttrs: computed by process.js, typed in head.d.ts, never read

#1798
Closed glennmichael123 opened this 14 days ago · 0 comments
14 days ago

Verified against @stacksjs/stx@0.2.152.

htmlAttrs is a declared-but-unread option. It is typed, it is computed and merged on the render path, and then the function that would emit it silently ignores it.

The three places

Typed — dist/head.d.ts:193

htmlAttrs?: Record<string, string>

Computed — dist/process.js:279, inside the autoShell branch that builds headConfig

...runtimeHead.htmlAttrs && { htmlAttrs: { ...baseHeadConfig.htmlAttrs || {}, ...runtimeHead.htmlAttrs } },

Dropped — dist/document-shell.js, generateDocumentShell

const {
  title: configTitle = "stx App",
  lang = "en",
  meta = [],
  link = [],
  script: headScriptTags = [],
  headRaw = "",
  bodyClass = "",
  bodyAttrs = {}
} = headConfig

No htmlAttrs in the destructure, and the output is hardcoded:

return `<!DOCTYPE html>
<html lang="${lang}">

grep -n htmlAttrs dist/document-shell.js returns nothing. Its sibling bodyAttrs is destructured and emitted, so the asymmetry looks accidental rather than intentional.

Why it bites

It is the blocker for migrating a hand-written layout onto the generated shell, which is the thing the DOCTYPE rules ask you to do.

Concretely, in an app I'm migrating: the marketing layout scopes its entire design-token set to a class on the root element, because two layouts define the same custom properties with different values and the SPA router keeps the entry page's styles alive across navigation. So the tokens live in one Crosswind preflights sheet, gated as:

html.marketing { --bg: #f4f4f2; --accent: #2563eb; /* … */ }

The layout carries <html lang="en" class="marketing …">. Delete the hand-written shell — which is exactly what "never write a document" asks for — and the class is gone, the selector never matches, and the landing page loses its palette, its base font, and its mega-menu CSS in one step. There is no supported way to put a class back on the root element.

The available workarounds are all worse than the rule being fixed:

  • move the scope to body.marketing — but scroll-behavior and the :root-anchored custom properties do not relocate cleanly, and bodyClass is global config rather than per-layout
  • set the class from a pre-paint script — reintroduces exactly the hand-written vanilla JS the standards are trying to delete, to express a static fact
  • keep the DOCTYPE — which is the rule being violated

Suggested fix

Symmetric with the existing bodyAttrs handling:

const { /* … */ htmlAttrs = {}, bodyClass = "", bodyAttrs = {} } = headConfig
const htmlAttrStr = Object.entries(htmlAttrs)
  .filter(([k]) => k !== 'lang')
  .map(([k, v]) => ` ${k}="${v}"`).join("")
// …
return `<!DOCTYPE html>
<html lang="${lang}"${htmlAttrStr}>`

Filtering lang avoids emitting it twice when a caller passes both; alternatively let htmlAttrs.lang win over the lang option.

Happy to send a PR if the shape above is right.

Sign in to comment on this issue.