diff --git a/test/bake/bake-harness.ts b/test/bake/bake-harness.ts index 3eb6ded89261..c968445737fe 100644 --- a/test/bake/bake-harness.ts +++ b/test/bake/bake-harness.ts @@ -319,28 +319,33 @@ export class Dev extends EventEmitter { const b = { write: resetSeenFilesWithResolvers, [Symbol.asyncDispose]: async () => { - if (wantsHmrEvent && interactive) { - await seenFiles.promise; - } else if (wantsHmrEvent) { - await Promise.race([seenFiles.promise]); - } - // One Bun.write can surface as several watcher events (notably on - // Windows); let them coalesce so releasing the batch bundles once. - await Bun.sleep(50); + // The batch ends even when the wait or the overlay check below throws, + // so a test that expects one write to reject can keep using `dev`. + try { + if (wantsHmrEvent && interactive) { + await seenFiles.promise; + } else if (wantsHmrEvent) { + await Promise.race([seenFiles.promise]); + } + // One Bun.write can surface as several watcher events (notably on + // Windows); let them coalesce so releasing the batch bundles once. + await Bun.sleep(50); - dev.off("watch_synchronization", onSeenFiles); + dev.off("watch_synchronization", onSeenFiles); - this.socket!.send("H"); - await wait; + this.socket!.send("H"); + await wait; - let errors = options.errors; - if (errors !== null) { - errors ??= []; - for (const client of this.connectedClients) { - await client.expectErrorOverlay(errors, options.snapshot); + let errors = options.errors; + if (errors !== null) { + errors ??= []; + for (const client of this.connectedClients) { + await client.expectErrorOverlay(errors, options.snapshot); + } } + } finally { + this.batchingChanges = null; } - this.batchingChanges = null; }, }; this.batchingChanges = b; @@ -547,7 +552,7 @@ export class Dev extends EventEmitter { this.output.on("panic", onPanic); if (this.nodeEnv === "development") { try { - await client.output.waitForLine(hmrClientInitRegex); + await client.waitForPageLoad(); } catch (e) { this.output.off("panic", onPanic); try { @@ -892,15 +897,40 @@ export class Client extends EventEmitter { return withAnnotatedStack(snapshotCallerLocation(), async () => { await maybeWaitInteractive("hard-reload"); if (this.exited) throw new Error("Client is not running."); - this.#proc.send({ type: "hard-reload" }); - - if (this.hmr) { - await this.output.waitForLine(hmrClientInitRegex); - await this.expectErrorOverlay(options.errors ?? []); + if (!this.hmr) { + this.#proc.send({ type: "hard-reload" }); + return; } + const loaded = this.waitForPageLoad(); + this.#proc.send({ type: "hard-reload" }); + await loaded; + await this.expectErrorOverlay(options.errors ?? []); }); } + /** + * Resolves once the page being loaded has connected its HMR socket and the + * fixture acked it, which it only does after the page's stylesheets loaded. + * Call this before the load starts so the ack cannot be missed. The line + * wait adds a timeout and the fixture's output when it dies while loading. + */ + async waitForPageLoad(): Promise { + const acked = Promise.withResolvers(); + const onAck = () => acked.resolve(); + const onExit = (code: number | string) => { + const mapped = exitCodeMapStrings[code]; + acked.reject(new Error(`Client exited while loading the page${mapped ? `: ${mapped}` : ` (${code})`}`)); + }; + this.once("received-hmr-event", onAck); + this.once("exit", onExit); + try { + await Promise.all([this.output.waitForLine(hmrClientInitRegex), acked.promise]); + } finally { + this.off("received-hmr-event", onAck); + this.off("exit", onExit); + } + } + elemText(selector: string): Promise { return withAnnotatedStack(snapshotCallerLocation(), async () => { const text = await this.js` @@ -913,6 +943,35 @@ export class Client extends EventEmitter { }); } + /** + * Waits until the element's innerHTML is `text`. For DOM that a framework + * updates asynchronously after the harness has already synchronized with the + * dev server, e.g. React committing a server-side route reload. + */ + expectElemText(selector: string, text: string): Promise { + return withAnnotatedStack(snapshotCallerLocation(), async () => { + await this.js` + const read = () => document.querySelector(${selector})?.innerHTML; + if (read() === ${text}) return; + await new Promise((resolve, reject) => { + const observer = new MutationObserver(() => { + if (read() !== ${text}) return; + observer.disconnect(); + clearTimeout(timer); + resolve(); + }); + // Observe the document itself: a re-render may replace rather + // than patch it, which an observer on the old root would never see. + observer.observe(document, { subtree: true, childList: true, characterData: true }); + const timer = setTimeout(() => { + observer.disconnect(); + reject(new Error("Expected " + ${selector} + " to become " + JSON.stringify(${text}) + ", last saw " + JSON.stringify(read()))); + }, ${interactive ? interactive_timeout : 2000 * WAIT_MULTIPLIER}); + }); + `; + }); + } + elemsText(selector: string): Promise { return withAnnotatedStack(snapshotCallerLocation(), async () => { const elems = await this.js< @@ -1046,61 +1105,45 @@ export class Client extends EventEmitter { expectErrorOverlay(errors: ErrorSpec[], caller: string | null = null) { return withAnnotatedStack(caller ?? snapshotCallerLocationMayFail(), async () => { this.suppressInteractivePrompt = true; - let retries = 0; - let hasVisibleModal = false; - while (retries < 5) { - hasVisibleModal = await this.js`document.querySelector("bun-hmr")?.style.display === "block"`; - if (hasVisibleModal) break; - await Bun.sleep(200); - retries++; - } - this.suppressInteractivePrompt = false; - if (errors && errors.length > 0) { - if (!hasVisibleModal) { - await maybeWaitInteractive("expectErrorOverlay"); - throw new Error("Expected errors, but none found"); - } - - // Create unique message ID for this evaluation - const messageId = Math.random().toString(36).slice(2); - - // Send the evaluation request and wait for response - this.#proc.send({ - type: "get-errors", - args: [messageId], - }); - - const [result] = await EventEmitter.once(this, `get-errors-result-${messageId}`); - - if (result.error) { - throw new Error(result.error); + let hasVisibleModal: boolean; + try { + hasVisibleModal = await this.#hasVisibleErrorOverlay(); + // Build errors are already on the page when callers get here: the error + // page renders them before its socket connects, and after a write the + // fixture acks the build only after the runtime handled the errors + // frame. Only runtime errors reach the overlay later (the runtime remaps + // them through /_bun/report_error first), so polling is only useful when + // errors are expected. + for (let retries = 0; errors.length > 0 && !hasVisibleModal && retries < 5; retries++) { + await Bun.sleep(200); + hasVisibleModal = await this.#hasVisibleErrorOverlay(); } - const actualErrors = result.value; - const expectedErrors = [...errors].sort(); - expect(actualErrors).toEqual(expectedErrors); - } else { - if (hasVisibleModal) { - // Create unique message ID for this evaluation - const messageId = Math.random().toString(36).slice(2); - - // Send the evaluation request and wait for response - this.#proc.send({ - type: "get-errors", - args: [messageId], - }); - - const [result] = await EventEmitter.once(this, `get-errors-result-${messageId}`); + } finally { + this.suppressInteractivePrompt = false; + } + if (!hasVisibleModal) { + if (errors.length === 0) return; + await maybeWaitInteractive("expectErrorOverlay"); + throw new Error("Expected errors, but none found"); + } - if (result.error) { - throw new Error(result.error); - } - const actualErrors = result.value; - expect(actualErrors).toEqual([]); - } + const messageId = Math.random().toString(36).slice(2); + this.#proc.send({ + type: "get-errors", + args: [messageId], + }); + const [result] = await EventEmitter.once(this, `get-errors-result-${messageId}`); + if (result.error) { + throw new Error(result.error); } + expect(result.value).toEqual([...errors].sort()); }); } + #hasVisibleErrorOverlay() { + return this.js`document.querySelector("bun-hmr")?.style.display === "block"`; + } + getStringMessage(): Promise { return withAnnotatedStack(snapshotCallerLocation(), async () => { if (this.messages.length === 0) { diff --git a/test/bake/client-fixture.mjs b/test/bake/client-fixture.mjs index 72733b118502..218df60b719a 100644 --- a/test/bake/client-fixture.mjs +++ b/test/bake/client-fixture.mjs @@ -32,15 +32,16 @@ let expectingReload = false; let webSockets = []; let pendingReload = null; let pendingReloadTimer = null; -let isUpdating = null; +// Bumped whenever the current window is abandoned (reload requested, or a new +// window created). Acks captured by an older window are dropped: the harness +// expects exactly one "received-hmr-event" per build per client, and after a +// reload that one ack comes from the new window once it has connected. +let windowGeneration = 0; let objectURLRegistry = new Map(); let internalAPIs; function reset() { - if (isUpdating !== null) { - clearImmediate(isUpdating); - isUpdating = null; - } + windowGeneration++; for (const ws of webSockets) { ws.onclose = () => {}; ws.onerror = () => {}; @@ -71,15 +72,21 @@ function createWindow(windowUrl) { height: 768, }); + const generation = ++windowGeneration; + const ackToHarness = () => { + if (generation !== windowGeneration) return; + process.send({ type: "received-hmr-event", args: [] }); + }; + // The HMR runtime reads this symbol-keyed callback off `globalThis` (which is // `window` inside happy-dom's script context) and passes its internal hooks. let hmrEventHookInstalled = false; - let pendingHotUpdateAcks = 0; + let pendingBuildAcks = 0; let hmrScriptQueued = false; - const sendHmrAck = () => { - if (pendingHotUpdateAcks === 0) return; - pendingHotUpdateAcks--; - process.send({ type: "received-hmr-event", args: [] }); + const ackBuild = () => { + if (pendingBuildAcks === 0) return; + pendingBuildAcks--; + ackToHarness(); }; window[Symbol.for("bun testing api, may change at any time")] = internal => { window.internal = internal; @@ -88,9 +95,10 @@ function createWindow(windowUrl) { // Ack a hot update only once the new module code has actually run. Node's // Blob.arrayBuffer() resolves on a later macrotask than the WS listener's // setImmediate, so acking from the WS listener would race the eval. - // Full reloads are not acked here; the new window acks from the - // `[Bun] Hot-module-reloading socket connected` handler after loadPage. - internal.onEvent("bun:afterUpdate", sendHmrAck); + // Updates that end in a full reload are acked by the new window's + // "socket connected" handler instead; this window's generation is stale + // by the time its runtime gets here. + internal.onEvent("bun:afterUpdate", ackBuild); } }; @@ -110,25 +118,29 @@ function createWindow(windowUrl) { webSockets.push(this); this.addEventListener("message", event => { const data = new Uint8Array(event.data); - if (data[0] === "u".charCodeAt(0) && hmrEventHookInstalled) { + const kind = String.fromCharCode(data[0]); + // One ack per build, for the last frame it sends this page. finalize_bundle + // (DevServer.rs) publishes errors ("e") before the hot update ("u") and, + // while a page running the HMR runtime is connected, always ends with + // "u", so those pages ack "u" only; acking "e" would release the harness + // before the update behind it is applied. The bundling error page only + // subscribes to errors, so its last frame is "e", applied synchronously + // by the runtime's own listener right after this one. ("u" without the + // hook means the runtime lost its onEvent testing export; acking keeps + // that a test failure instead of a hang.) + if (hmrEventHookInstalled ? kind === "u" : kind === "e" || kind === "u") { // JS updates queue a script tag and ack via bun:afterUpdate once it - // evals; everything else (CSS, reloads, route reloads) acks here on - // the next tick when no script was queued. - pendingHotUpdateAcks++; + // evals; everything else (CSS, errors, server-side route reloads) + // acks on the next tick when no script was queued. + pendingBuildAcks++; hmrScriptQueued = false; - isUpdating = setImmediate(() => { - isUpdating = null; - if (!hmrScriptQueued) sendHmrAck(); - }); - } else if (data[0] === "e".charCodeAt(0) || data[0] === "u".charCodeAt(0)) { - isUpdating = setImmediate(() => { - process.send({ type: "received-hmr-event", args: [] }); - isUpdating = null; + setImmediate(() => { + if (!hmrScriptQueued) ackBuild(); }); } if (!allowWebSocketMessages) { const allowedTypes = ["n", "r"]; - if (allowedTypes.includes(String.fromCharCode(data[0]))) { + if (allowedTypes.includes(kind)) { return; } dumpWebSocketMessage("[E] WebSocket message received while messages are not allowed", data); @@ -230,9 +242,7 @@ function createWindow(windowUrl) { // If no stylesheets of any kind, just emit the event if (styleLinks.length === 0 && styleTags.length === 0 && adoptedSheets.length === 0) { - process.nextTick(() => { - process.send({ type: "received-hmr-event", args: [] }); - }); + process.nextTick(ackToHarness); return; } @@ -270,9 +280,7 @@ function createWindow(windowUrl) { if (checkAttempts >= MAX_CHECK_ATTEMPTS && !allLoaded) { console.warn("[W] Reached maximum CSS load check attempts, proceeding anyway"); } - process.nextTick(() => { - process.send({ type: "received-hmr-event", args: [] }); - }); + process.nextTick(ackToHarness); } else { // Wait a bit and check again console.info( diff --git a/test/bake/dev-and-prod.test.ts b/test/bake/dev-and-prod.test.ts index 26c769669420..3f1670194fd3 100644 --- a/test/bake/dev-and-prod.test.ts +++ b/test/bake/dev-and-prod.test.ts @@ -295,10 +295,11 @@ devTest("hmr handles rapid consecutive edits", { // `num_subscribers(HotUpdate) == 0` / `active_viewers == 0` and the // hot_update is dropped server-side (DevServer.rs finalize_bundle), so // the sentinel never reaches the client. Re-writing on each - // `received-hmr-event` (which fires on socket open and on every 'u'/'e' - // WS frame) guarantees that at least one sentinel write lands after the - // server has a subscriber. The same-content writes are idempotent and - // the loop terminates the moment waitForMessage resolves below. + // `received-hmr-event` (which the fixture sends when a page's socket has + // connected and once per hot update it applies) guarantees that at least + // one sentinel write lands after the server has a subscriber. The + // same-content writes are idempotent and the loop terminates the moment + // waitForMessage resolves below. const sentinelContent = hmrSelfAcceptingModule("render sentinel"); const rewriteSentinel = () => writeFileSync(target, sentinelContent); client.on("reload", rewriteSentinel); diff --git a/test/bake/dev/hot.test.ts b/test/bake/dev/hot.test.ts index 92e158155dea..e17782029e14 100644 --- a/test/bake/dev/hot.test.ts +++ b/test/bake/dev/hot.test.ts @@ -1,5 +1,6 @@ // Hot tests ensure that the `import.meta.hot` interface is functional import { expect } from "bun:test"; +import { runWithErrorPromise } from "harness"; import { renameSync, unlinkSync, writeFileSync } from "node:fs"; import { devTest, emptyHtmlFile } from "../bake-harness"; @@ -633,12 +634,71 @@ devTest("dev.write resolves only after the new module body has run", { globalThis.marker = "updated"; import.meta.hot.accept(); `, - // errors: null skips the post-write expectErrorOverlay poll (5 * 200ms), - // which would otherwise mask a premature ack. - { errors: null }, ); // dev.write resolves on bun:afterUpdate, i.e. after replaceModules has // awaited the 500ms TLA. Acking on WS receipt would see "initial" here. expect(await c.js`globalThis.marker`).toBe("updated"); }, }); + +devTest("dev.client rejects when the page has a build error the test did not expect", { + files: { + "index.html": emptyHtmlFile({ scripts: ["index.ts"] }), + "index.ts": `import "./missing";`, + }, + async test(dev) { + // runWithErrorPromise rather than expect().rejects: on Windows that matcher + // does not deliver the client's IPC messages while it waits, so anything + // the harness resolves from an IPC message hangs inside it. + const error = await runWithErrorPromise(() => dev.client("/")); + expect(error?.message).toContain("index.ts:1:8: error: Could not resolve"); + }, +}); + +devTest("dev.write rejects on an unexpected build error and resolves once the fix has been applied", { + files: { + "index.html": emptyHtmlFile({ scripts: ["index.ts"] }), + "index.ts": ` + globalThis.marker = "initial"; + import.meta.hot.accept(); + `, + }, + async test(dev) { + await using c = await dev.client("/"); + expect(await c.js`globalThis.marker`).toBe("initial"); + + const error = await runWithErrorPromise(() => dev.write("index.ts", `import "./missing";`)); + expect(error?.message).toContain("index.ts:1:8: error: Could not resolve"); + + // The build that fixes the error sends an errors frame (hiding the overlay) + // followed by the hot update. dev.write must resolve on the update, after + // the 200ms top-level await below, not as soon as the overlay is gone. + await dev.write( + "index.ts", + ` + await new Promise(r => setTimeout(r, 200)); + globalThis.marker = "fixed"; + import.meta.hot.accept(); + `, + ); + expect(await c.js`globalThis.marker`).toBe("fixed"); + }, +}); + +devTest("dev.write resolves only after a reload it triggers has loaded the new page", { + files: { + "index.html": emptyHtmlFile({ scripts: ["index.ts"] }), + // Not self-accepting, so updating it falls back to a full reload. + "index.ts": `globalThis.marker = "v1";`, + }, + async test(dev) { + await using c = await dev.client("/"); + expect(await c.js`globalThis.marker`).toBe("v1"); + + await c.expectReload(() => dev.write("index.ts", `globalThis.marker = "v2";`)); + // This build is acked by the reloaded page once it has connected, so its + // scripts have run. The abandoned page still evaluates the update and emits + // bun:afterUpdate before that; acking from there would see undefined here. + expect(await c.js`globalThis.marker`).toBe("v2"); + }, +}); diff --git a/test/bake/dev/ssg-pages-router.test.ts b/test/bake/dev/ssg-pages-router.test.ts index f8b2fbe8731f..a3512ac02c60 100644 --- a/test/bake/dev/ssg-pages-router.test.ts +++ b/test/bake/dev/ssg-pages-router.test.ts @@ -157,7 +157,9 @@ devTest("SSG pages router - hot reload on page changes", { // this %c%s%c is a react devtools thing and I don't know how to turn it off await c.expectMessage("%c%s%c updated load"); - expect(await c.elemText("h1")).toBe("Updated Content"); + // dev.write() resolves once the client received the route reload; fetching + // the new page and committing it is asynchronous on the framework side. + await c.expectElemText("h1", "Updated Content"); }, });