ReviewOS

also looking at this

stacks/stx

fix(serve): dev no-store caching + nested dynamic route regex

#1731
Merged glennmichael123 wants to merge fix/serve-cache-and-nested-routes into main
2 files +120 -35
packages/bun-plugin/src/serve.tsmodified+57-35
Changes to packages/bun-plugin/src/serve.ts
@@ -32,6 +32,41 @@ import { deriveLayoutGroup } from 'stx-router/layout-metadata'
3232// from the importer's location and a stale copy there beats the vendored one.
3333const defaultStxModule = import('@stacksjs/stx')
3434
35/**
36 * Build the candidate regexes for a dynamic route file (stacksjs/stx#1927).
37 *
38 * `fileRouteBase` is the route path with the file extension already
39 * stripped (e.g. `judges/[id]/profile/index`). Returns one regex per
40 * acceptable URL shape:
41 * - the literal path with `[param]` → `([^/]+)`
42 * - the same with a trailing `/index` removed, so
43 * `judges/[id]/profile/index` also matches `judges/35/profile`
44 * (without this, nested dynamic routes 404 on hard reload)
45 * - both of the above again with a leading `pages/` stripped, for
46 * projects that nest routable views under `pages/`
47 */
48export function buildDynamicRouteRegexes(fileRouteBase: string): RegExp[] {
49 const toPattern = (p: string): string => p
50 .replace(/\[([^\]]+)\]/g, '([^/]+)') // [param] → capture group
51 .replace(/\//g, '\\/') // escape slashes
52
53 const patterns: RegExp[] = [new RegExp(`^${toPattern(fileRouteBase)}$`)]
54
55 const noIndex = fileRouteBase.replace(/\/index$/, '')
56 if (noIndex !== fileRouteBase)
57 patterns.push(new RegExp(`^${toPattern(noIndex)}$`))
58
59 if (fileRouteBase.startsWith('pages/')) {
60 const pretty = fileRouteBase.slice(6)
61 patterns.push(new RegExp(`^${toPattern(pretty)}$`))
62 const prettyNoIndex = pretty.replace(/\/index$/, '')
63 if (prettyNoIndex !== pretty)
64 patterns.push(new RegExp(`^${toPattern(prettyNoIndex)}$`))
65 }
66
67 return patterns
68}
69
3570export interface ServeOptions {
3671 patterns: string[]
3772 port?: number
@@ -305,7 +340,7 @@ export async function bundleBrowserAsset(entrypoint: string): Promise<Response>
305340 status: 500,
306341 headers: {
307342 'Content-Type': 'text/plain; charset=utf-8',
308 'Cache-Control': 'no-cache',
343 'Cache-Control': 'no-store',
309344 },
310345 })
311346 }
@@ -313,7 +348,7 @@ export async function bundleBrowserAsset(entrypoint: string): Promise<Response>
313348 return new Response(await result.outputs[0].text(), {
314349 headers: {
315350 'Content-Type': 'application/javascript',
316 'Cache-Control': 'no-cache',
351 'Cache-Control': 'no-store',
317352 },
318353 })
319354}
@@ -406,6 +441,15 @@ export async function serve(options: ServeOptions): Promise<void> {
406441 signature: Map<string, number>
407442 }
408443 const htmlCache = new Map<string, HtmlCacheEntry>()
444 // Rendered-HTML cache is OFF in the dev server (stacksjs/stx#1926).
445 // `templateSignatureFresh` compares dependency mtimes, but transitive
446 // `@include`s sometimes don't get registered into the dependency set
447 // during `processDirectives` — so editing a deeply-included partial
448 // intermittently served stale HTML on alternating reloads. The dev
449 // server must be the source of truth on every request; a fresh render
450 // is ~50ms, which is a fine price for correctness. Flip this to `true`
451 // only if the dependency-tracking gap is fully closed.
452 const ENABLE_HTML_CACHE = false
409453 // Crosswind-generated CSS cache, keyed by sorted class-set. Lives here for
410454 // the same reason as `htmlCache`: so the watcher can wipe it on a source
411455 // edit and pick up a new utility's CSS the next render.
@@ -1060,7 +1104,7 @@ export async function serve(options: ServeOptions): Promise<void> {
10601104 // render, return the cached HTML. The stat fan-out is ~1ms total for a
10611105 // typical 5-dep page; a fresh render is ~50ms, so even on miss we only
10621106 // pay the stat cost once. Query-driven pages opt out entirely.
1063 if (!skipCacheHint) {
1107 if (ENABLE_HTML_CACHE && !skipCacheHint) {
10641108 const cacheKey = htmlCacheKey(filePath)
10651109 const cachedEntry = htmlCache.get(cacheKey)
10661110 if (cachedEntry && await templateSignatureFresh(cachedEntry.signature))
@@ -1173,7 +1217,7 @@ export async function serve(options: ServeOptions): Promise<void> {
11731217 // `context.__stx_skip_cache = true`). The dependency set was populated
11741218 // by processDirectives — every layout / component / partial it pulled
11751219 // in is now part of the invalidation signature.
1176 if (!context.__stx_skip_cache && !skipCacheHint) {
1220 if (ENABLE_HTML_CACHE && !context.__stx_skip_cache && !skipCacheHint) {
11771221 const signature = await buildTemplateSignature(filePath, dependencies)
11781222 htmlCache.set(htmlCacheKey(filePath), { html: output, signature })
11791223 }
@@ -1323,32 +1367,10 @@ export async function serve(options: ServeOptions): Promise<void> {
13231367
13241368 // Convert dynamic route file to regex pattern
13251369 // e.g., pages/data/[model].stx -> ^pages/data/([^/]+)$
1370 // Build candidate regexes (incl. the `/index`-stripped variant so
1371 // nested dynamic routes resolve on hard reload — stacksjs/stx#1927).
13261372 const fileRouteBase = relativeFilePath.replace(/\.(stx|md|html)$/, '')
1327 const routePattern = fileRouteBase
1328 .replace(/\[([^\]]+)\]/g, '([^/]+)') // Replace [param] with capture group
1329 .replace(/\//g, '\\/') // Escape slashes
1330
1331 // Try matching with and without 'pages/' prefix
1332 const regexPatterns = [
1333 new RegExp(`^${routePattern}$`),
1334 ]
1335 // Also match without trailing `/index` so [id]/profile/index.stx
1336 // resolves /:id/profile (not just /:id/profile/index).
1337 const fileRouteNoIndex = fileRouteBase.replace(/\/index$/, '')
1338 if (fileRouteNoIndex !== fileRouteBase) {
1339 const noIndexPattern = fileRouteNoIndex.replace(/\[([^\]]+)\]/g, '([^/]+)').replace(/\//g, '\\/')
1340 regexPatterns.push(new RegExp(`^${noIndexPattern}$`))
1341 }
1342 if (fileRouteBase.startsWith('pages/')) {
1343 const prettyBase = fileRouteBase.slice(6)
1344 const prettyPattern = prettyBase.replace(/\[([^\]]+)\]/g, '([^/]+)').replace(/\//g, '\\/')
1345 regexPatterns.push(new RegExp(`^${prettyPattern}$`))
1346 const prettyNoIndex = prettyBase.replace(/\/index$/, '')
1347 if (prettyNoIndex !== prettyBase) {
1348 const prettyNoIndexPattern = prettyNoIndex.replace(/\[([^\]]+)\]/g, '([^/]+)').replace(/\//g, '\\/')
1349 regexPatterns.push(new RegExp(`^${prettyNoIndexPattern}$`))
1350 }
1351 }
1373 const regexPatterns = buildDynamicRouteRegexes(fileRouteBase)
13521374
13531375 for (const regex of regexPatterns) {
13541376 const match = normalizedPath.match(regex)
@@ -2068,7 +2090,7 @@ export async function serve(options: ServeOptions): Promise<void> {
20682090 'X-STX-Fragment': 'true',
20692091 'X-STX-Layout': pageLayout,
20702092 'X-STX-Layout-Group': pageLayoutGroup,
2071 'Cache-Control': 'no-cache',
2093 'Cache-Control': 'no-store',
20722094 ...corsHeaders,
20732095 },
20742096 })
@@ -2089,7 +2111,7 @@ export async function serve(options: ServeOptions): Promise<void> {
20892111 return new Response(injectHmrClient(cleaned), {
20902112 headers: {
20912113 'Content-Type': 'text/html; charset=utf-8',
2092 'Cache-Control': 'no-cache',
2114 'Cache-Control': 'no-store',
20932115 ...corsHeaders,
20942116 },
20952117 })
@@ -2101,7 +2123,7 @@ export async function serve(options: ServeOptions): Promise<void> {
21012123 return new Response(injectHmrClient(contentWithExt), {
21022124 headers: {
21032125 'Content-Type': 'text/html; charset=utf-8',
2104 'Cache-Control': 'no-cache',
2126 'Cache-Control': 'no-store',
21052127 ...corsHeaders,
21062128 },
21072129 })
@@ -2118,7 +2140,7 @@ export async function serve(options: ServeOptions): Promise<void> {
21182140 return new Response(buildFile, {
21192141 headers: {
21202142 'Content-Type': contentType,
2121 'Cache-Control': 'no-cache', // Build artifacts change during dev
2143 'Cache-Control': 'no-store', // Build artifacts change during dev
21222144 },
21232145 })
21242146 }
@@ -2166,7 +2188,7 @@ export async function serve(options: ServeOptions): Promise<void> {
21662188 // stale copy from disk cache without revalidating. The build path (in
21672189 // `build-views.ts` / SSG) is what stamps long-lived cache headers on
21682190 // production output; the dev server should never set them.
2169 'Cache-Control': 'no-cache',
2191 'Cache-Control': 'no-store',
21702192 },
21712193 })
21722194 }
@@ -2224,7 +2246,7 @@ export async function serve(options: ServeOptions): Promise<void> {
22242246 // appear on the next normal reload. The previous `max-age=3600`
22252247 // meant up to an hour of stale assets after a swap. Production
22262248 // builds copy public/ to dist/ where a CDN stamps long caching.
2227 'Cache-Control': 'no-cache',
2249 'Cache-Control': 'no-store',
22282250 },
22292251 })
22302252 }
packages/bun-plugin/test/dynamic-routes.test.tsadded+63-0
Changes to packages/bun-plugin/test/dynamic-routes.test.ts
@@ -0,0 +1,63 @@
1import { describe, expect, it } from 'bun:test'
2import { buildDynamicRouteRegexes } from '../src/serve'
3
4/**
5 * Dynamic-route regex construction (stacksjs/stx#1927).
6 *
7 * Nested dynamic routes (e.g. `judges/[id]/profile/index.stx`) used to
8 * 404 on hard reload because the route regex kept the trailing
9 * `/index`, so `^judges/([^/]+)/profile/index$` never matched the URL
10 * `judges/35/profile`. These tests pin the `/index`-stripped variant.
11 */
12
13function matchesAny(fileRouteBase: string, url: string): boolean {
14 return buildDynamicRouteRegexes(fileRouteBase).some(re => re.test(url))
15}
16
17describe('buildDynamicRouteRegexes — single-segment dynamic routes', () => {
18 it('matches /judges/35 from judges/[id]', () => {
19 expect(matchesAny('judges/[id]', 'judges/35')).toBe(true)
20 })
21
22 it('matches /judges/35 from judges/[id]/index', () => {
23 expect(matchesAny('judges/[id]/index', 'judges/35')).toBe(true)
24 })
25})
26
27describe('buildDynamicRouteRegexes — nested dynamic routes (the bug)', () => {
28 it('matches /judges/35/profile from judges/[id]/profile/index', () => {
29 expect(matchesAny('judges/[id]/profile/index', 'judges/35/profile')).toBe(true)
30 })
31
32 it('matches /judges/35/reviews from judges/[id]/reviews/index', () => {
33 expect(matchesAny('judges/[id]/reviews/index', 'judges/35/reviews')).toBe(true)
34 })
35
36 it('matches /court-houses/1/bench from court-houses/[id]/bench/index', () => {
37 expect(matchesAny('court-houses/[id]/bench/index', 'court-houses/1/bench')).toBe(true)
38 })
39
40 it('still matches the index-included URL for back-compat', () => {
41 expect(matchesAny('judges/[id]/profile/index', 'judges/35/profile/index')).toBe(true)
42 })
43
44 it('does NOT match an unrelated path', () => {
45 expect(matchesAny('judges/[id]/profile/index', 'judges/35/cases')).toBe(false)
46 })
47
48 it('captures the dynamic segment, not the static suffix', () => {
49 const [, noIndexRe] = buildDynamicRouteRegexes('judges/[id]/profile/index')
50 const m = 'judges/35/profile'.match(noIndexRe)
51 expect(m?.[1]).toBe('35')
52 })
53})
54
55describe('buildDynamicRouteRegexes — pages/ prefix', () => {
56 it('matches the pages/-stripped URL', () => {
57 expect(matchesAny('pages/judges/[id]/profile/index', 'judges/35/profile')).toBe(true)
58 })
59
60 it('matches the full pages/ path too', () => {
61 expect(matchesAny('pages/data/[model]', 'pages/data/users')).toBe(true)
62 })
63})