ReviewOS

stacks/stx

fix: `definePageMeta({ layout })` is declared and documented but never read — the field is a silent no-op, and `@extends` on a nonexistent layout only warns

#1879
Closed glennmichael123 opened this 14 days ago · 2 comments
14 days ago

Summary

PageMeta.layout is part of the public type and appears in definePageMeta's own documentation example, but no code path reads it. A page that declares definePageMeta({ layout: 'app' }) type-checks, looks like the idiomatic declaration, and renders with no layout at all. Layout selection in practice has exactly three sources — @extends/@layout, an auto-discovered _layout.stx, and defaultLayout — and none of them consult page meta. Separately, naming a layout that does not exist is only a runtime warning, so the same no-layout render is what an author gets from a typo.

Impact (real app)

analyticshq has 33 views on @extends and two pages that reached for a page-meta-style declaration instead. resources/views/dashboard.stx:1067 and resources/views/account.stx:86 both carry:

<meta name="stx-layout" content="app">

That is a hand-forged copy of the marker stx normally emits (process.ts:504-517), written because there was no working typed way to say "this page belongs to the app layout group" — definePageMeta({ layout: 'app' }) compiles and does nothing, so it was never a candidate. resources/layouts/ contains only auth.stx and marketing.stx; the string "app" names a group with no layout file behind it, and neither page uses @extends, so both render unlayouted while asserting a layout to the router.

The consequence we documented in resources/layouts/auth.stx:9-12 while migrating three other pages off the same pattern:

Replaces the hand-written <meta name="stx-layout" content="auth"> that login, register and sites/new each carried. That meta was a hint the pages asserted about themselves; the FILENAME is what actually names a layout group, so the two could disagree and nothing would say so.

The router's full-document-vs-fragment swap decision hinges on that group string (packages/router/src/client.ts:436-440), so a page asserting a group it does not belong to is a real routing hazard, not a cosmetic one.

Evidence in stx

Declared and documented — packages/stx/src/head.ts:

