Skip to content

Commit 90821f3

Browse files
committed
fix: isolate the shim port from application throws, and close the doc drift
Review found five things. The shim port the per-tab fallback hands the relay runs application code synchronously in its postMessage, and the relay deletes a port whose postMessage throws. That heuristic reads a throw as "the tab is gone", which is right for a real MessagePort and wrong here: an overlay render that threw would permanently unsubscribe the tab and silently kill its live reload. The pre-#1397 fallback attached to the EventSource directly, where a handler throw detached nothing, so the guard restores the old behaviour rather than adding a new one. The ?v= test claimed to cover the early path while building the handler with dev: false, so it exercised the prod path instead. It cannot be written the other way, since fileResponse hard-codes no-cache whenever dev is true, so immutable is unobservable there. Retitled to what it actually pins, plus a dev-mode assertion that a fingerprinted asset still serves on the early path. Correcting the middleware prose left two code-block comments two lines below still saying "every request", and the same claim was still on the agent-facing skill reference the scaffold ships, in the root AGENTS.md layout table, and in the blog example's own middleware. Inserting the two constants between the module header and the function orphaned its @PARAM tags onto a constant, and the header no longer mentioned that reloadClientJs inlines this source too.
1 parent 23d0d08 commit 90821f3

8 files changed

Lines changed: 73 additions & 16 deletions

File tree

.agents/skills/webjs/references/muscle-memory-gotchas.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ Export `GET` / `POST` / etc. as named async functions `(request, { params }) =>
202202

203203
### `middleware.ts` is per-segment and chainable, not one matcher config
204204

205-
The file stays `middleware.ts`, NOT Next 16's renamed `proxy.ts`. WebJs middleware is in-process, chainable, and per-segment (the Remix / Koa model). There is no `export const config = { matcher }` and no single-file restriction. The default export is `async (req, next) => Response`: return a Response to short-circuit, or call `next()` and post-process. Colocate `app/admin/middleware.ts` next to the admin routes and it runs for that subtree only. An optional root `middleware.ts` runs on every request, outermost to innermost.
205+
The file stays `middleware.ts`, NOT Next 16's renamed `proxy.ts`. WebJs middleware is in-process, chainable, and per-segment (the Remix / Koa model). There is no `export const config = { matcher }` and no single-file restriction. The default export is `async (req, next) => Response`: return a Response to short-circuit, or call `next()` and post-process. Colocate `app/admin/middleware.ts` next to the admin routes and it runs for that subtree only. An optional root `middleware.ts` runs on every app request, outermost to innermost. Two things are served ahead of it: the framework's own `/__webjs/*` assets and probes, in both modes, and in DEV only, `/public/*` plus the `/sw.js` / `/offline.html` root remaps and `/favicon.ico`, so a stylesheet is never queued behind the dev startup analysis. In production those static files go through root middleware normally, so a middleware that guards an asset still guards it where it counts.
206206

207207
### No `<Link>`, no `next/navigation`, no `next/*` libraries
208208

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ app/ ROUTING ONLY (thin adapters importing from modules/;
185185
<path>/route.js HTTP handler at /<path>
186186
<segment>/middleware.js per-segment middleware
187187
<segment>/loading.js auto Suspense boundary
188-
middleware.js root middleware (every request; .ts/.mts/.mjs too)
188+
middleware.js root middleware (every app request; .ts/.mts/.mjs too)
189189
readiness.js optional /__webjs/ready check (return false/throw = 503)
190190
env.js optional boot-time env validation (schema or validator fn; fails fast)
191191
instrumentation.js optional boot-time hook (register(); wire APM via setOnError, #848)

examples/blog/middleware.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Global middleware. Runs on every request before webjs routes it.
2+
* Global middleware. Runs on every app request before webjs routes it.
33
* Return a Response to short-circuit; call next() to continue.
44
*
55
* To add framework sessions:

packages/server/src/dev-reload-worker.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@
22
* The dev live-reload SharedWorker relay (#887), the BROWSER half. Kept as a
33
* standalone browser-safe module (no node imports) so the served worker inlines
44
* the EXACT source a browser test drives, with no drift, the same pattern as
5-
* `dev-overlay.js` (#264). `reloadWorkerJs` in dev.js reads this file, strips
6-
* the `export` keyword, and appends a
7-
* `startReloadWorker(self, EventSource, '<eventsUrl>')` call.
5+
* `dev-overlay.js` (#264).
6+
*
7+
* BOTH served dev scripts inline this file, `export`-stripped. `reloadWorkerJs`
8+
* appends a `startReloadWorker(self, EventSource, '<eventsUrl>')` call for the
9+
* SharedWorker, and since #1397 `reloadClientJs` inlines it too, so the per-tab
10+
* fallback runs this same relay over a shim port instead of a second copy of
11+
* the boot-id rule and the reload debounce.
812
*
913
* One SharedWorker is shared across every tab of the origin (a SharedWorker is
1014
* keyed by its script URL), so it holds the ONE `EventSource` to
1115
* `/__webjs/events` and fans each `reload` / `webjs-error` out to every tab over
1216
* its `MessagePort`. Tab count never touches the browser's per-host HTTP/1.1
1317
* connection cap, which the per-tab `EventSource` it replaces used to exhaust.
14-
*
15-
* @param {{ onconnect: any }} scope the worker global (`self`)
16-
* @param {new (url: string) => any} EventSourceCtor the `EventSource` constructor
17-
* @param {string} eventsUrl the base-path-aware `/__webjs/events` URL
1818
*/
1919

