ReviewOS
Reload

also looking at this

stacks/stx

fix(serve): render every request when cache is false

#2106
Open mkantautas wants to merge fix/serve-cache-option into main
2 files +109 -4

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/stx/src/serve.tsmodified+11-4
@@ -150,6 +150,11 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
150150 */
151151 const fileCache = new Map<string, { content: string, mtime: number, status?: number, headers?: Record<string, string> }>()
152152
153 /** `cache: false` in the stx options turns this cache off. Read per use: the config can reload. */
154 function useFileCache(): boolean {
155 return stxOptions.cache !== false
156 }
157
153158 /**
154159 * Process a stx file
155160 */
@@ -158,7 +163,7 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
158163 const cacheKey = `${filePath}:${stats.mtimeMs}`
159164
160165 // Check cache
161 const cached = fileCache.get(cacheKey)
166 const cached = useFileCache() ? fileCache.get(cacheKey) : undefined
162167 if (cached) {
163168 return { content: cached.content, status: cached.status, headers: cached.headers }
164169 }
@@ -262,7 +267,8 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
262267 const headers = readResponseHeaders(context)
263268
264269 // Cache result
265 fileCache.set(cacheKey, { content: output, mtime: stats.mtimeMs, status, headers })
270 if (useFileCache())
271 fileCache.set(cacheKey, { content: output, mtime: stats.mtimeMs, status, headers })
266272
267273 return { content: output, status, headers }
268274 }
@@ -275,7 +281,7 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
275281 const cacheKey = `${filePath}:${stats.mtimeMs}`
276282
277283 // Check cache
278 const cached = fileCache.get(cacheKey)
284 const cached = useFileCache() ? fileCache.get(cacheKey) : undefined
279285 if (cached) {
280286 return cached.content
281287 }
@@ -284,7 +290,8 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
284290 const { content } = await readMarkdownFile(filePath, stxOptions)
285291
286292 // Cache result
287 fileCache.set(cacheKey, { content, mtime: stats.mtimeMs })
293 if (useFileCache())
294 fileCache.set(cacheKey, { content, mtime: stats.mtimeMs })
288295
289296 return content
290297 }
packages/stx/test/serve/serve-cache-option.test.tsadded+98-0
@@ -0,0 +1,98 @@
1/**
2 * `cache: false` and the programmatic `serve()` page cache.
3 *
4 * `serve()` cached every rendered page under the page file's path and mtime
5 * and never read the `cache` option, so `cache: false` in the stx options
6 * (or the project's stx.config.ts) changed nothing: a page whose server script
7 * computes something per request kept returning its first render, and a page
8 * whose partial changed kept serving the old one until the page file itself
9 * changed.
10 */
11import { afterAll, beforeAll, describe, expect, it } from 'bun:test'
12import fs from 'node:fs'
13import path from 'node:path'
14import { serve } from '../../src/serve'
15
16const DIR = path.join(import.meta.dir, 'fixtures-cache-option')
17const PAGES = path.join(DIR, 'pages')
18const PARTIALS = path.join(DIR, 'partials')
19
20const servers: Array<Awaited<ReturnType<typeof serve>>> = []
21
22async function start(stxOptions: Record<string, unknown> = {}, configDir = DIR): Promise<(page: string) => Promise<string>> {
23 const server = await serve({ port: 0, root: PAGES, configDir, watch: false, stxOptions: { partialsDir: PARTIALS, ...stxOptions } })
24 servers.push(server)
25 const base = server.url.replace(/\/$/, '')
26 return async (page: string) => {
27 const response = await fetch(`${base}/${page}`)
28 const html = await response.text()
29 const main = html.match(/<main>([\s\S]*?)<\/main>/)?.[1]
30 if (response.status !== 200 || main === undefined)
31 throw new Error(`/${page} answered ${response.status}: ${html.slice(0, 200)}`)
32 return main
33 }
34}
35
36/** The page's render count, failing on anything that is not one. */
37async function renders(get: (page: string) => Promise<string>): Promise<number> {
38 const count = Number(await get('counter'))
39 expect(Number.isInteger(count)).toBe(true)
40 return count
41}
42
43beforeAll(async () => {
44 await fs.promises.mkdir(PAGES, { recursive: true })
45 await fs.promises.mkdir(PARTIALS, { recursive: true })
46 await Bun.write(path.join(PARTIALS, 'note.stx'), '<p>v1</p>')
47 await Bun.write(path.join(PAGES, 'with-partial.stx'), `<main> /var/www/reviewos-reviewos/releases/3e057f9/resources/views/layouts/app.stx:3878:34265: include error: Error loading include file note: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/note.stx' ╭──────────────────────────────────────────────────────────────╮ │  ERROR  Include Error │ ╰──────────────────────────────────────────────────────────────╯ File: app.stx (line 3878, col 34265) Message: Error loading include file note: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/note.stx' Context:  3876:
packages/stx/src/serve.tsmodified+11-4
@@ -150,6 +150,11 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
150150 */
151151 const fileCache = new Map<string, { content: string, mtime: number, status?: number, headers?: Record<string, string> }>()
152152
153 /** `cache: false` in the stx options turns this cache off. Read per use: the config can reload. */
154 function useFileCache(): boolean {
155 return stxOptions.cache !== false
156 }
157
153158 /**
154159 * Process a stx file
155160 */
@@ -158,7 +163,7 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
158163 const cacheKey = `${filePath}:${stats.mtimeMs}`
159164
160165 // Check cache
161 const cached = fileCache.get(cacheKey)
166 const cached = useFileCache() ? fileCache.get(cacheKey) : undefined
162167 if (cached) {
163168 return { content: cached.content, status: cached.status, headers: cached.headers }
164169 }
@@ -262,7 +267,8 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
262267 const headers = readResponseHeaders(context)
263268
264269 // Cache result
265 fileCache.set(cacheKey, { content: output, mtime: stats.mtimeMs, status, headers })
270 if (useFileCache())
271 fileCache.set(cacheKey, { content: output, mtime: stats.mtimeMs, status, headers })
266272
267273 return { content: output, status, headers }
268274 }
@@ -275,7 +281,7 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
275281 const cacheKey = `${filePath}:${stats.mtimeMs}`
276282
277283 // Check cache
278 const cached = fileCache.get(cacheKey)
284 const cached = useFileCache() ? fileCache.get(cacheKey) : undefined
279285 if (cached) {
280286 return cached.content
281287 }
@@ -284,7 +290,8 @@ export async function serve(options: ServeOptions = {}): Promise<ServeResult> {
284290 const { content } = await readMarkdownFile(filePath, stxOptions)
285291
286292 // Cache result
287 fileCache.set(cacheKey, { content, mtime: stats.mtimeMs })
293 if (useFileCache())
294 fileCache.set(cacheKey, { content, mtime: stats.mtimeMs })
288295
289296 return content
290297 }
3877: > 3878:
packages/stx/test/serve/serve-cache-option.test.tsadded+98-0
@@ -0,0 +1,98 @@
1/**
2 * `cache: false` and the programmatic `serve()` page cache.
3 *
4 * `serve()` cached every rendered page under the page file's path and mtime
5 * and never read the `cache` option, so `cache: false` in the stx options
6 * (or the project's stx.config.ts) changed nothing: a page whose server script
7 * computes something per request kept returning its first render, and a page
8 * whose partial changed kept serving the old one until the page file itself
9 * changed.
10 */
11import { afterAll, beforeAll, describe, expect, it } from 'bun:test'
12import fs from 'node:fs'
13import path from 'node:path'
14import { serve } from '../../src/serve'
15
16const DIR = path.join(import.meta.dir, 'fixtures-cache-option')
17const PAGES = path.join(DIR, 'pages')
18const PARTIALS = path.join(DIR, 'partials')
19
20const servers: Array<Awaited<ReturnType<typeof serve>>> = []
21
22async function start(stxOptions: Record<string, unknown> = {}, configDir = DIR): Promise<(page: string) => Promise<string>> {
23 const server = await serve({ port: 0, root: PAGES, configDir, watch: false, stxOptions: { partialsDir: PARTIALS, ...stxOptions } })
24 servers.push(server)
25 const base = server.url.replace(/\/$/, '')
26 return async (page: string) => {
27 const response = await fetch(`${base}/${page}`)
28 const html = await response.text()
29 const main = html.match(/<main>([\s\S]*?)<\/main>/)?.[1]
30 if (response.status !== 200 || main === undefined)
31 throw new Error(`/${page} answered ${response.status}: ${html.slice(0, 200)}`)
32 return main
33 }
34}
35
36/** The page's render count, failing on anything that is not one. */
37async function renders(get: (page: string) => Promise<string>): Promise<number> {
38 const count = Number(await get('counter'))
39 expect(Number.isInteger(count)).toBe(true)
40 return count
41}
42
43beforeAll(async () => {
44 await fs.promises.mkdir(PAGES, { recursive: true })
45 await fs.promises.mkdir(PARTIALS, { recursive: true })
46 await Bun.write(path.join(PARTIALS, 'note.stx'), '<p>v1</p>')
47 await Bun.write(path.join(PAGES, 'with-partial.stx'), `<main> ╭──────────────────────────────────────────────────────────────╮ │  ERROR  Include Error │ ╰──────────────────────────────────────────────────────────────╯ File: app.stx Message: Error loading include file note: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/note.stx' </main>`)
48 await Bun.write(path.join(PAGES, 'counter.stx'), `<script server>
49const renders = globalThis.__stxServeCacheOptionRenders = (globalThis.__stxServeCacheOptionRenders ?? 0) + 1
50</script>
51<main>{{ renders }}</main>
52`)
53})
54
55afterAll(async () => {
56 for (const server of servers)
57 server.stop()
58 await fs.promises.rm(DIR, { recursive: true, force: true })
59})
60
61describe('serve() with cache: false', () => {
62 it('renders every request', async () => {
63 const get = await start({ cache: false })
64 const first = await renders(get)
65 expect(await renders(get)).toBe(first + 1)
66 })
67
68 it('serves a changed partial on the next request', async () => {
69 const get = await start({ cache: false })
70 expect(await get('with-partial')).toBe('<p>v1</p>')
71 await Bun.write(path.join(PARTIALS, 'note.stx'), '<p>v2</p>')
72 expect(await get('with-partial')).toBe('<p>v2</p>')
73 })
74
75 it('still caches by default', async () => {
76 const get = await start()
77 const first = await renders(get)
78 expect(await renders(get)).toBe(first)
79 })
80
81 it('follows cache in a config that reloads', async () => {
82 const configDir = path.join(DIR, 'reloading-config')
83 const config = path.join(configDir, 'stx.config.ts')
84 await fs.promises.mkdir(configDir, { recursive: true })
85 await Bun.write(config, 'export default { extends: [], cache: false }\n')
86 const get = await start({}, configDir)
87 const first = await renders(get)
88 expect(await renders(get)).toBe(first + 1)
89
90 await Bun.write(config, 'export default { extends: [], cache: true }\n')
91 const cached = await renders(get)
92 expect(await renders(get)).toBe(cached)
93
94 await Bun.write(config, 'export default { extends: [], cache: false }\n')
95 const next = await renders(get)
96 expect(await renders(get)).toBe(next + 1)
97 })
98})
3879: 3880:  </main>`)
48 await Bun.write(path.join(PAGES, 'counter.stx'), `<script server>
49const renders = globalThis.__stxServeCacheOptionRenders = (globalThis.__stxServeCacheOptionRenders ?? 0) + 1
50</script>
51<main>{{ renders }}</main>
52`)
53})
54
55afterAll(async () => {
56 for (const server of servers)
57 server.stop()
58 await fs.promises.rm(DIR, { recursive: true, force: true })
59})
60
61describe('serve() with cache: false', () => {
62 it('renders every request', async () => {
63 const get = await start({ cache: false })
64 const first = await renders(get)
65 expect(await renders(get)).toBe(first + 1)
66 })
67
68 it('serves a changed partial on the next request', async () => {
69 const get = await start({ cache: false })
70 expect(await get('with-partial')).toBe('<p>v1</p>')
71 await Bun.write(path.join(PARTIALS, 'note.stx'), '<p>v2</p>')
72 expect(await get('with-partial')).toBe('<p>v2</p>')
73 })
74
75 it('still caches by default', async () => {
76 const get = await start()
77 const first = await renders(get)
78 expect(await renders(get)).toBe(first)
79 })
80
81 it('follows cache in a config that reloads', async () => {
82 const configDir = path.join(DIR, 'reloading-config')
83 const config = path.join(configDir, 'stx.config.ts')
84 await fs.promises.mkdir(configDir, { recursive: true })
85 await Bun.write(config, 'export default { extends: [], cache: false }\n')
86 const get = await start({}, configDir)
87 const first = await renders(get)
88 expect(await renders(get)).toBe(first + 1)
89
90 await Bun.write(config, 'export default { extends: [], cache: true }\n')
91 const cached = await renders(get)
92 expect(await renders(get)).toBe(cached)
93
94 await Bun.write(config, 'export default { extends: [], cache: false }\n')
95 const next = await renders(get)
96 expect(await renders(get)).toBe(next + 1)
97 })
98})
@endif