107  export interface PageMeta {
108    title?: string
109    description?: string
111    status?: number
112    layout?: string | false
113    middleware?: string | string[]
118    validate?: (route: { params: Record<string, string> }) => boolean | Promise<boolean>
119    [key: string]: unknown
120  }
340  * definePageMeta({
341  *   title: 'Dashboard',
342  *   layout: 'admin',
343  *   middleware: ['auth', 'admin'],
344  *   keepAlive: true
345  * })
348  export function definePageMeta(config: PageMeta): void {
349    currentPageMeta = { ...currentPageMeta, ...config }

Never read. grep -rn "pageMeta\b" packages/stx/src packages/bun-plugin/src (excluding head.ts) returns only:

  • packages/stx/src/ssg.ts:1006-1015 — pageMeta.middleware only
  • packages/stx/src/dev-server/serve-app.ts:681-714 — pageMeta.validate and pageMeta.middleware only
  • packages/bun-plugin/src/serve.ts:2342 — siteConfig.pages, a different thing entirely

Actual layout selection, packages/stx/src/process.ts:

802  const layoutMatch = output.match(/@(?:layout|extends)\(\s*['"]([^'"]+)['"]\s*\)/)
804    layoutPath = layoutMatch[1]
848    layoutPath = autoLayoutPath          // _layout.stx walked up the tree
857    layoutPath = defaultLayoutFile       // defaultLayout in layoutsDir

layoutPath is never assigned from page meta. And an unresolvable name degrades quietly — process.ts:1034-1043:

if (!layoutFullPath) {
  const warning = `Layout not found: ${layoutPath} (referenced from ${filePath})`
  errorLogger.log(new Error(warning), { layoutPath, filePath }, 'warning')
  if (resolvedOptions.debug) { throw new StxRuntimeError(warning, filePath) }
  return output          // page renders with no layout
}

Prior art

  • Nuxt — definePageMeta({ layout: 'app' }), where layout is typed as a union of the layout files on disk, so a wrong name is a type error before it is a runtime one.
  • Next — nested layout.tsx by directory; there is no second mechanism that can disagree.
  • Blade — @extends only.

Ask

  1. Read PageMeta.layout in the render pipeline: extract it before layout resolution (the same cheap source scan serve.ts:1136-1160 already does for middleware) and let it set layoutPath in process.ts:802-807. Define the precedence explicitly — suggested: @extends > definePageMeta({ layout }) > auto _layout.stx > defaultLayout, with layout: false equivalent to @nolayout.
  2. If it is not going to be implemented, remove layout from PageMeta (head.ts:112) and from the doc example at :340-345. A typed field that silently does nothing is worse than an absent one.
  3. Generate a union type of the layouts on disk so a wrong name is a type error, matching Nuxt.
  4. Make an unresolvable layout name fail the build rather than warn-and-render-unlayouted (process.ts:1034-1043), or at minimum log at error severity with the list of layouts that do exist.
  5. Document that <meta name="stx-layout"> is stx output (emitted at process.ts:504-517) and not an authoring surface — hand-writing it desynchronises the router's layout-group check at packages/router/src/client.ts:436-440.

Related: #1849 (same shape: PageMeta.middleware/validate declared and never read on the client path), #1798 (same shape: htmlAttrs typed and never read — fixed)

14 days ago

Fixed in 1ad461a — asks 1, 4 and 5. Asks 2 and 3 addressed below.

Ask 1. definePageMeta({ layout }) now selects the layout, read from the original template source before layout resolution. Precedence as you suggested: @extends/@layout > definePageMeta({ layout }) > auto _layout.stx > defaultLayout, with layout: false equivalent to @nolayout.

One thing that fell out of implementing it: a page declaring a layout in meta writes a plain body, not @section blocks. Without wrapping it as the content section — the way the auto-layout path already does — the layout resolves and then renders empty, which would have been a subtler version of the same bug. Covered by a test.

HEAD    layout applied: NO   body present: YES
fixed   layout applied: YES  body present: YES

Ask 4 — worse than the issue states. You described the unresolvable name as "only a runtime warning". It wasn't even that: errorLogger.log() buffers in memory and only writes to a file when file logging is enabled, so the Layout not found: … string at process.ts:1034-1043 was never printed anywhere. The only console output was Template not found: aap from the generic resolver in utils.ts — which doesn't say a layout failed, or what the alternatives were. It now logs at error severity and writes to the console, listing the layouts that exist:

[stx] Layout not found: aap (referenced from …/page.stx). Available layouts: app, marketing

I stopped short of failing the build, per your "or at minimum" — that's a behavioural break for anyone currently relying on the quiet degrade, and it's a one-line change once you want it.

Ask 5. Documented on the field itself (head.ts), where someone reaching for it will actually read it: the meta tag is stx output, the router reads it to choose between fragment and full-document swap, and a page asserting a group it doesn't belong to mis-routes rather than mis-styles.

Ask 2 is moot now that ask 1 is implemented.

Ask 3 (generated union of layouts on disk) is not done. It's the right end state and I'd like to do it, but it needs a generated declaration plus module augmentation on PageMeta — the same machinery as the route map, not a local change — and it's a distinct feature from making the field work. Worth its own issue; happy to pick it up.

While in there I also consolidated the extractor: there were four separate regexes over definePageMeta (ssg.ts:1123, ssg.ts:1665, page-response.ts:11, bun-plugin/serve.ts:1194), each recognising a slightly different subset — the same shape as the two-lists-that-disagree problems in #1866 and #1851. New code goes through one extractPageMetaFromSource.

For analyticshq: dashboard.stx:1067 and account.stx:86 can drop the hand-written <meta name="stx-layout"> for definePageMeta({ layout: … }) — but note both currently name 'app', and resources/layouts/ has only auth.stx and marketing.stx. With this change that misnamed layout will now be reported instead of silently rendering unlayouted, so expect it to speak up until the layout exists or the name changes.

Covered by packages/stx/test/page-meta-layout.test.ts (9 tests). Full suite: 11119 pass, 1 pre-existing unrelated failure.

14 days ago

Ask 3 is now done too — 61c0a00. That closes every ask on this issue.

generateLayoutTypes() emits a union of the layouts on disk, generated on dev-server start, when a layout is added or removed, and during SSG builds:

definePageMeta({ layout: 'aap' })
// error TS2322: Type '"aap"' is not assignable to type 'false | keyof KnownLayouts | undefined'

The interesting part is where it augments. The obvious move was to copy the route-type generator's shape — but that generator emits declare module "stx/routes" { interface RouteMap … }, and nothing in the source tree declares or imports stx/routes. So RouteMap is a dangling declaration that cannot make anything a type error: it's the same "typed and never read" problem this issue is about, one level up. Copying it would have produced a file that looks like a fix and checks nothing.

So this augments @stacksjs/stx, where KnownLayouts is really declared and really referenced by PageMeta['layout'].

Two safety properties, both tested:

  • keyof KnownLayouts is never while the interface is empty, which widens LayoutName back to string. Projects that haven't generated the declaration, or haven't added the state directory to their tsconfig, are unaffected.
  • An empty layouts directory emits no augmentation at all rather than an empty interface — otherwise the file would look like a constraint while constraining nothing.

The decisive test compiles a real project against the real package via tsc, since a string-shaped assertion over the emitted text would have passed for the dangling version too. I verified it's non-vacuous by retyping layout back to plain string — the test fails, as it should.

Worth flagging separately: stx/routes / RouteMap is still dangling. Same class of bug as this issue, still live, and it means typed route navigation isn't actually checked today. Happy to open that as its own issue.

Full suite: 11180 pass, 1 pre-existing unrelated failure.

Sign in to comment on this issue.