From b4b7ccf88cf2e0c7199168bfa1bba2579b6c278a Mon Sep 17 00:00:00 2001 From: iris Date: Tue, 26 May 2026 01:25:49 +0200 Subject: [PATCH] =?UTF-8?q?dashboard:=20address=20argus=20nits=20=E2=80=94?= =?UTF-8?q?=20bfcache=20restore=20+=20worker=20IIFE=20(#448)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mara on #453: address argus's two yellow nits. bfcache restore gap previously openStream registered a pagehide unsubscribe but never re-subscribed on pageshow, so a bfcache restore left the consumer's onmessage bound but no events flowing. fix: maintain a registry of live subscriptions (Map). bind page lifecycle hooks once: - pagehide: unsubscribe every URL, drop route listeners, invalidate the cached SharedWorker port (it may be collected if all other tabs closed while this page was frozen). - pageshow { persisted: true }: get a fresh port via getSharedPort (creates a new SharedWorker if needed), re-attach every route listener, re-subscribe to every URL. target.readyState resets to CONNECTING so the worker's synthetic open after subscribe fires the consumer's onStreamOpen and triggers a refresh. worker bundle format stream-worker.js was bundled as format: 'esm' but loaded as classic via new SharedWorker(url, name). today's worker has no imports/exports so the ESM bundle is syntactically valid as a classic script; argus's concern was that a future contributor adding an import would silently break things. fix: switched build.mjs to format: 'iife'. esbuild now wraps the worker output in (() => { ... })(); any future import statement would surface as a build error rather than ship broken code. verified output starts with the IIFE wrapper. other - target.close() now reads _sharedPort lazily so close-after-bfcache (port may have been recreated) doesn't try to postMessage on a stale reference. - _activeSubs.delete on close keeps the registry honest if a consumer ever explicitly closes a stream (none do today, but the shape stays correct). validation: npm run build clean. stream-worker.js: 1.8 kb → 1.9 kb (IIFE wrapper). common.js bfcache logic adds ~30 LOC inside the existing module — bundle deltas negligible. --- frontend/packages/dashboard/build.mjs | 13 +++- frontend/packages/dashboard/src/common.js | 91 ++++++++++++++++++----- 2 files changed, 83 insertions(+), 21 deletions(-) diff --git a/frontend/packages/dashboard/build.mjs b/frontend/packages/dashboard/build.mjs index d2e87153..e8cf87b3 100644 --- a/frontend/packages/dashboard/build.mjs +++ b/frontend/packages/dashboard/build.mjs @@ -49,14 +49,19 @@ await build({ // scripts run in a different global (`self` is the worker scope, no // `window`) so they can't be inlined into app.js / flow.js. Output is // at `static/stream-worker.js`; common.js's `openStream` references -// `/static/stream-worker.js` as the SharedWorker URL. ES-module-shaped -// (so a future `import` from within the worker can pull in shared -// utilities), browser target — webworker target subset. +// `/static/stream-worker.js` as the SharedWorker URL. `format: 'iife'` +// matches the classic-script load (`new SharedWorker(url, name)` with +// no `{ type: 'module' }`); Firefox is the #448 target and module +// SharedWorker support there is patchy, so keeping the worker as a +// classic script + IIFE bundle is the compatible default. argus nit +// on #453: if a future contributor adds an `import` to this bundle, +// the IIFE format will surface it as a build error rather than +// silently shipping broken code. await build({ entryPoints: [src('stream-worker.js')], outdir: staticDir(''), bundle: true, - format: 'esm', + format: 'iife', platform: 'browser', target: ['es2022'], sourcemap: true, diff --git a/frontend/packages/dashboard/src/common.js b/frontend/packages/dashboard/src/common.js index 09f6f15e..8998ff46 100644 --- a/frontend/packages/dashboard/src/common.js +++ b/frontend/packages/dashboard/src/common.js @@ -79,26 +79,84 @@ export const form = (action, btnClass, btnLabel, confirmMsg, extra = {}, opts = const SHARED_WORKER_PATH = '/static/stream-worker.js'; const SHARED_WORKER_NAME = 'hyperhive-stream'; -// One SharedWorker instance per page, reused by all openStream calls -// on that page. Lazy — pages with no streams don't spawn the worker. +// One SharedWorker port per page, reused by all openStream calls on +// that page. Invalidated on `pagehide` so a bfcache restore picks up +// a fresh port (the cached one may have been collected if all other +// tabs closed while this page was frozen — argus nit on #453). let _sharedPort = null; -function getSharedPort() { - if (_sharedPort) return _sharedPort; +function makeSharedPort() { if (typeof SharedWorker === 'undefined') return null; try { const sw = new SharedWorker(SHARED_WORKER_PATH, SHARED_WORKER_NAME); - _sharedPort = sw.port; - _sharedPort.start(); - return _sharedPort; + sw.port.start(); + return sw.port; } catch (err) { console.warn('SharedWorker unavailable, falling back to direct EventSource:', err); return null; } } +function getSharedPort() { + if (!_sharedPort) _sharedPort = makeSharedPort(); + return _sharedPort; +} + +// Registry of live subscriptions on this page. Keyed by url so a +// second openStream call for the same URL (would only happen on a +// hypothetical multi-consumer page) attaches to the existing route +// rather than overlapping. Each entry caches the route function so +// bfcache-restore re-bind can re-attach it to the fresh port. +// +// Today's pages only call openStream once with one URL; the registry +// shape just keeps the bfcache-restore path correct if that changes +// (e.g. /index.html later subscribing to two streams). +const _activeSubs = new Map(); + +// One-shot wiring of the page-wide lifecycle hooks: on bfcache +// freeze (`pagehide { persisted: true }`) we unsubscribe so the +// worker can close the upstream when the last live subscriber +// leaves; on bfcache restore (`pageshow { persisted: true }`) we +// invalidate the cached port (it may be dead if all other tabs +// closed during the freeze) and re-attach every active subscription +// to a fresh port. argus nit on #453: without this, the consumer's +// onmessage stays bound but no events flow after a bfcache restore. +let _lifecycleBound = false; +function bindLifecycleOnce() { + if (_lifecycleBound) return; + _lifecycleBound = true; + window.addEventListener('pagehide', () => { + if (!_sharedPort) return; + for (const url of _activeSubs.keys()) { + try { _sharedPort.postMessage({ kind: 'unsubscribe', url }); } + catch { /* port dead — worker side already cleaned up */ } + } + // Drop port routes too; the bfcache-restore path will re-add + // them on a fresh port. Leaving stale routes on a dead port + // would just keep a closure alive without cost, but cleaning + // up keeps the registry shape honest. + for (const sub of _activeSubs.values()) { + try { _sharedPort.removeEventListener('message', sub.route); } + catch { /* same */ } + } + _sharedPort = null; + }); + window.addEventListener('pageshow', (ev) => { + if (!ev.persisted) return; // cold load — openStream just bound listeners + if (!_activeSubs.size) return; + const port = getSharedPort(); + if (!port) return; // SharedWorker really gone; fallback already in place + for (const [url, sub] of _activeSubs) { + sub.target.readyState = 0; // CONNECTING — the worker will fire 'open' + port.addEventListener('message', sub.route); + try { port.postMessage({ kind: 'subscribe', url }); } + catch { /* port dead immediately — skip */ } + } + }); +} export function openStream(url) { const port = getSharedPort(); if (!port) return new EventSource(url); + bindLifecycleOnce(); // Build an EventSource-shaped facade so consumer code is unchanged. // `target.onmessage` / `onopen` / `onerror` are assigned by the @@ -111,9 +169,14 @@ export function openStream(url) { onerror: null, readyState: 0, // CONNECTING close() { - try { port.postMessage({ kind: 'unsubscribe', url }); } - catch { /* port dead */ } - port.removeEventListener('message', route); + const p = _sharedPort; + if (p) { + try { p.postMessage({ kind: 'unsubscribe', url }); } + catch { /* port dead */ } + try { p.removeEventListener('message', route); } + catch { /* same */ } + } + _activeSubs.delete(url); }, }; const route = (e) => { @@ -137,15 +200,9 @@ export function openStream(url) { } } }; + _activeSubs.set(url, { target, route }); port.addEventListener('message', route); port.postMessage({ kind: 'subscribe', url }); - // Drop the subscription when the tab unloads so the worker can - // close the upstream EventSource when the last subscriber leaves. - // `pagehide` fires for both real unloads and bfcache transitions. - window.addEventListener('pagehide', () => { - try { port.postMessage({ kind: 'unsubscribe', url }); } - catch { /* port dead — worker side already cleaned up */ } - }); return target; }