diff --git a/.changeset/fix-beforeunload-cancel-blocks-reload.md b/.changeset/fix-beforeunload-cancel-blocks-reload.md new file mode 100644 index 0000000000..23bab78542 --- /dev/null +++ b/.changeset/fix-beforeunload-cancel-blocks-reload.md @@ -0,0 +1,5 @@ +--- +"webpack-dev-server": patch +--- + +Keep hot and live reload working after a cancelled "Leave site?" dialog. diff --git a/client-src/index.js b/client-src/index.js index cf6f88fc3a..c8c7adb9f1 100644 --- a/client-src/index.js +++ b/client-src/index.js @@ -276,8 +276,41 @@ const logEnabledFeatures = (features) => { logEnabledFeatures(enabledFeatures); +// `beforeunload` only means the page *may* be leaving: any listener can cancel +// it, and a cancelled unload fires no event of its own to say so. `pagehide` +// means it really is going, but only arrives once the next document loads, so +// nothing tells the two apart at a fixed moment — hence a grace period rather +// than suppressing until `pagehide`. Too short and a slow navigation can still +// be interrupted by a reload (#544); too long and a cancelled dialog leaves +// updates silently dropped (#5571). +const UNLOAD_GRACE_PERIOD = 2000; + +/** @type {ReturnType | undefined} */ +let unloadGraceTimer; + self.addEventListener("beforeunload", () => { status.isUnloading = true; + + clearTimeout(unloadGraceTimer); + + unloadGraceTimer = setTimeout(() => { + status.isUnloading = false; + }, UNLOAD_GRACE_PERIOD); +}); + +// The page really is going now, so stop reloading it for good. +self.addEventListener("pagehide", () => { + clearTimeout(unloadGraceTimer); + + status.isUnloading = true; +}); + +// Restored from the back/forward cache: the same script keeps running, so a +// flag left set by the navigation away would block updates from here on. +self.addEventListener("pageshow", () => { + clearTimeout(unloadGraceTimer); + + status.isUnloading = false; }); const overlay = diff --git a/test/client/index.test.js b/test/client/index.test.js index bc155e067b..9c2eea09d7 100644 --- a/test/client/index.test.js +++ b/test/client/index.test.js @@ -320,4 +320,79 @@ describe("index", () => { t.assert.snapshot(log.log.info.mock.calls[1][0]); t.assert.snapshot(sendMessage.mock.calls[0][0]); }); + + describe("unloading", () => { + // Longer than the client's own grace period, so a suppression that should + // have lapsed has had every chance to. + const AFTER_GRACE_PERIOD = 2500; + + const sleep = (ms) => + new Promise((resolve) => { + setTimeout(resolve, ms); + }); + + /** + * Drives a live reload to the point where only `isUnloading` can stop it. + * @returns {Promise} whether the page was reloaded + */ + async function pageReloads() { + self.location.reload.mockReset(); + + onSocketMessage.liveReload(); + onSocketMessage.hash(`hash-${Math.random()}`); + onSocketMessage.ok(); + + // The live reload path polls for a usable window on an interval rather + // than reloading straight away, so a reload that is coming needs a few + // turns, and one that is suppressed never arrives at all. + for (let i = 0; i < 20; i++) { + if (self.location.reload.mock.calls.length > 0) return true; + + await sleep(5); + } + + return false; + } + + it("should reload with no unload in progress", async () => { + expect(await pageReloads()).toBe(true); + }); + + it("should not reload while the page may be leaving", async () => { + self.dispatchEvent(new Event("beforeunload")); + + expect(await pageReloads()).toBe(false); + }); + + it("should reload again once a cancelled unload has lapsed", async () => { + self.dispatchEvent(new Event("beforeunload")); + + expect(await pageReloads()).toBe(false); + + // No `pagehide` follows a cancelled dialog, so this is the page staying. + await sleep(AFTER_GRACE_PERIOD); + + expect(await pageReloads()).toBe(true); + }); + + it("should not reload once the page is really gone", async () => { + self.dispatchEvent(new Event("beforeunload")); + self.dispatchEvent(new Event("pagehide")); + + await sleep(AFTER_GRACE_PERIOD); + + expect(await pageReloads()).toBe(false); + }); + + it("should reload after a restore from the back/forward cache", async () => { + self.dispatchEvent(new Event("beforeunload")); + self.dispatchEvent(new Event("pagehide")); + + expect(await pageReloads()).toBe(false); + + self.dispatchEvent(new Event("pageshow")); + + expect(await pageReloads()).toBe(true); + }); + }); }); diff --git a/test/e2e/overlay.test.js b/test/e2e/overlay.test.js index a175beec83..ac719643c8 100644 --- a/test/e2e/overlay.test.js +++ b/test/e2e/overlay.test.js @@ -372,16 +372,32 @@ describe("overlay", () => { }), ); + // Marks this document so the wait below can tell it from the reloaded one. + await page.evaluate(() => { + globalThis.documentFromBeforeTheFix = true; + }); + fs.writeFileSync(pathToOverlayFixture, overlayFixtureCode); - await page.waitForSelector("#webpack-dev-server-client-overlay", { - hidden: true, - }); + // This fixture builds, so the client dismisses the overlay when `invalid` + // announces the rebuild and then live reloads the page once that build + // lands. Reading on the dismiss reads the document from before the fix + // and races the reload that follows, which is what destroyed the + // execution context mid-evaluate. Wait for the reloaded page instead. + await waitForExpect(async () => { + const reloaded = await page.evaluate( + () => globalThis.documentFromBeforeTheFix === undefined, + ); - pageHtml = await page.evaluate(() => document.body.outerHTML); - overlayHandle = await page.$("#webpack-dev-server-client-overlay"); + expect(reloaded).toBe(true); + + overlayHandle = await page.$("#webpack-dev-server-client-overlay"); + + expect(overlayHandle).toBeNull(); + + pageHtml = await page.evaluate(() => document.body.outerHTML); + }, 60000); - expect(overlayHandle).toBeNull(); t.assert.snapshot( await format(pageHtml, { parser: "html", @@ -442,18 +458,29 @@ describe("overlay", () => { }), ); + const firstErrorOverlayHtml = overlayHtml; + fs.writeFileSync(pathToOverlayFixture, "`;a"); - await page.waitForSelector("#webpack-dev-server-client-overlay", { - hidden: true, - }); - await page.waitForSelector("#webpack-dev-server-client-overlay"); + // The `invalid` message announcing the rebuild dismisses the overlay and + // the errors it produces show it again, so the hidden state in between + // lasts only as long as that build and is not reliably observable — + // waiting for it is a race this test loses as a two minute timeout. What + // is under test is the second, different error reaching the overlay. + await waitForExpect(async () => { + overlayHandle = await page.$("#webpack-dev-server-client-overlay"); - overlayHandle = await page.$("#webpack-dev-server-client-overlay"); - pageHtml = await page.evaluate(() => document.body.outerHTML); + expect(overlayHandle).not.toBeNull(); + + overlayFrame = await overlayHandle.contentFrame(); + overlayHtml = await overlayFrame.evaluate( + () => document.body.outerHTML, + ); + + expect(overlayHtml).not.toBe(firstErrorOverlayHtml); + }, 60000); - overlayFrame = await overlayHandle.contentFrame(); - overlayHtml = await overlayFrame.evaluate(() => document.body.outerHTML); + pageHtml = await page.evaluate(() => document.body.outerHTML); t.assert.snapshot( await format(pageHtml, { @@ -466,16 +493,32 @@ describe("overlay", () => { }), ); + // Marks this document so the wait below can tell it from the reloaded one. + await page.evaluate(() => { + globalThis.documentFromBeforeTheFix = true; + }); + fs.writeFileSync(pathToOverlayFixture, overlayFixtureCode); - await page.waitForSelector("#webpack-dev-server-client-overlay", { - hidden: true, - }); + // This fixture builds, so the client dismisses the overlay when `invalid` + // announces the rebuild and then live reloads the page once that build + // lands. Reading on the dismiss reads the document from before the fix + // and races the reload that follows, which is what destroyed the + // execution context mid-evaluate. Wait for the reloaded page instead. + await waitForExpect(async () => { + const reloaded = await page.evaluate( + () => globalThis.documentFromBeforeTheFix === undefined, + ); - pageHtml = await page.evaluate(() => document.body.outerHTML); - overlayHandle = await page.$("#webpack-dev-server-client-overlay"); + expect(reloaded).toBe(true); + + overlayHandle = await page.$("#webpack-dev-server-client-overlay"); + + expect(overlayHandle).toBeNull(); + + pageHtml = await page.evaluate(() => document.body.outerHTML); + }, 60000); - expect(overlayHandle).toBeNull(); t.assert.snapshot( await format(pageHtml, { parser: "html",