2020
/**
@@ -42,6 +42,14 @@ export const RELOAD_QUIET_MS = 2000;
4242
*/
4343
export const RELOAD_MAX_HOLD_MS = 5000;
4444

45+
/**
46+
* @param {{ onconnect: any, setTimeout?: any, clearTimeout?: any }} scope the
47+
* worker global (`self`), or a plain shim object for the per-tab fallback.
48+
* Timers are read off it when it has them, which is what lets a browser test
49+
* drive the debounce on a fake clock.
50+
* @param {new (url: string) => any} EventSourceCtor the `EventSource` constructor
51+
* @param {string} eventsUrl the base-path-aware `/__webjs/events` URL
52+
*/
4553
export function startReloadWorker(scope, EventSourceCtor, eventsUrl) {
4654
/** @type {Set<any>} */
4755
const ports = new Set();

packages/server/src/dev.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3084,8 +3084,19 @@ function __webjsDirectEvents() {
30843084
const scope = {};
30853085
startReloadWorker(scope, EventSource, ${eventsUrl});
30863086
scope.onconnect({ ports: [{ start() {}, postMessage(m) {
3087-
if (m.type === 'reload') __webjsReloadWhenReady();
3088-
else if (m.type === 'webjs-error') __webjsApplyError(m.data);
3087+
// Nothing may throw out of here. The relay's fanout deletes a port whose
3088+
// postMessage throws, which is the right read for a REAL MessagePort (a
3089+
// throw there means the tab is gone) and the wrong one for this shim,
3090+
// whose postMessage runs application code synchronously: an overlay
3091+
// render that threw would permanently unsubscribe this tab and silently
3092+
// kill live reload for the rest of the page's life. The old fallback
3093+
// attached to the EventSource directly, where a handler throw detached
3094+
// nothing, so swallowing here restores that behaviour rather than adding
3095+
// a new one.
3096+
try {
3097+
if (m.type === 'reload') __webjsReloadWhenReady();
3098+
else if (m.type === 'webjs-error') __webjsApplyError(m.data);
3099+
} catch (_) { /* a bad frame must not cost this tab its live reload */ }
30893100
} }] });
30903101
}
30913102
try {

packages/server/test/dev/public-before-analysis.test.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,25 @@ test('a stale webjs.dev.regenerate output is rebuilt before serving on the early
160160
assert.equal(await res.text(), 'body{color:lime}', 'the stale output was regenerated, not served as-is');
161161
});
162162

163-
// #243: the `?v=` fingerprint still decides the cache header on the early path.
164-
test('?v= still yields immutable on the early path, un-versioned keeps the 1h fallback', async () => {
163+
// #243: the `?v=` fingerprint still decides the cache header through the
164+
// extracted function. This is the PROD path deliberately, and it is the one
165+
// case in this file that is not about the dev hoist. `fileResponse` hard-codes
166+
// `cache-control: no-cache` whenever `opts.dev` is true, so `immutable` is
167+
// unobservable on the dev early path by construction; asserting it there would
168+
// be a test that cannot fail. What this pins is that the extraction did not
169+
// drop the fingerprint handling on the call site that still serves it.
170+
test('?v= still yields immutable through the extracted serve (prod), un-versioned keeps the 1h fallback', async () => {
165171
const app = await createRequestHandler({ appDir: makeApp(), dev: false });
166172
const versioned = await app.handle(new Request('http://x/public/a.css?v=abc123'));
167173
assert.match(versioned.headers.get('cache-control') || '', /immutable/, 'content-addressed is immutable');
168174
const plain = await app.handle(new Request('http://x/public/a.css'));
169175
assert.doesNotMatch(plain.headers.get('cache-control') || '', /immutable/, 'un-fingerprinted is not');
176+
177+
// The dev half of the same concern, which IS on the early path: a `?v=`
178+
// request still resolves and serves rather than falling through, even though
179+
// the header it earns in dev is `no-cache` either way.
180+
const devApp = await createRequestHandler({ appDir: makeApp(), dev: true });
181+
const devVersioned = await devApp.handle(new Request('http://x/public/a.css?v=abc123'));
182+
assert.equal(devVersioned.status, 200, 'a fingerprinted asset still serves on the dev early path');
183+
assert.equal(await devVersioned.text(), 'body{color:red}\n');
170184
});

packages/server/test/dev/reload-shared-connection.test.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,30 @@ test('dev serves the reload SharedWorker, and the client uses it with a direct E
4848
assert.match(clientSrc, /function startReloadWorker/, 'the client inlines the relay module for the fallback');
4949
assert.match(clientSrc, /startReloadWorker\(scope, EventSource, "\/__webjs\/events"\)/, 'the fallback runs the relay against the real EventSource');
5050
assert.match(clientSrc, /scope\.onconnect\(\{ ports: \[\{/, 'and drives it over a shim port');
51+
// The shim's postMessage runs application code synchronously, and the relay's
52+
// fanout DELETES a port whose postMessage throws (correct for a real
53+
// MessagePort, where a throw means the tab is gone). Unguarded, an overlay
54+
// render that threw would permanently unsubscribe the tab and silently kill
55+
// its live reload, which the pre-#1397 fallback could not do because it
56+
// attached to the EventSource directly.
57+
//
58+
// Sliced to the fallback function's own body first. A regex over the whole
59+
// client matches the SharedWorker bootstrap's `try { ... } catch` further
60+
// down and passes with the guard removed, which is a test that observes
61+
// nothing (found by running exactly that counterfactual).
62+
const fallbackStart = clientSrc.indexOf('function __webjsDirectEvents()');
63+
assert.notEqual(fallbackStart, -1, 'the fallback function is in the client');
64+
const fallbackBody = clientSrc.slice(fallbackStart, clientSrc.indexOf('if (typeof SharedWorker', fallbackStart));
65+
assert.match(
66+
fallbackBody,
67+
/postMessage\(m\)\s*\{[^]*?try\s*\{/,
68+
'the shim port opens a try before running any application code',
69+
);
70+
assert.match(fallbackBody, /\}\s*catch\s*\(_\)/, 'and swallows the throw so the relay cannot drop the tab');
71+
assert.ok(
72+
fallbackBody.indexOf('try {') < fallbackBody.indexOf('__webjsReloadWhenReady()'),
73+
'the guard opens BEFORE the reload call, not around something else',
74+
);
5175
assert.match(clientSrc, /catch\s*\(_\)\s*\{\s*__webjsDirectEvents/, 'a worker failure falls back');
5276
// The debounce (#1397) is part of the relay, so it ships in BOTH scripts.
5377
assert.match(clientSrc, /const RELOAD_QUIET_MS/, 'the reload debounce ships in the client fallback');

website/app/docs/middleware/page.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export default function Middleware() {
1111
<p>Place a <code>middleware.ts</code> at the root of your project (next to <code>app/</code>, not inside it). This middleware runs on <strong>every app request</strong> before WebJs routes it to a page, API route, or server action. Any of <code>middleware.ts</code>, <code>.js</code>, <code>.mts</code>, or <code>.mjs</code> works, and <code>.ts</code> wins if you somehow have more than one.</p>
1212
<p>Two things are served ahead of it, so root middleware never sees them. The framework's own <code>/__webjs/*</code> assets and health probes bypass it in both dev and production, because they are framework infrastructure your app needs to boot rather than app routes. In <strong>development only</strong>, static files under <code>/public/*</code> (plus the <code>/sw.js</code> and <code>/offline.html</code> root remaps and <code>/favicon.ico</code>) are served ahead of it as well, so a stylesheet is never queued behind the dev server's startup analysis. In production those static files go through root middleware normally, so a middleware that protects an asset still protects it where it counts.</p>
1313
<code-block>my-app/
14-
middleware.ts # root middleware: runs on every request
14+
middleware.ts # root middleware: runs on every app request
1515
app/
1616
page.ts
1717
api/
@@ -33,7 +33,7 @@ export default async function middleware(
3333
<h2>Per-Segment Middleware</h2>
3434
<p>Place a <code>middleware.ts</code> inside any directory under <code>app/</code> to scope it to that subtree. It runs only for requests whose URL matches that segment and its children.</p>
3535
<code-block>my-app/
36-
middleware.ts # root: every request
36+
middleware.ts # root: every app request
3737
app/
3838
page.ts # /: root + no segment middleware
3939
dashboard/

0 commit comments

Comments
 (0)