From dbf26f9dc98a1c8a1aa0af5a37a54999edf18faf Mon Sep 17 00:00:00 2001 From: alexander-akait <4567934+alexander-akait@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:20:13 +0000 Subject: [PATCH 1/5] fix(overlay): take focus when opening and hand it back when closing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlay opened without touching focus, so reaching it meant clicking into the frame first: Escape and the arrow keys did nothing until then, and a screen reader stayed wherever the page was. Its frame had no accessible name either, leaving it announced by its `about:blank` url. Only the render that opens it takes focus — paginating re-renders the card, and moving focus then would take it off the button being clicked. These are the parts webpack-dev-server's own overlay has and this one did not, which it would otherwise lose by switching to this one. --- .changeset/overlay-focus.md | 5 ++++ client-src/overlay.js | 39 +++++++++++++++++++++++++++++++ test/e2e/overlay.test.js | 46 +++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 .changeset/overlay-focus.md diff --git a/.changeset/overlay-focus.md b/.changeset/overlay-focus.md new file mode 100644 index 000000000..786a60194 --- /dev/null +++ b/.changeset/overlay-focus.md @@ -0,0 +1,5 @@ +--- +"webpack-dev-middleware": patch +--- + +Move focus into the error overlay when it opens and give it back when it closes, and give its frame an accessible name diff --git a/client-src/overlay.js b/client-src/overlay.js index 74b749216..d35d29771 100644 --- a/client-src/overlay.js +++ b/client-src/overlay.js @@ -127,7 +127,9 @@ const colors = { * @property {HTMLElement | null} card visible panel inside the iframe * @property {boolean} runtimeListenersAttached whether the window listeners are attached * @property {boolean} hostKeydownAttached whether the host document's Escape listener is attached + * @property {boolean} focusOnRender whether the next render is the first one of a newly opened overlay * @property {number} pageIndex page shown when paginating + * @property {Element | null} previousActiveElement what the page had focused before the overlay opened * @property {Record} problemsBySource each reporting source's problems * @property {{ type: "errors" | "warnings", lines: string[] } | null} currentProblems union of every source, as displayed * @property {{ createHTML: (value: string) => EXPECTED_ANY } | undefined} trustedTypesPolicy trusted types policy @@ -141,7 +143,9 @@ function createOverlayState() { card: null, runtimeListenersAttached: false, hostKeydownAttached: false, + focusOnRender: false, pageIndex: 0, + previousActiveElement: null, problemsBySource: {}, currentProblems: null, trustedTypesPolicy: undefined, @@ -436,6 +440,23 @@ export function clear(source) { (state.frame.parentNode).removeChild(state.frame); } + // Hand focus back to whatever had it, or the page is left with focus on a + // removed element and the next Tab starts from the top of the document. + const { previousActiveElement } = state; + + if ( + previousActiveElement && + typeof (/** @type {HTMLElement} */ (previousActiveElement).focus) === + "function" && + // Removed from the document while the overlay was up: focusing it does + // nothing useful, and reading `isConnected` is how to tell. + previousActiveElement.isConnected !== false + ) { + /** @type {HTMLElement} */ (previousActiveElement).focus(); + } + + state.previousActiveElement = null; + state.focusOnRender = false; state.frame = null; state.card = null; state.problemsBySource = {}; @@ -482,8 +503,18 @@ function ensureOverlay() { ); } + // Whatever the page had focused, so it can be given back — the overlay + // takes focus to be reachable by keyboard, and is rude if it keeps it. + state.previousActiveElement = document.activeElement; + // Only the render that opens the overlay takes focus. Paginating re-renders + // it, and moving focus then would take it off the button being clicked. + state.focusOnRender = true; + state.frame = document.createElement("iframe"); state.frame.id = OVERLAY_ID; + // An iframe with no accessible name is announced by its url, which here is + // `about:blank`. + state.frame.title = "Build errors and warnings"; state.frame.src = "about:blank"; applyStyle(state.frame, backdropStyles); document.body.appendChild(state.frame); @@ -712,6 +743,14 @@ function renderProblems() { ? "Use ‹ › or the arrow keys to navigate. Click outside, press Esc, or fix the code to dismiss." : "Click outside, press Esc, or fix the code to dismiss."; card.appendChild(hint); + + // Focus reaches into the frame so Escape and the arrow keys work without a + // click first, and so a screen reader lands on the problem rather than + // staying where the page was. + if (state.focusOnRender) { + state.focusOnRender = false; + closeButton.focus(); + } } render = renderProblems; diff --git a/test/e2e/overlay.test.js b/test/e2e/overlay.test.js index 37cb7f4cb..b91814c22 100644 --- a/test/e2e/overlay.test.js +++ b/test/e2e/overlay.test.js @@ -184,6 +184,52 @@ describe("error overlay (browser)", () => { expect(await page.$(`#${OVERLAY_ID}`)).toBeNull(); }); + it("takes focus when it opens and hands it back when it closes", async () => { + hotApp = await createHotApp({ code: acceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + // Something on the page holds focus before the overlay appears. + await page.evaluate(() => { + const input = document.createElement("input"); + + input.id = "focus-me"; + document.body.appendChild(input); + input.focus(); + }); + + hotApp.edit("broken while focused {{{"); + const frame = await waitForOverlay(page); + + // Focus reaches into the frame, so Escape and the arrow keys work without + // clicking first, and a screen reader lands on the problem. + expect( + await frame.evaluate(() => + document.activeElement + ? document.activeElement.getAttribute("aria-label") + : null, + ), + ).toBe("Close"); + + // The frame is announced by name rather than by its `about:blank` url. + expect( + await page.$eval(`#${OVERLAY_ID}`, (element) => element.title), + ).toBeTruthy(); + + await page.keyboard.press("Escape"); + await waitForNoOverlay(page); + + // Focus goes back where it was, rather than being lost with the removed + // frame and sending the next Tab to the top of the document. + expect( + await page.evaluate(() => + document.activeElement ? document.activeElement.id : null, + ), + ).toBe("focus-me"); + }); + it("dismisses on backdrop and close-button clicks, but not inside the card", async () => { hotApp = await createHotApp({ code: acceptedApp("v1") }); ({ page, browser } = await runBrowser()); From 42e4e7d6820cb3be1aa536e5e5f97b32c66c1537 Mon Sep 17 00:00:00 2001 From: alexander-akait <4567934+alexander-akait@users.noreply.github.com> Date: Thu, 24 Sep 2026 20:50:53 +0000 Subject: [PATCH 2/5] test(overlay): cover the webpack-dev-server behaviors this overlay had no test for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mapping webpack-dev-server's 39 end-to-end overlay cases onto this one's 36 found six behaviors it relies on that nothing here pinned: an `errors` filter keeping one problem and rejecting all of them, a failure in the very first build (which arrives through the catch-up sync rather than a rebuild), one broken rebuild replacing another's overlay, Escape working a second time, and markup in a build error reaching the card as text. All six already behave the way webpack-dev-server's do — these are the tests, not fixes. --- test/e2e/overlay.test.js | 127 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/test/e2e/overlay.test.js b/test/e2e/overlay.test.js index b91814c22..5259b833d 100644 --- a/test/e2e/overlay.test.js +++ b/test/e2e/overlay.test.js @@ -893,6 +893,133 @@ describe("error overlay (browser)", () => { }); }); +describe("error overlay parity with webpack-dev-server (browser)", () => { + let hotApp; + let browser; + let page; + + afterEach(async () => { + ({ browser, app: hotApp } = await closeE2e(browser, hotApp)); + }); + + it("keeps an error the filter accepts", async () => { + hotApp = await createHotApp({ + query: + '?overlay={"errors":"function(message){return message.includes(`keep-me`)}"}', + code: acceptedApp("v1"), + }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + hotApp.edit("keep-me is not valid javascript {{{"); + const frame = await waitForOverlay(page); + + expect(await frame.evaluate(() => document.body.textContent)).toContain( + "keep-me", + ); + }); + + it("shows no overlay when the errors filter rejects everything", async () => { + hotApp = await createHotApp({ + query: '?overlay={"errors":"function(){return false}"}', + code: acceptedApp("v1"), + }); + ({ page, browser } = await runBrowser()); + const console_ = collectConsole(page); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + hotApp.edit("rejected by the filter {{{"); + // The build problem still reaches the console — just not the DOM. + await console_.waitFor("Module parse failed"); + + expect(await page.$(`#${OVERLAY_ID}`)).toBeNull(); + }); + + it("shows an error raised by the very first build", async () => { + // Broken before the browser ever connects, so the overlay has to come + // from the catch-up sync rather than from a rebuild. + hotApp = await createHotApp({ code: "broken from the start {{{" }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + const frame = await waitForOverlay(page); + + expect(await frame.evaluate(() => document.body.textContent)).toContain( + "Module parse failed", + ); + }); + + it("replaces the overlay when a rebuild is still broken", async () => { + hotApp = await createHotApp({ code: acceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + hotApp.edit("first breakage {{{"); + let frame = await waitForOverlay(page); + + expect(await frame.evaluate(() => document.body.textContent)).toContain( + "first breakage", + ); + + // A second, different failure must replace the first rather than leaving + // the stale one on screen. + hotApp.edit("second breakage {{{"); + frame = await waitForOverlay(page); + await frame.waitForFunction(() => + document.body.textContent.includes("second breakage"), + ); + + expect(await frame.evaluate(() => document.body.textContent)).not.toContain( + "first breakage", + ); + }); + + it("dismisses with Escape more than once", async () => { + hotApp = await createHotApp({ code: acceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + hotApp.edit("broken once {{{"); + await waitForOverlay(page); + await page.keyboard.press("Escape"); + await waitForNoOverlay(page); + + // Dismissing must not tear down whatever lets the next problem re-open + // it — a listener removed for good would leave the page blind. + hotApp.edit("broken twice {{{"); + await waitForOverlay(page); + await page.keyboard.press("Escape"); + await waitForNoOverlay(page); + + expect(await page.$(`#${OVERLAY_ID}`)).toBeNull(); + }); + + it("escapes markup in a build error instead of rendering it", async () => { + hotApp = await createHotApp({ code: acceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + // The error text carries markup, which must reach the card as text. + hotApp.edit("const bad = \"\" {{{"); + const frame = await waitForOverlay(page); + + expect( + await frame.evaluate(() => document.querySelectorAll("img").length), + ).toBe(0); + expect(await page.evaluate(() => globalThis.xss)).toBeUndefined(); + }); +}); + describe("overlay shared state across bundled copies (browser)", () => { const OVERLAY_ENTRY = require.resolve("../../client-src/overlay.js"); const OVERLAY_STATE_KEY = "__webpack_dev_middleware_hot_overlay_state__"; From 176acade7b1e25d16e5d24d1dcc6f3fe4e36bc78 Mon Sep 17 00:00:00 2001 From: alexander-akait <4567934+alexander-akait@users.noreply.github.com> Date: Thu, 24 Sep 2026 21:00:21 +0000 Subject: [PATCH 3/5] fix(client): report connecting over either transport, and cover both MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connected` was logged by the Server-Sent Events transport itself, so a page using the WebSocket one got no sign it had connected at all. It is said by the shared socket now, on the same event, so neither transport can be the one that stays quiet. Found by running the same four behaviors over both: connecting and applying a hot update, falling back to a reload when nothing accepts it, showing and clearing a build error, and catching up a client on what it missed. The harness takes `transport: "ws"`, which also exercises `attach` — the WebSocket path had no browser coverage before this. --- .changeset/connected-log-both-transports.md | 5 + client-src/clients/EventSourceClient.js | 3 - client-src/clients/createSocket.js | 3 + test/e2e/transport.test.js | 112 ++++++++++++++++++++ test/helpers/hot-app.js | 33 +++++- 5 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 .changeset/connected-log-both-transports.md create mode 100644 test/e2e/transport.test.js diff --git a/.changeset/connected-log-both-transports.md b/.changeset/connected-log-both-transports.md new file mode 100644 index 000000000..d298e8e86 --- /dev/null +++ b/.changeset/connected-log-both-transports.md @@ -0,0 +1,5 @@ +--- +"webpack-dev-middleware": patch +--- + +Say `connected` whichever transport the client used, rather than only Server-Sent Events diff --git a/client-src/clients/EventSourceClient.js b/client-src/clients/EventSourceClient.js index 3f2296075..eff8bd55c 100644 --- a/client-src/clients/EventSourceClient.js +++ b/client-src/clients/EventSourceClient.js @@ -1,5 +1,3 @@ -import { log } from "../utils/log.js"; - /** @typedef {import("./createSocket.js").CommunicationClient} CommunicationClient */ /** @typedef {import("./createSocket.js").ClientHandler} ClientHandler */ @@ -34,7 +32,6 @@ export default class EventSourceClient { this.client.addEventListener("open", () => { this.lastActivity = Date.now(); - log.info("connected"); if (this.openHandler) { this.openHandler(); diff --git a/client-src/clients/createSocket.js b/client-src/clients/createSocket.js index b7888227b..4d8e3b86e 100644 --- a/client-src/clients/createSocket.js +++ b/client-src/clients/createSocket.js @@ -68,6 +68,9 @@ export default function createSocket(Client, url, options = {}) { client = new Client(url, options.clientOptions); client.onOpen(() => { + // Said here rather than in a transport, or whichever one did not say it + // would leave the page with no sign it had connected at all. + log.info("connected"); attempt = 0; }); diff --git a/test/e2e/transport.test.js b/test/e2e/transport.test.js new file mode 100644 index 000000000..a5c38d63e --- /dev/null +++ b/test/e2e/transport.test.js @@ -0,0 +1,112 @@ +import collectConsole from "../helpers/console-collector"; +import { + OVERLAY_ID, + acceptedApp, + closeE2e, + unacceptedApp, + waitForAppText, + waitForNoOverlay, + waitForOverlay, +} from "../helpers/e2e"; +import createHotApp from "../helpers/hot-app"; +import runBrowser from "../helpers/run-browser"; + +jest.setTimeout(400000); + +// Everything below is the same story over each transport. The client and the +// server agree on which one through matching options, and nothing above them +// is supposed to be able to tell the difference — which is exactly the claim +// worth testing rather than assuming. +for (const transport of ["sse", "ws"]) { + describe(`hot client over ${transport} (browser)`, () => { + let hotApp; + let browser; + let page; + + afterEach(async () => { + ({ browser, app: hotApp } = await closeE2e(browser, hotApp)); + }); + + it("connects and applies a hot update", async () => { + hotApp = await createHotApp({ transport, code: acceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + const console_ = collectConsole(page); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + await console_.waitFor("connected"); + + // A marker that survives HMR but not a navigation, so the update below + // is proved to have been applied rather than reloaded into place. + await page.evaluate(() => { + globalThis.notReloaded = true; + }); + + hotApp.edit(acceptedApp("v2")); + await waitForAppText(page, "v2"); + + expect(await page.evaluate(() => globalThis.notReloaded)).toBe(true); + }); + + it("falls back to a reload when the update cannot be applied", async () => { + hotApp = await createHotApp({ transport, code: unacceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + await page.evaluate(() => { + globalThis.notReloaded = true; + }); + + hotApp.edit(unacceptedApp("v2")); + await waitForAppText(page, "v2"); + + // Nothing accepted the update, so the page went round again. + expect(await page.evaluate(() => globalThis.notReloaded)).toBeUndefined(); + }); + + it("shows a build error in the overlay and clears it on recovery", async () => { + hotApp = await createHotApp({ transport, code: acceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + hotApp.edit("this is not valid javascript {{{"); + const frame = await waitForOverlay(page); + + expect(await frame.evaluate(() => document.body.textContent)).toContain( + "Module parse failed", + ); + + // Recovering from a broken build cannot be applied hot, so the page + // reloads — wait for it to settle before reading the DOM, or the query + // races the navigation. + hotApp.edit(acceptedApp("v2")); + await waitForAppText(page, "v2"); + await waitForNoOverlay(page); + + expect(await page.$(`#${OVERLAY_ID}`)).toBeNull(); + }); + + it("catches a client up on what it missed while it was away", async () => { + hotApp = await createHotApp({ transport, code: acceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + // Broken before the page is opened, so the overlay can only come from + // the catch-up a newly connected client is sent. + hotApp.edit("broken before connecting {{{"); + await page.reload(); + + const frame = await waitForOverlay(page); + + expect(await frame.evaluate(() => document.body.textContent)).toContain( + "Module parse failed", + ); + }); + }); +} diff --git a/test/helpers/hot-app.js b/test/helpers/hot-app.js index 171be5799..bec754ab2 100644 --- a/test/helpers/hot-app.js +++ b/test/helpers/hot-app.js @@ -112,7 +112,11 @@ function makeConfig( * app becomes a named compilation whose client connects with `?name=` * and renders from `.js`. `pageHeaders` are sent with the HTML page * (e.g. a Content-Security-Policy). - * @param {{ query?: string, code?: string, files?: Record, apps?: { name: string, code: string }[], hot?: EXPECTED_ANY, stats?: EXPECTED_ANY, pageHeaders?: Record, publicPath?: string, setup?: (server: EXPECTED_ANY) => void, hmrPlugin?: boolean }} options options + * Pass `transport: "ws"` to serve the events over a WebSocket instead of + * Server-Sent Events: the middleware is told to, the client is asked for the + * matching transport, and the HTTP server is handed over so it can answer the + * upgrade. + * @param {{ query?: string, code?: string, files?: Record, apps?: { name: string, code: string }[], hot?: EXPECTED_ANY, stats?: EXPECTED_ANY, pageHeaders?: Record, publicPath?: string, setup?: (server: EXPECTED_ANY) => void, hmrPlugin?: boolean, transport?: ("sse" | "ws") }} options options * @returns {Promise} handles for the running app */ async function createHotApp({ @@ -126,6 +130,7 @@ async function createHotApp({ publicPath = "/", setup, hmrPlugin = true, + transport = "sse", }) { const dir = fs.mkdtempSync( path.join(fs.realpathSync.native(os.tmpdir()), "wdm-e2e-"), @@ -152,6 +157,13 @@ async function createHotApp({ /** @type {string[]} */ let scripts; + // The client picks its transport from its own query, so it has to be told + // the same thing the middleware was. + const clientQuery = + transport === "ws" + ? `?transport=ws${query ? `&${query.replace(/^\?/, "")}` : ""}` + : query; + if (apps) { config = apps.map((app) => { // One context dir per compilation: editing one app's entry must not @@ -161,7 +173,7 @@ async function createHotApp({ fs.mkdirSync(appDir, { recursive: true }); entryFiles[app.name] = path.join(appDir, "entry.js"); fs.writeFileSync(entryFiles[app.name], app.code); - return makeConfig(app.name, appDir, entryFiles[app.name], query); + return makeConfig(app.name, appDir, entryFiles[app.name], clientQuery); }); scripts = apps.map((app) => `/${app.name}.js`); } else { @@ -171,7 +183,7 @@ async function createHotApp({ "", dir, entryFiles[""], - query, + clientQuery, publicPath, hmrPlugin, ); @@ -189,7 +201,13 @@ async function createHotApp({ } }); - instance = middleware(compiler, { hot, stats }); + instance = middleware(compiler, { + hot: + transport === "ws" && hot + ? { ...(hot === true ? {} : hot), transport: "ws" } + : hot, + stats, + }); const app = express(); @@ -228,6 +246,13 @@ async function createHotApp({ }); server = await listen(0); + + // A WebSocket handshake is an upgrade the HTTP server answers, which the + // middleware never sees — so it only works once the server is handed over. + if (transport === "ws") { + instance.attach(server); + } + const { port } = server.address(); await new Promise((resolve) => { From 22c6cbd612d37886aba909eff9b18aa3317a5451 Mon Sep 17 00:00:00 2001 From: alexander-akait <4567934+alexander-akait@users.noreply.github.com> Date: Thu, 24 Sep 2026 21:14:01 +0000 Subject: [PATCH 4/5] fix(client): close the gaps between the two transports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WebSocket one logged a raw `error`, which the Server-Sent Events one never did. Making both say it would have been the wrong way to agree: `EventSource` fires `error` on every routine reconnection, and a WebSocket `error` carries no detail by specification, so it prints an opaque object. Neither logs it now — the shared socket already reports the reconnection, which is the part worth reading. The WebSocket one also had no equivalent of the other's `closed` guard, so a close or message the browser had already queued still reached the caller after it had closed, and the shared socket would reconnect a connection nobody wanted. The contract they share is now tested once and run against each, which is what turned both of these up, and all three files are fully covered. --- .changeset/transport-parity.md | 5 + client-src/clients/EventSourceClient.js | 13 +- client-src/clients/WebSocketClient.js | 58 +++-- test/client-socket.test.js | 54 +++++ test/client-transports.test.js | 309 ++++++++++++++++++++++++ 5 files changed, 424 insertions(+), 15 deletions(-) create mode 100644 .changeset/transport-parity.md create mode 100644 test/client-transports.test.js diff --git a/.changeset/transport-parity.md b/.changeset/transport-parity.md new file mode 100644 index 000000000..d15e56568 --- /dev/null +++ b/.changeset/transport-parity.md @@ -0,0 +1,5 @@ +--- +"webpack-dev-middleware": patch +--- + +Make the two client transports behave alike: neither logs a raw connection error, and neither reports anything after being closed diff --git a/client-src/clients/EventSourceClient.js b/client-src/clients/EventSourceClient.js index eff8bd55c..2a8a9a2eb 100644 --- a/client-src/clients/EventSourceClient.js +++ b/client-src/clients/EventSourceClient.js @@ -8,6 +8,10 @@ const DEFAULT_TIMEOUT = 20 * 1000; * Server-Sent Events. A connection can die without the browser firing `error` * — a proxy that stops forwarding, a laptop that slept — so this one watches * for silence as well, and reports that as a close for the caller to reconnect. + * + * A failure is not logged here, for the same reason the WebSocket one does not + * log it: `error` fires on every routine reconnection, so saying so would be + * noise rather than news. * @implements {CommunicationClient} */ export default class EventSourceClient { @@ -31,6 +35,10 @@ export default class EventSourceClient { this.client = new window.EventSource(url); this.client.addEventListener("open", () => { + if (this.closed) { + return; + } + this.lastActivity = Date.now(); if (this.openHandler) { @@ -39,6 +47,10 @@ export default class EventSourceClient { }); this.client.addEventListener("message", (event) => { + if (this.closed) { + return; + } + this.lastActivity = Date.now(); if (this.messageHandler) { @@ -62,7 +74,6 @@ export default class EventSourceClient { * End this connection and report it, once. */ handleDisconnect() { - /* istanbul ignore next -- @preserve reached only by an event queued before close() */ if (this.closed) { return; } diff --git a/client-src/clients/WebSocketClient.js b/client-src/clients/WebSocketClient.js index 7f0efb0d6..8b7bda39b 100644 --- a/client-src/clients/WebSocketClient.js +++ b/client-src/clients/WebSocketClient.js @@ -1,5 +1,3 @@ -import { log } from "../utils/log.js"; - /** @typedef {import("./createSocket.js").CommunicationClient} CommunicationClient */ /** @typedef {import("./createSocket.js").ClientHandler} ClientHandler */ @@ -30,6 +28,11 @@ function toWebSocketURL(url) { * A WebSocket. The browser reports a dropped connection itself, and the server * pings to find a half-open one, so unlike Server-Sent Events this needs no * watchdog of its own. + * + * A failure is not logged here. The `error` event carries no detail by + * specification, so it would print an opaque object, and it is followed by the + * `close` the shared socket already reports and acts on — which is also all + * Server-Sent Events say, so neither transport is noisier than the other. * @implements {CommunicationClient} */ export default class WebSocketClient { @@ -37,9 +40,42 @@ export default class WebSocketClient { * @param {string} url url to connect to */ constructor(url) { + /** @type {ClientHandler | undefined} */ + this.openHandler = undefined; + /** @type {ClientHandler | undefined} */ + this.closeHandler = undefined; + /** @type {ClientHandler | undefined} */ + this.messageHandler = undefined; + // Set once closed, so an event the socket had already queued cannot report + // anything after the caller asked for none. + this.closed = false; + this.client = new WebSocket(toWebSocketURL(url)); - this.client.onerror = (error) => { - log.error(error); + + this.client.onopen = () => { + if (this.openHandler) { + this.openHandler(); + } + }; + + this.client.onclose = () => { + if (this.closed) { + return; + } + + if (this.closeHandler) { + this.closeHandler(); + } + }; + + this.client.onmessage = (event) => { + if (this.closed) { + return; + } + + if (this.messageHandler) { + this.messageHandler(event.data); + } }; } @@ -47,34 +83,28 @@ export default class WebSocketClient { * @param {ClientHandler} fn called once the connection is open */ onOpen(fn) { - this.client.onopen = () => { - fn(); - }; + this.openHandler = fn; } /** * @param {ClientHandler} fn called once the connection is gone */ onClose(fn) { - this.client.onclose = () => { - fn(); - }; + this.closeHandler = fn; } /** * @param {ClientHandler} fn called with each message, as a string */ onMessage(fn) { - this.client.onmessage = (event) => { - fn(event.data); - }; + this.messageHandler = fn; } /** * Close without reporting it, so the caller does not reconnect. */ close() { - this.client.onclose = null; + this.closed = true; this.client.close(); } } diff --git a/test/client-socket.test.js b/test/client-socket.test.js index da7b1b6b3..7f3c696a6 100644 --- a/test/client-socket.test.js +++ b/test/client-socket.test.js @@ -168,6 +168,25 @@ describe("createSocket", () => { expect(instances).toHaveLength(21); }); + it("backs off exponentially by default", () => { + const { FakeClient, instances } = createFakeClient(); + + // No `retryDelay`: the default doubles each time, so a server that is + // down does not get hammered once a second forever. + createSocket(FakeClient, "ws://localhost/hmr"); + + instances[0].closeHandler(); + jest.advanceTimersByTime(1100); + expect(instances).toHaveLength(2); + + instances[1].closeHandler(); + // The second wait is past two seconds, which the first was not. + jest.advanceTimersByTime(1100); + expect(instances).toHaveLength(2); + jest.advanceTimersByTime(1000); + expect(instances).toHaveLength(3); + }); + it("says it is reconnecting while the attempts are bounded", () => { const info = jest.spyOn(globalThis.console, "info").mockImplementation(); const { FakeClient, instances } = createFakeClient(); @@ -209,6 +228,41 @@ describe("createSocket", () => { info.mockRestore(); }); + it("takes an explicit choice about announcing retries", () => { + const info = jest.spyOn(globalThis.console, "info").mockImplementation(); + const { FakeClient, instances } = createFakeClient(); + + // Bounded attempts would announce themselves, but the caller said not to. + createSocket(FakeClient, "ws://localhost/hmr", { + logRetries: false, + retries: 2, + retryDelay: () => 1000, + }); + + instances[0].closeHandler(); + + expect(info).not.toHaveBeenCalledWith( + expect.stringContaining("Trying to reconnect"), + ); + + info.mockRestore(); + }); + + it("closes cleanly while waiting to reconnect", () => { + const { FakeClient, instances } = createFakeClient(); + const socket = createSocket(FakeClient, "ws://localhost/hmr", { + retryDelay: () => 1000, + }); + + // Closed with no live connection, only a pending timer — there is nothing + // to close, and the timer must not fire afterwards. + instances[0].closeHandler(); + socket.close(); + jest.advanceTimersByTime(10000); + + expect(instances).toHaveLength(1); + }); + it("stops reconnecting once closed", () => { const { FakeClient, instances } = createFakeClient(); const socket = createSocket(FakeClient, "ws://localhost/hmr", { diff --git a/test/client-transports.test.js b/test/client-transports.test.js new file mode 100644 index 000000000..d6522d1c0 --- /dev/null +++ b/test/client-transports.test.js @@ -0,0 +1,309 @@ +import EventSourceClient from "../client-src/clients/EventSourceClient"; +import WebSocketClient from "../client-src/clients/WebSocketClient"; + +// eslint-disable-next-line jsdoc/reject-any-type +/** @typedef {any} EXPECTED_OBJECT */ + +/** + * Stand in for the browser's `EventSource`, driven from the test. + * @returns {EXPECTED_OBJECT} the constructor and what it built + */ +function fakeEventSource() { + /** @type {EXPECTED_OBJECT[]} */ + const instances = []; + + globalThis.window = /** @type {EXPECTED_OBJECT} */ ({ + EventSource: function EventSource(url) { + /** @type {EXPECTED_OBJECT} */ + const source = { + url, + closed: false, + listeners: {}, + addEventListener(type, fn) { + source.listeners[type] = fn; + }, + close() { + source.closed = true; + }, + emit(type, event) { + if (source.listeners[type]) { + source.listeners[type](event); + } + }, + }; + + instances.push(source); + + return source; + }, + }); + + return instances; +} + +/** + * Stand in for the browser's `WebSocket`, plus the anchor the url resolver uses. + * @returns {EXPECTED_OBJECT} the sockets built + */ +function fakeWebSocket() { + /** @type {EXPECTED_OBJECT[]} */ + const instances = []; + + globalThis.document = /** @type {EXPECTED_OBJECT} */ ({ + createElement: () => { + /** @type {EXPECTED_OBJECT} */ + const anchor = {}; + + Object.defineProperty(anchor, "href", { + get: () => anchor.resolved, + set: (value) => { + anchor.resolved = /^[a-z]+:\/\//i.test(value) + ? value + : `https://example.test${value}`; + }, + }); + + return anchor; + }, + }); + + globalThis.WebSocket = /** @type {EXPECTED_OBJECT} */ ( + function WebSocket(url) { + /** @type {EXPECTED_OBJECT} */ + const socket = this; + + socket.url = url; + socket.closed = false; + socket.close = () => { + socket.closed = true; + }; + + instances.push(socket); + } + ); + + return instances; +} + +/** + * The two transports answer the same calls, so the cases below are written + * once and run against each. `emit` is how the test plays the browser. + */ +const transports = [ + { + name: "EventSourceClient", + Client: EventSourceClient, + setup: fakeEventSource, + teardown: () => { + delete globalThis.window; + }, + // `EventSource` has no `close` event — a dropped connection is an `error`, + // which is exactly the difference this shared contract hides. + emit: (instance, type, event) => + instance.emit(type === "close" ? "error" : type, event), + }, + { + name: "WebSocketClient", + Client: WebSocketClient, + setup: fakeWebSocket, + teardown: () => { + delete globalThis.document; + delete globalThis.WebSocket; + }, + emit: (instance, type, event) => { + const handler = { + open: "onopen", + close: "onclose", + message: "onmessage", + }; + + if (instance[handler[type]]) { + instance[handler[type]](event); + } + }, + }, +]; + +for (const { name, Client, setup, teardown, emit } of transports) { + describe(`${name} (the transport contract)`, () => { + /** @type {EXPECTED_OBJECT[]} */ + let instances; + + beforeEach(() => { + instances = setup(); + }); + + afterEach(() => { + teardown(); + jest.useRealTimers(); + }); + + it("reports the connection opening", () => { + const client = new Client("/__webpack_hmr"); + const opened = jest.fn(); + + client.onOpen(opened); + emit(instances[0], "open", {}); + + expect(opened).toHaveBeenCalledTimes(1); + }); + + it("reports the connection closing", () => { + const client = new Client("/__webpack_hmr"); + const closed = jest.fn(); + + client.onClose(closed); + emit(instances[0], "close", {}); + + expect(closed).toHaveBeenCalledTimes(1); + }); + + it("hands the message on as a string", () => { + const client = new Client("/__webpack_hmr"); + const received = jest.fn(); + + client.onMessage(received); + emit(instances[0], "message", { data: '{"action":"built"}' }); + + expect(received).toHaveBeenCalledWith('{"action":"built"}'); + }); + + it("tolerates an event arriving before anything is listening", () => { + const client = new Client("/__webpack_hmr"); + + // The caller registers its handlers after constructing, so an event in + // between must not throw. + expect(client).toBeDefined(); + expect(() => { + emit(instances[0], "open", {}); + emit(instances[0], "message", { data: "{}" }); + emit(instances[0], "close", {}); + }).not.toThrow(); + }); + + it("closes the underlying connection", () => { + const client = new Client("/__webpack_hmr"); + + client.close(); + + expect(instances[0].closed).toBe(true); + }); + + it("says nothing after being closed", () => { + const client = new Client("/__webpack_hmr"); + const closed = jest.fn(); + const received = jest.fn(); + + client.onClose(closed); + client.onMessage(received); + client.close(); + + // Whatever the browser had already queued must not reach the caller + // after it asked for none — or the shared socket reconnects a + // connection nobody wants. + emit(instances[0], "close", {}); + emit(instances[0], "message", { data: "{}" }); + + expect(closed).not.toHaveBeenCalled(); + expect(received).not.toHaveBeenCalled(); + }); + }); +} + +describe("EventSourceClient (what only it does)", () => { + /** + * @param {EXPECTED_OBJECT} instance fake source + * @param {string} type event type + * @param {EXPECTED_OBJECT} event event + */ + function emit(instance, type, event) { + instance.emit(type, event); + } + + /** @type {EXPECTED_OBJECT[]} */ + let instances; + + beforeEach(() => { + jest.useFakeTimers(); + instances = fakeEventSource(); + }); + + afterEach(() => { + delete globalThis.window; + jest.useRealTimers(); + }); + + it("reports a close when the connection falls silent", () => { + const client = new EventSourceClient("/__webpack_hmr", { timeout: 1000 }); + const closed = jest.fn(); + + client.onClose(closed); + + // A connection can die without the browser firing `error` at all, so + // silence past the timeout is the only thing that notices. The watchdog + // ticks twice per timeout, so the first tick past it is at 1500ms. + jest.advanceTimersByTime(1500); + + expect(closed).toHaveBeenCalledTimes(1); + expect(instances[0].closed).toBe(true); + }); + + it("keeps the connection while messages keep arriving", () => { + const client = new EventSourceClient("/__webpack_hmr", { timeout: 1000 }); + const closed = jest.fn(); + + client.onClose(closed); + + for (let i = 0; i < 4; i++) { + jest.advanceTimersByTime(600); + emit(instances[0], "message", { data: "💓" }); + } + + expect(closed).not.toHaveBeenCalled(); + }); + + it("reports a close once, however it was noticed", () => { + const client = new EventSourceClient("/__webpack_hmr", { timeout: 1000 }); + const closed = jest.fn(); + + client.onClose(closed); + instances[0].emit("error", {}); + // The watchdog is still armed when the error arrives; both must not + // report, or the shared socket schedules two reconnections. + jest.advanceTimersByTime(5000); + instances[0].emit("error", {}); + + expect(closed).toHaveBeenCalledTimes(1); + }); + + it("stops the watchdog when closed", () => { + const client = new EventSourceClient("/__webpack_hmr", { timeout: 1000 }); + const closed = jest.fn(); + + client.onClose(closed); + client.close(); + jest.advanceTimersByTime(60000); + + expect(closed).not.toHaveBeenCalled(); + }); + + it("ignores an open that arrives after it was closed", () => { + const client = new EventSourceClient("/__webpack_hmr", { timeout: 1000 }); + const opened = jest.fn(); + + client.onOpen(opened); + client.close(); + + // A connection that completed its handshake as the caller closed it would + // otherwise look open, and reset the watchdog on a dead source. + instances[0].emit("open", {}); + + expect(opened).not.toHaveBeenCalled(); + }); + + it("falls back to a default timeout", () => { + const client = new EventSourceClient("/__webpack_hmr"); + + expect(client.timeout).toBe(20000); + }); +}); From 3eb45805014357a6a9c122b3c38c427b35278d3c Mon Sep 17 00:00:00 2001 From: alexander-akait <4567934+alexander-akait@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:09:29 +0000 Subject: [PATCH 5/5] fix(overlay): keep focus where the user left it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps in the focus handling, both found in review. `document.activeElement` names a shadow host rather than the control focused inside it, and focusing a host that does not delegate focus does nothing at all — so the overlay dropped focus instead of handing it back. Descend into open roots before remembering what to restore. Paging through problems rebuilds the card, which destroys the very button that was clicked; focus then fell back to the frame's body, so a keyboard user lost the navigation after the first press. Each control is named across the re-render and the equivalent one takes focus again. The test helper could not have caught either over a WebSocket, because it could not restart a server at all: an upgraded socket is no longer the HTTP server's to close, so `stopHttp()` waited on a connected client forever, and the replacement server was never handed back to the transport. Both fixed, and the reconnect story now runs over both transports. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA --- .changeset/overlay-focus.md | 2 +- client-src/overlay.js | 54 +++++++++++++++++++-- test/client-transports.test.js | 4 ++ test/e2e/overlay.test.js | 85 ++++++++++++++++++++++++++++++++++ test/e2e/transport.test.js | 35 ++++++++++++++ test/helpers/hot-app.js | 44 +++++++++++++++--- 6 files changed, 213 insertions(+), 11 deletions(-) diff --git a/.changeset/overlay-focus.md b/.changeset/overlay-focus.md index 786a60194..a143a467d 100644 --- a/.changeset/overlay-focus.md +++ b/.changeset/overlay-focus.md @@ -2,4 +2,4 @@ "webpack-dev-middleware": patch --- -Move focus into the error overlay when it opens and give it back when it closes, and give its frame an accessible name +Move focus into the error overlay when it opens, keep it on the navigation while paging through problems, and give it back to whatever the page had focused — a control inside an open shadow root included — when it closes, and give its frame an accessible name diff --git a/client-src/overlay.js b/client-src/overlay.js index d35d29771..3a02ceff5 100644 --- a/client-src/overlay.js +++ b/client-src/overlay.js @@ -505,7 +505,21 @@ function ensureOverlay() { // Whatever the page had focused, so it can be given back — the overlay // takes focus to be reachable by keyboard, and is rude if it keeps it. - state.previousActiveElement = document.activeElement; + // `document.activeElement` names the shadow host rather than the control + // inside it, and focusing a host that does not delegate focus does nothing + // at all, so descend to the control itself. A closed root reports no + // `activeElement`, and there the host really is all there is to go back to. + let previouslyFocused = document.activeElement; + + while ( + previouslyFocused && + previouslyFocused.shadowRoot && + previouslyFocused.shadowRoot.activeElement + ) { + previouslyFocused = previouslyFocused.shadowRoot.activeElement; + } + + state.previousActiveElement = previouslyFocused; // Only the render that opens the overlay takes focus. Paginating re-renders // it, and moving focus then would take it off the button being clicked. state.focusOnRender = true; @@ -610,6 +624,17 @@ function renderProblems() { const { type, lines } = state.currentProblems; const paginated = paginate && lines.length > 1; + // Emptying the card destroys whatever it had focused, and the browser then + // falls back to the frame's body — so paging with the keyboard would lose + // the button being used after the very first press. Which control it was is + // remembered here and given back below, since the rebuilt one is a + // different element. + const previouslyFocused = frameDocument.activeElement; + const refocus = + previouslyFocused && card.contains(previouslyFocused) + ? previouslyFocused.getAttribute("data-control") + : null; + // Accent the top bar with the problem color (red for errors, yellow for warnings). card.style.borderTopColor = `#${problemColor(type)}`; setHTML(card, ""); @@ -619,6 +644,8 @@ function renderProblems() { closeButton.type = "button"; closeButton.textContent = "×"; closeButton.setAttribute("aria-label", "Close"); + // Names this control across the re-render that replaces it, for `refocus`. + closeButton.setAttribute("data-control", "close"); applyStyle(closeButton, closeButtonStyles); closeButton.addEventListener("click", () => { clear(); @@ -626,6 +653,8 @@ function renderProblems() { card.appendChild(closeButton); const visible = paginated ? [lines[state.pageIndex]] : lines; + /** @type {{ previous?: HTMLButtonElement, next?: HTMLButtonElement }} */ + const navButtons = {}; if (paginated) { // Header row: badge and the problem's first line (usually the file @@ -672,13 +701,15 @@ function renderProblems() { * @param {string} text button text * @param {number} delta page delta * @param {string} ariaLabel accessible label + * @param {string} control name kept across re-renders, for `refocus` * @returns {HTMLButtonElement} nav button */ - const makeNavButton = (text, delta, ariaLabel) => { + const makeNavButton = (text, delta, ariaLabel, control) => { const button = frameDocument.createElement("button"); button.type = "button"; button.textContent = text; button.setAttribute("aria-label", ariaLabel); + button.setAttribute("data-control", control); applyStyle(button, { border: "none", background: "transparent", @@ -699,9 +730,17 @@ function renderProblems() { counter.textContent = `${state.pageIndex + 1} / ${lines.length}`; applyStyle(counter, { color: "#f2f2f2" }); - nav.appendChild(makeNavButton("‹", -1, "Previous problem")); + navButtons.previous = makeNavButton( + "‹", + -1, + "Previous problem", + "previous", + ); + navButtons.next = makeNavButton("›", 1, "Next problem", "next"); + + nav.appendChild(navButtons.previous); nav.appendChild(counter); - nav.appendChild(makeNavButton("›", 1, "Next problem")); + nav.appendChild(navButtons.next); header.appendChild(badge); header.appendChild(nav); card.appendChild(header); @@ -750,6 +789,13 @@ function renderProblems() { if (state.focusOnRender) { state.focusOnRender = false; closeButton.focus(); + } else if (refocus) { + // The same control when this render still has one — a problem set that + // shrank back to a single page has no navigation left — and the close + // button otherwise, so focus stays inside the card either way. + ( + navButtons[/** @type {"previous" | "next"} */ (refocus)] || closeButton + ).focus(); } } diff --git a/test/client-transports.test.js b/test/client-transports.test.js index d6522d1c0..938779446 100644 --- a/test/client-transports.test.js +++ b/test/client-transports.test.js @@ -130,6 +130,10 @@ for (const { name, Client, setup, teardown, emit } of transports) { let instances; beforeEach(() => { + // `EventSourceClient` arms a watchdog in its constructor, and the cases + // below that never close their client would otherwise leave a real + // interval ticking after the file is done. + jest.useFakeTimers(); instances = setup(); }); diff --git a/test/e2e/overlay.test.js b/test/e2e/overlay.test.js index 5259b833d..b143ed6f4 100644 --- a/test/e2e/overlay.test.js +++ b/test/e2e/overlay.test.js @@ -230,6 +230,44 @@ describe("error overlay (browser)", () => { ).toBe("focus-me"); }); + it("hands focus back to a control inside a shadow root", async () => { + hotApp = await createHotApp({ code: acceptedApp("v1") }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + + await page.evaluate(() => { + const host = document.createElement("div"); + + document.body.appendChild(host); + + const input = document.createElement("input"); + + input.id = "in-shadow"; + host.attachShadow({ mode: "open" }).appendChild(input); + input.focus(); + }); + + hotApp.edit("broken while a shadow control is focused {{{"); + await waitForOverlay(page); + await page.keyboard.press("Escape"); + await waitForNoOverlay(page); + + // `document.activeElement` names the host rather than the control, and a + // host that does not delegate focus cannot be focused at all — so without + // descending into the root first, focus is simply dropped here. + expect( + await page.evaluate(() => { + const active = document.activeElement; + const inner = + active && active.shadowRoot && active.shadowRoot.activeElement; + + return inner ? inner.id : null; + }), + ).toBe("in-shadow"); + }); + it("dismisses on backdrop and close-button clicks, but not inside the card", async () => { hotApp = await createHotApp({ code: acceptedApp("v1") }); ({ page, browser } = await runBrowser()); @@ -661,6 +699,53 @@ describe("error overlay (browser)", () => { ); }); + it("keeps focus on the navigation across a page change", async () => { + hotApp = await createHotApp({ + code: ` + try { + require("./a"); + } catch (err) { + // expected + } + try { + require("./b"); + } catch (err) { + // expected + } + `, + files: { + "a.js": "broken a {{{", + "b.js": "broken b {{{", + }, + }); + ({ page, browser } = await runBrowser()); + + await page.goto(hotApp.url); + + const frame = await waitForOverlay(page); + + await frame.waitForFunction(() => + document.body.textContent.includes("1 / 2"), + ); + await frame.click('[aria-label="Previous problem"]'); + await frame.click('[aria-label="Next problem"]'); + await frame.waitForFunction(() => + document.body.textContent.includes("2 / 2"), + ); + + // Turning the page rebuilds the card, which destroys the very button that + // was clicked. Handing focus to its replacement is what keeps a keyboard + // user on the navigation rather than dropping them at the top of the card + // after the first press. + expect( + await frame.evaluate(() => + document.activeElement + ? document.activeElement.getAttribute("aria-label") + : null, + ), + ).toBe("Next problem"); + }); + it("shows the full problem list when paginate=false", async () => { hotApp = await createHotApp({ query: '?overlay={"paginate":false}', diff --git a/test/e2e/transport.test.js b/test/e2e/transport.test.js index a5c38d63e..f1cad4b15 100644 --- a/test/e2e/transport.test.js +++ b/test/e2e/transport.test.js @@ -90,6 +90,41 @@ for (const transport of ["sse", "ws"]) { expect(await page.$(`#${OVERLAY_ID}`)).toBeNull(); }); + it("reconnects after the server restarts", async () => { + hotApp = await createHotApp({ + transport, + query: "?timeout=1000", + code: acceptedApp("v1"), + // Heartbeats faster than the shortened timeout, so the inactivity + // watchdog does not churn disconnect/reconnect cycles mid-test. + hot: { heartbeat: 300 }, + }); + ({ page, browser } = await runBrowser()); + const console_ = collectConsole(page); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + await console_.waitFor("connected"); + + // Rebuilt while the server is down, so the update can only have arrived + // through the catch-up sync a reconnected client is sent. Restarting + // has to hand the new server to the transport again — a WebSocket + // upgrade is answered by the server, not by the middleware. + await hotApp.stopHttp(); + + const rebuilt = hotApp.nextBuild(); + + hotApp.edit(acceptedApp("v2")); + await rebuilt; + await hotApp.startHttp(); + + await waitForAppText(page, "v2"); + + expect( + await page.evaluate(() => document.getElementById("app").textContent), + ).toContain("v2"); + }); + it("catches a client up on what it missed while it was away", async () => { hotApp = await createHotApp({ transport, code: acceptedApp("v1") }); ({ page, browser } = await runBrowser()); diff --git a/test/helpers/hot-app.js b/test/helpers/hot-app.js index bec754ab2..9931b87e7 100644 --- a/test/helpers/hot-app.js +++ b/test/helpers/hot-app.js @@ -245,13 +245,42 @@ async function createHotApp({ }); }); - server = await listen(0); + /** @type {EXPECTED_ANY[]} */ + let upgradedSockets = []; - // A WebSocket handshake is an upgrade the HTTP server answers, which the - // middleware never sees — so it only works once the server is handed over. - if (transport === "ws") { - instance.attach(server); - } + /** + * A WebSocket handshake is an upgrade the HTTP server answers, which the + * middleware never sees — so it only works once the server is handed over. + * Every server this app listens on needs that again, the replacement one a + * restart brings up included. + * @param {EXPECTED_ANY} httpServer the server now serving this app + */ + const attachTransport = (httpServer) => { + if (transport !== "ws") { + return; + } + + instance.attach(httpServer); + // Once a socket is upgraded it stops being the HTTP server's to close, + // so `closeAllConnections()` does not reach it and a shutdown would + // wait on a connected client forever. Tracked here to be severed by + // hand, which is also what a client sees when a server really dies. + httpServer.on("upgrade", (/** @type {EXPECTED_ANY} */ _req, socket) => { + upgradedSockets.push(socket); + }); + }; + + /** Sever every upgraded connection this app is holding open. */ + const severUpgraded = () => { + for (const socket of upgradedSockets) { + socket.destroy(); + } + + upgradedSockets = []; + }; + + server = await listen(0); + attachTransport(server); const { port } = server.address(); @@ -328,6 +357,7 @@ async function createHotApp({ */ stopHttp() { return new Promise((resolve) => { + severUpgraded(); server.closeAllConnections(); server.close(() => resolve()); }); @@ -340,6 +370,7 @@ async function createHotApp({ */ async startHttp() { server = await listen(port); + attachTransport(server); }, /** @@ -354,6 +385,7 @@ async function createHotApp({ resolve(); return; } + severUpgraded(); server.closeAllConnections(); server.close(() => resolve()); });