ReviewOS

stacks/stx

checkLayoutChange never fires on a cold click: navigating across layout GROUPS fragment-swaps, so a page renders with the previous group's chrome

#1833
Closed glennmichael123 opened this 14 days ago · 3 comments
14 days ago

The router emits X-STX-Layout-Group, stamps <meta name="stx-layout">, and implements checkLayoutChange to force a full reload when the group changes. On a cold click (nothing prefetched) the check does not fire, so the destination is fragment-swapped into the previous group's shell and renders with the wrong chrome.

Reproduction

Two pages in different groups:

  • /compare/sentry → X-STX-Layout: layouts/marketing, X-STX-Layout-Group: marketing (shell includes SiteNav + SiteFooter)
  • /pricing → X-STX-Layout: layouts/default, X-STX-Layout-Group: default (shell includes neither)

Both are served correctly — headers and <meta name="stx-layout"> are right on each hard load.

Click the in-page link from /compare/sentry to /pricing, then measure whether the document was replaced:

// before the click
window.__m = 1
const t0 = performance.timeOrigin
// after the click settles
performance.timeOrigin === t0   // true  → same document
window.__m === 1                // true  → same document
location.pathname               // "/pricing"
document.querySelector('meta[name="stx-layout"]').content  // "layouts/default"

Both signals say same document: it was a fragment swap. Expected a full document load, because the group changed marketing → default.

Result: /pricing is displayed inside the marketing shell — site nav and footer present — while a hard load of /pricing has neither. The same URL renders differently depending on how the user arrived, and the stx-layout meta now claims layouts/default on a document whose chrome is layouts/marketing.

Measured on the real app: hard-loaded /pricing has 2 data-stx-link anchors and 0 footer links; /compare has 76 and 27. After the SPA hop, /pricing has the marketing set.

Where it looks wrong

packages/router/src/client.ts:497

