ReviewOS
Reload

also looking at this

stacks/stx

fix(hmr): send SSE keepalive every 5s so /_stx/hmr survives idle timeouts

#1778
Merged glennmichael123 wants to merge fix/hmr-keepalive-sse into main
3 files +345 -43
packages/bun-plugin/src/serve.tsmodified+168-42
@@ -45,6 +45,40 @@ const defaultStxModule = import('@stacksjs/stx')
4545 * - both of the above again with a leading `pages/` stripped, for
4646 * projects that nest routable views under `pages/`
4747 */
48/**
49 * Snapshot of the in-flight request, threaded from the fetch handler down to
50 * the `<script server>` render (stacksjs/stacks#1967).
51 *
52 * The module-level `activeServe*` singletons are NOT safe to read at render
53 * time: a render can suspend at an `await` (onRequest hooks, DB queries in
54 * server scripts) while a concurrent request — the HMR EventSource opens on
55 * every page load — runs its own handler and resets the singletons in its
56 * `finally`. Threading an immutable per-request snapshot removes that race;
57 * the singletons remain only as a fallback for legacy call sites.
58 */
59export interface ServeRequestContext {
60 /** Full request URL (`req.url`). */
61 url: string
62 /** Normalized request path (no query string). */
63 path: string
64 /** Query string including the leading `?`, or `''`. */
65 search: string
66 /** `Host` header, or `''`. */
67 host: string
68 /** Raw `Cookie` header, or `''`. */
69 cookieHeader: string
70 /** Parsed cookies — always an object. */
71 cookies: Record<string, string>
72 /** Best-effort client IP, or `''`. */
73 ip: string
74 /** Active locale for the SSR pass, when i18n is enabled. */
75 locale: string | null
76 /** Dynamic route params (`[id].stx` → `{ id }`) — set by the dynamic-route renderer. */
77 params: Record<string, string>
78 /** Extra keys merged from a non-Response `onRequest` return (app-owned). */
79 [key: string]: unknown
80}
81
4882export function buildDynamicRouteRegexes(fileRouteBase: string): RegExp[] {
4983 const toPattern = (p: string): string => p
5084 .replace(/\[([^\]]+)\]/g, '([^/]+)') // [param] → capture group
@@ -317,14 +351,24 @@ export interface ServeOptions {
317351 * return await proxyToApi(req)
318352 * }
319353 *
320 * The runtime awaits the return and only short-circuits on a truthy value,
354 * The runtime awaits the return and only short-circuits on a `Response`,
321355 * so both sync and async null/undefined behave identically.
356 *
357 * Returning a plain object instead merges it into the per-request context
358 * that `<script server>` blocks see as `__stxServeContext` (and that is
359 * mirrored on `globalThis.__stxServeContext` during the render). This is
360 * the race-free channel for frameworks that authenticate/localize in
361 * `onRequest` and need that state visible to server scripts — plain
362 * globals set inside the hook can be clobbered by concurrent requests,
363 * and AsyncLocalStorage `enterWith()` does not survive the hook's await
364 * boundary (stacksjs/stacks#1967).
322365 */
323366 onRequest?: (req: Request) =>
324367 | Response
368 | Record<string, unknown>
325369 | null
326370 | undefined
327 | Promise<Response | null | undefined>
371 | Promise<Response | Record<string, unknown> | null | undefined>
328372
329373 /**
330374 * **Page middleware.** Modelled on Laravel's named-route middleware.
@@ -942,17 +986,50 @@ export async function serve(options: ServeOptions): Promise<void> {
942986 }
943987 }
944988
945 /** Expose the current request query string + Host/cookies/IP to `<script server>` blocks. */
946 function injectServeRequestContext(context: Record<string, any>) {
947 if (activeServeSearch) {
948 context.__stxServeSearch = activeServeSearch
949 ;(globalThis as { __stxServeSearch?: string }).__stxServeSearch = activeServeSearch
989 /**
990 * Expose the current request (query string, Host, cookies, IP, url, locale,
991 * route params) to `<script server>` blocks. Prefers the threaded per-request
992 * snapshot over the module singletons — the singletons can be clobbered by a
993 * concurrent request while this render is suspended at an await
994 * (stacksjs/stacks#1967); the snapshot cannot.
995 */
996 function injectServeRequestContext(context: Record<string, any>, reqCtx?: ServeRequestContext) {
997 const search = reqCtx ? reqCtx.search : activeServeSearch
998 const host = reqCtx ? reqCtx.host : activeServeHost
999 const cookies = reqCtx ? reqCtx.cookies : activeServeCookies
1000 const ip = reqCtx ? reqCtx.ip : activeServeIp
1001
1002 // Unconditional: a query-less request must OVERWRITE the previous
1003 // request's value on the shared global, or stale search strings leak
1004 // across requests (the old `if (activeServeSearch)` guard did exactly
1005 // that — stacksjs/stacks#1967).
1006 context.__stxServeSearch = search
1007 ;(globalThis as { __stxServeSearch?: string }).__stxServeSearch = search
1008 if (host)
1009 context.host = host
1010 context.cookies = cookies
1011 if (ip)
1012 context.ip = ip
1013
1014 // The whole request in one object. As a context key it becomes an ambient
1015 // `__stxServeContext` binding inside `<script server>` (a per-render
1016 // snapshot, immune to global clobbering); the globalThis mirror serves
1017 // out-of-continuation readers — e.g. Stacks' `requestContext` fallback —
1018 // and is refreshed here, immediately before script execution, so it is
1019 // current for the synchronous prefix of every server script.
1020 const full: ServeRequestContext = reqCtx ?? {
1021 url: '',
1022 path: '',
1023 search,
1024 host,
1025 cookieHeader: activeServeCookieHeader,
1026 cookies,
1027 ip,
1028 locale: activeServeLocale,
1029 params: {},
9501030 }
951 if (activeServeHost)
952 context.host = activeServeHost
953 context.cookies = activeServeCookies
954 if (activeServeIp)
955 context.ip = activeServeIp
1031 context.__stxServeContext = full
1032 ;(globalThis as { __stxServeContext?: ServeRequestContext }).__stxServeContext = full
9561033 }
9571034
9581035 /** Render cache must vary by locale/host/cookies — same `.stx` file can serve different `t()`/host/cookie-gated output. */
@@ -1329,7 +1406,7 @@ export async function serve(options: ServeOptions): Promise<void> {
13291406 }
13301407
13311408 // Lazy template processing function
1332 async function processTemplate(filePath: string): Promise<string> {
1409 async function processTemplate(filePath: string, reqCtx?: ServeRequestContext): Promise<string> {
13331410 const content = await Bun.file(filePath).text()
13341411 const skipCacheHint = /(?:^|[^\w$])__stx_skip_cache\s*=\s*true/.test(content)
13351412
@@ -1370,7 +1447,10 @@ export async function serve(options: ServeOptions): Promise<void> {
13701447 // again and wipe what useHead just populated.
13711448 resetHead()
13721449 injectServeLocaleContext(context)
1373 injectServeRequestContext(context)
1450 injectServeRequestContext(context, reqCtx)
1451 // Static routes have no dynamic segments — give server scripts the same
1452 // always-an-object `params` shape the dynamic path provides.
1453 context.params = reqCtx?.params ?? {}
13741454 for (const scriptBody of serverScripts) {
13751455 await extractVariables(scriptBody, context, filePath)
13761456 }
@@ -1477,7 +1557,7 @@ export async function serve(options: ServeOptions): Promise<void> {
14771557 }
14781558
14791559 // Function to get or create route
1480 async function getRoute(requestPath: string): Promise<string | null> {
1560 async function getRoute(requestPath: string, reqCtx?: ServeRequestContext): Promise<string | null> {
14811561 // Dev mode: never cache — always re-process templates so file changes are reflected
14821562 // Production caching is handled by the production server, not the dev serve path
14831563
@@ -1541,7 +1621,7 @@ export async function serve(options: ServeOptions): Promise<void> {
15411621
15421622 if (normalizedFilePath === possible || relativeFilePath === possible) {
15431623 // Process template on every request (dev mode — no caching)
1544 const output = await processTemplate(filePath)
1624 const output = await processTemplate(filePath, reqCtx)
15451625 return output
15461626 }
15471627 }
@@ -1578,7 +1658,7 @@ export async function serve(options: ServeOptions): Promise<void> {
15781658 `/${fileRoute}.md` === requestPath ||
15791659 `/${fileRoute}.html` === requestPath ||
15801660 (isIndexFile && isRootRequest)) {
1581 const output = await processTemplate(filePath)
1661 const output = await processTemplate(filePath, reqCtx)
15821662 routes.set(requestPath, output)
15831663 return output
15841664 }
@@ -1589,7 +1669,7 @@ export async function serve(options: ServeOptions): Promise<void> {
15891669 const prettyRoute = fileRoute.slice(6) // Remove 'pages/' prefix
15901670 if (`/${prettyRoute}` === requestPath ||
15911671 prettyRoute === normalizedPath) {
1592 const output = await processTemplate(filePath)
1672 const output = await processTemplate(filePath, reqCtx)
15931673 routes.set(requestPath, output)
15941674 return output
15951675 }
@@ -1630,7 +1710,7 @@ export async function serve(options: ServeOptions): Promise<void> {
16301710 const paramValues = match.slice(1)
16311711
16321712 // Process template with dynamic params in context
1633 const output = await processTemplateDynamic(filePath, paramNames, paramValues, normalizedPath)
1713 const output = await processTemplateDynamic(filePath, paramNames, paramValues, normalizedPath, reqCtx)
16341714 routes.set(requestPath, output)
16351715 return output
16361716 }
@@ -1646,6 +1726,7 @@ export async function serve(options: ServeOptions): Promise<void> {
16461726 paramNames: string[],
16471727 paramValues: string[],
16481728 routePath: string,
1729 reqCtx?: ServeRequestContext,
16491730 ): Promise<string> {
16501731
16511732 const content = await Bun.file(filePath).text()
@@ -1669,11 +1750,34 @@ export async function serve(options: ServeOptions): Promise<void> {
16691750 __route: routePath,
16701751 }
16711752
1672 // Add each param to context
1753 // Name→value map of the dynamic segments, URL-decoded (path segments
1754 // arrive percent-encoded; scripts want `café`, not `caf%C3%A9`). Single
1755 // source of truth for the ambient bindings below, the server-script
1756 // `params` object, and the client-side `window.stx._rp` injection.
1757 const paramsObj: Record<string, string> = {}
16731758 for (let i = 0; i < paramNames.length; i++) {
1674 context[paramNames[i]] = paramValues[i]
1759 let value = paramValues[i] ?? ''
1760 try {
1761 value = decodeURIComponent(value)
1762 }
1763 catch {
1764 // malformed escape — keep the raw segment
1765 }
1766 paramsObj[paramNames[i]] = value
16751767 }
16761768
1769 // Add each param to context as a bare identifier (`[id].stx` → `id`)...
1770 for (const name of paramNames) {
1771 context[name] = paramsObj[name]
1772 }
1773 // ...and as the documented `params` object — @stacksjs/stx's
1774 // variable-extractor plumbs `context.params` into every server script,
1775 // but the serve path never set it, so `params.id` was always `{}`
1776 // (stacksjs/stacks#1967).
1777 context.params = paramsObj
1778 if (reqCtx)
1779 reqCtx.params = paramsObj
1780
16771781 const { processDirectives, extractVariables, defaultConfig, injectRouterScript: injectRouter, resetHead: resetHeadDyn } = await stxModule
16781782 // Fresh head state per request (see static-route handler above for rationale)
16791783 resetHeadDyn()
@@ -1683,7 +1787,7 @@ export async function serve(options: ServeOptions): Promise<void> {
16831787 // was first added (only the static-route path called
16841788 // injectServeRequestContext), so every dynamic route silently never
16851789 // saw `host`/`cookies`/`ip`/`__stxServeSearch` until this fix.
1686 injectServeRequestContext(context)
1790 injectServeRequestContext(context, reqCtx)
16871791 for (const scriptBody of dynServerScripts) {
16881792 await extractVariables(scriptBody, context, filePath)
16891793 }
@@ -1724,12 +1828,9 @@ export async function serve(options: ServeOptions): Promise<void> {
17241828 const dependencies = new Set<string>()
17251829 output = await processDirectives(output, context, filePath, config, dependencies)
17261830
1727 // Inject route params for client-side useRoute().params
1831 // Inject route params for client-side useRoute().params — the same
1832 // decoded paramsObj the server script saw, so both sides agree.
17281833 if (paramNames.length > 0) {
1729 const paramsObj: Record<string, string> = {}
1730 for (let i = 0; i < paramNames.length; i++) {
1731 paramsObj[paramNames[i]] = paramValues[i]
1732 }
17331834 const paramsScript = `<script>if(window.stx)window.stx._rp=${JSON.stringify(paramsObj)};else window.__stx_rp=${JSON.stringify(paramsObj)};</script>`
17341835 if (output.includes('</head>')) {
17351836 output = output.replace('</head>', `${paramsScript}\n</head>`)
@@ -2031,6 +2132,21 @@ export async function serve(options: ServeOptions): Promise<void> {
20312132 activeServeCookieHeader = req.headers.get('cookie') || ''
20322133 activeServeCookies = parseCookies(req)
20332134 activeServeIp = server.requestIP(req)?.address || (req.headers.get('x-forwarded-for') || '').split(',')[0]!.trim()
2135 // Immutable-per-request snapshot threaded down to the render.
2136 // The singletons above can be reset by a concurrent request's
2137 // `finally` while this render is suspended at an await; this
2138 // object cannot (stacksjs/stacks#1967).
2139 const reqCtx: ServeRequestContext = {
2140 url: req.url,
2141 path,
2142 search: activeServeSearch,
2143 host: activeServeHost,
2144 cookieHeader: activeServeCookieHeader,
2145 cookies: activeServeCookies,
2146 ip: activeServeIp,
2147 locale: activeServeLocale,
2148 params: {},
2149 }
20342150 try {
20352151
20362152 // Handle CORS preflight
@@ -2056,16 +2172,19 @@ export async function serve(options: ServeOptions): Promise<void> {
20562172 // and gives the readyState a definite "open" before any change.
20572173 c.enqueue(hmrEncoder.encode('data: {"type":"connected"}\n\n'))
20582174 // Keepalive ping. Without traffic, the stream idles and gets
2059 // killed by Bun.serve's `idleTimeout` (default 10s) — the browser
2060 // sees `ERR_INCOMPLETE_CHUNKED_ENCODING` and falls into a
2061 // reconnect loop that mostly hides the HMR being broken. A
2062 // 15s comment line (the leading `:` is the SSE comment syntax,
2063 // ignored by the EventSource API) is well under any sane idle
2064 // timeout (Bun's, reverse proxy's, browser's) and costs nothing.
2175 // killed by Bun.serve's `idleTimeout` (default 10s) or a reverse
2176 // proxy's read timeout — the browser then sees
2177 // `ERR_INCOMPLETE_CHUNKED_ENCODING` and falls into a reconnect
2178 // loop that mostly hides the HMR being broken. The ping MUST fire
2179 // faster than the shortest such timeout: 15s was longer than Bun's
2180 // 10s default, so the connection died before the first ping ever
2181 // arrived. 5s comfortably beats Bun's 10s and typical proxy (rpx,
2182 // nginx) read timeouts. The leading `:` is SSE comment syntax
2183 // (ignored by the EventSource API) and costs nothing.
20652184 keepalive = setInterval(() => {
20662185 try { c.enqueue(hmrEncoder.encode(': keepalive\n\n')) }
20672186 catch { /* stream closed — cancel handler clears the timer */ }
2068 }, 15_000)
2187 }, 5_000)
20692188 },
20702189 cancel() {
20712190 if (keepalive) clearInterval(keepalive)
@@ -2082,10 +2201,17 @@ export async function serve(options: ServeOptions): Promise<void> {
20822201 })
20832202 }
20842203
2085 // Custom onRequest handler — short-circuits if a Response is returned
2204 // Custom onRequest handler — short-circuits if a Response is
2205 // returned. A plain-object return is merged into the request
2206 // context instead, giving the hook a race-free way to hand
2207 // state (auth cookies, locale, a user object, ...) to
2208 // `<script server>` blocks — see the onRequest option docs.
20862209 if (options.onRequest) {
2087 const customResponse = await options.onRequest(req)
2088 if (customResponse) return customResponse
2210 const hookResult = await options.onRequest(req)
2211 if (hookResult instanceof Response)
2212 return hookResult
2213 if (hookResult && typeof hookResult === 'object')
2214 Object.assign(reqCtx, hookResult)
20892215 }
20902216
20912217 // ── Page middleware pipeline ────────────────────────────────────
@@ -2272,10 +2398,10 @@ export async function serve(options: ServeOptions): Promise<void> {
22722398
22732399 // Redirect root to /home if no index exists (dashboard pattern)
22742400 if (path === '/') {
2275 const indexContent = await getRoute('/')
2401 const indexContent = await getRoute('/', reqCtx)
22762402 if (!indexContent) {
22772403 // Try /home as default landing page
2278 const homeContent = await getRoute('/home')
2404 const homeContent = await getRoute('/home', reqCtx)
22792405 if (homeContent) {
22802406 return new Response(null, {
22812407 status: 302,
@@ -2289,7 +2415,7 @@ export async function serve(options: ServeOptions): Promise<void> {
22892415 }
22902416
22912417 // Try to serve the requested page (lazy load on demand)
2292 const content = await getRoute(path)
2418 const content = await getRoute(path, reqCtx)
22932419 if (content) {
22942420 // SPA navigation: return only <main> content as fragment
22952421 const isSpaNav = req.headers.get('X-STX-Router') === 'true'
@@ -2427,7 +2553,7 @@ export async function serve(options: ServeOptions): Promise<void> {
24272553 }
24282554
24292555 // Try without extension
2430 const contentWithExt = await getRoute(`${path}.html`)
2556 const contentWithExt = await getRoute(`${path}.html`, reqCtx)
24312557 if (contentWithExt) {
24322558 return new Response(injectHmrClient(contentWithExt), {
24332559 headers: {
@@ -2585,7 +2711,7 @@ export async function serve(options: ServeOptions): Promise<void> {
25852711 // index-file conventions all just work.
25862712 for (const custom404 of ['/404', '/errors/404']) {
25872713 try {
2588 const customContent = await getRoute(custom404)
2714 const customContent = await getRoute(custom404, reqCtx)
25892715 if (customContent) {
25902716 return new Response(
25912717 isProd ? customContent : injectHmrClient(customContent),