if(checkLayoutChange(layoutCache[targetPath],url,layoutGroupCache[targetPath])){

The guard is called with values from the prefetch cache. On a cold click those entries are undefined, so the decision is made before the response that would carry X-STX-Layout-Group has been read. checkLayoutChange itself (client.ts:416-434) is correct and does compare groups:

if(curGroup!==nextGroup){
  log('[router] layout group change:',curGroup,'→',nextGroup,'— full reload to:',targetUrl);
  return true;
}

so the logic is fine — it is being asked too early, with nothing to compare against.

Suggested fix

Re-check after the response headers arrive, not only before the fetch: the fragment path already reads X-STX-Layout / X-STX-Layout-Group (client.ts:531). If the group differs from getCurrentLayoutGroup() at that point, discard the fragment and fall back to location.assign(url) — the same handoff #1809 added for "fragment needs a runtime this page lacks". Warming layoutGroupCache on prefetch only narrows the window; a first click into an unvisited group is the common case.

Environment

Fork at f539929dfe, rebuilt dist, driven headlessly over CDP against a real app. Content and title were correct after the swap — the defect is confined to chrome and to the layout meta.

14 days ago

I picked this up to fix and could not reproduce it on current main (3525160823). Reporting the measurement rather than writing a fix for something that already behaves correctly.

Same app as the report, rebuilt dist for both packages/stx and packages/router, driven headlessly over CDP.

The exact reported hop, plus two controls:

navigationgroupresult
/compare/sentry → /pricingmarketing → defaultFULL_RELOAD
/ → /pricingmarketing → defaultFULL_RELOAD
/compare → /compare/sentrymarketing → marketingSPA

So a group change takes the document path and a same-group hop still fragment-swaps — which is the intended split, and the third row matters: the fix did not simply turn SPA navigation off.

The user-visible half — the same URL rendering differently by arrival path — is also gone. Full-page screenshots of /pricing are byte-identical hard-loaded versus arrived-at (63d8627e1053, 64582B both). Chrome counts on a hard load are /pricing 1 data-stx-link, 0 nav, 0 footer against /compare/sentry 62/2/1, and after the hop /pricing has its own set, not the marketing one.

Why it likely already works. The report's suggested fix — re-check after the response headers arrive rather than only against the prefetch cache — is present at packages/router/src/client.ts:565:

var newGroup = r.headers.get('X-STX-Layout-Group') || '';
…
if (isFragment && checkLayoutChange(newLayout, url, newGroup)) {
  // fetch the FULL page and do a document swap

That is the post-header guard, keyed on the header rather than layoutGroupCache[targetPath]. I have not bisected which commit made this effective — 260c4ce4, 338e4baed and 42128d8d3 all touched client.ts after f539929dfe — so I can't say whether it was added, or was always there and something downstream (the document path discarding the destination's runtime, fixed in 260c4ce4) was making it look like a fragment swap.

Worth noting one environmental difference from the report: on this app no X-STX-Layout-Group HTTP header is emitted on a plain GET at all — only the <meta name="stx-layout-group"> tag, which is correct on both pages. The header appears to exist only on the fragment path. If the original measurement was taken somewhere the header is emitted more broadly, that difference may matter to whoever confirms this.

Suggest closing unless someone can still reproduce; happy to dig further if so.

14 days ago

Closing as fixed — verified on 3525160823 rather than assumed.

A group change now takes the document path (/compare/sentry → /pricing and / → /pricing both FULL_RELOAD, marketing → default), while a same-group hop still fragment-swaps (/compare → /compare/sentry SPA) — so SPA navigation is intact, not switched off to make the symptom go away.

The user-visible half is gone too: /pricing renders byte-identically hard-loaded versus arrived-at (63d8627e1053, 64582B both), where the report had it inheriting the marketing shell.

Measurement and the caveats are in the comment above — notably that I did not bisect which commit made the post-header guard at client.ts:565 effective. Reopen if it still reproduces anywhere; happy to pick it back up.

14 days ago

Fixed in 35c13b7 — the symptom is exactly as you describe, but the mechanism is the opposite of the one in the report, and that changes where the fix goes.

checkLayoutChange does fire

On a cold cross-group click the post-response check reads X-STX-Layout-Group, returns true, and the router pays for a second full-document fetch. I instrumented it, and it was independently reproduced in real headless Chrome:

[router] layout check: current=(app) new=default(default)
[router] layout group change: app → default
[router] layout change — fetching full page for document swap
[router] full page fetched: 200

The line you cite (client.ts:526) is behind if(o.cache && cache[targetPath]) and is genuinely unreachable on a cold click — and the post-response re-check your fix asks for already exists at client.ts:565 and already runs. Warming the cache does not help because that guard already returns true: defaultLayoutGroup('') is 'app', which differs from 'marketing', so empty cache entries make it more likely to fire, not less.

What actually breaks

swap() then re-derives the same decision from <meta> tags (client.ts:921-931), gets the opposite answer, and takes the container-only branch. Note what is missing from that log: full body swap for layout change never appears. The router fetches the whole document and then keeps only its <main>.

So the destination's content lands inside the previous page's chrome, with no reload — which is your 76/27 anchors instead of 2/0.

The server's answer now wins inside swap().

Two sources, and they diverge easily

  • the bun-plugin serve path calls a layout-less page default while the client's fallback calls it app — same condition, two strings;
  • the i18n path rewrites every page's group to i18n:<locale>;
  • fixed here too: process.ts guarded on includes('<head') but replaced the literal '<head>', so a layout opening <head prefix="og: …"> passed the guard and got no layout metas at all. The same file already spells that as a regex elsewhere.

One note on the evidence

performance.timeOrigin being unchanged proves only that no browser navigation happened — which is equally true of the correct full-document swap, since it replaces the body in place. So it could not distinguish a fragment swap from a working body swap. Your functional observation was right; that particular signal just could not tell you which one you were looking at.

Verification

Both failing shapes (neither document carrying metas; metas contradicting headers) go from keeping the old chrome to replacing it. A same-group navigation still fragment-swaps and still keeps its chrome, and a server sending no layout headers still falls back to the metas. The two behavioural tests fail at the parent commit; the three regression guards pass there. Full suite 10741 pass.

Two things found alongside, not fixed here

  1. The report's headline is literally true in one configuration I did not fix: with a custom container (anything other than main / [data-stx-content]), shouldUseFragmentResponse() is false, so isFragment is false, so isFragment && checkLayoutChange(...) short-circuits and the check really never runs. Different cause from the one you gave, and worth its own issue if you use a custom container.
  2. Under stx dev specifically, a page appears to get either the router or the signals runtime but never both (injectRouterScript bails on a data-stx-router substring that the signals runtime itself contains). That would make SPA navigation inoperative there entirely — I have not verified it independently, but it is worth checking before anyone tries to reproduce #1833 on that server.

Sign in to comment on this issue.