From e852415cd515f421f82dc3be56eb6e9dae192757 Mon Sep 17 00:00:00 2001 From: Nathan Summers Date: Fri, 11 Jun 2021 15:01:35 -0700 Subject: [PATCH 1/2] Allow HMR status handlers to return a Promise The HMR system will wait until the promise settles before continuing. --- lib/hmr/HotModuleReplacement.runtime.js | 164 +++++++++--------- test/HotTestCases.template.js | 3 +- test/hotCases/invalidate/during-idle/index.js | 11 +- test/hotCases/status/accept/file.js | 3 + test/hotCases/status/accept/index.js | 21 +++ 5 files changed, 116 insertions(+), 86 deletions(-) create mode 100644 test/hotCases/status/accept/file.js create mode 100644 test/hotCases/status/accept/index.js diff --git a/lib/hmr/HotModuleReplacement.runtime.js b/lib/hmr/HotModuleReplacement.runtime.js index d3e8a0a612b..90ea15163df 100644 --- a/lib/hmr/HotModuleReplacement.runtime.js +++ b/lib/hmr/HotModuleReplacement.runtime.js @@ -9,6 +9,7 @@ var $interceptModuleExecution$ = undefined; var $moduleCache$ = undefined; // eslint-disable-next-line no-unused-vars var $hmrModuleData$ = undefined; +/** @type {() => Promise} */ var $hmrDownloadManifest$ = undefined; var $hmrDownloadUpdateHandlers$ = undefined; var $hmrInvalidateModuleHandlers$ = undefined; @@ -209,8 +210,12 @@ module.exports = function () { function setStatus(newStatus) { currentStatus = newStatus; + var results = []; + for (var i = 0; i < registeredStatusHandlers.length; i++) - registeredStatusHandlers[i].call(null, newStatus); + results[i] = registeredStatusHandlers[i].call(null, newStatus); + + return Promise.all(results); } function trackBlockingPromise(promise) { @@ -219,7 +224,7 @@ module.exports = function () { setStatus("prepare"); blockingPromises.push(promise); waitForBlockingPromises(function () { - setStatus("ready"); + return setStatus("ready"); }); return promise; case "prepare": @@ -243,47 +248,47 @@ module.exports = function () { if (currentStatus !== "idle") { throw new Error("check() is only allowed in idle status"); } - setStatus("check"); - return $hmrDownloadManifest$().then(function (update) { - if (!update) { - setStatus(applyInvalidatedModules() ? "ready" : "idle"); - return null; - } - - setStatus("prepare"); - - var updatedModules = []; - blockingPromises = []; - currentUpdateApplyHandlers = []; - - return Promise.all( - Object.keys($hmrDownloadUpdateHandlers$).reduce(function ( - promises, - key - ) { - $hmrDownloadUpdateHandlers$[key]( - update.c, - update.r, - update.m, - promises, - currentUpdateApplyHandlers, - updatedModules - ); - return promises; - }, - []) - ).then(function () { - return waitForBlockingPromises(function () { - if (applyOnUpdate) { - return internalApply(applyOnUpdate); - } else { - setStatus("ready"); + return setStatus("check") + .then($hmrDownloadManifest$) + .then(function (update) { + if (!update) { + return setStatus(applyInvalidatedModules() ? "ready" : "idle"); + } - return updatedModules; - } + return setStatus("prepare").then(function () { + var updatedModules = []; + blockingPromises = []; + currentUpdateApplyHandlers = []; + + return Promise.all( + Object.keys($hmrDownloadUpdateHandlers$).reduce(function ( + promises, + key + ) { + $hmrDownloadUpdateHandlers$[key]( + update.c, + update.r, + update.m, + promises, + currentUpdateApplyHandlers, + updatedModules + ); + return promises; + }, + []) + ).then(function () { + return waitForBlockingPromises(function () { + if (applyOnUpdate) { + return internalApply(applyOnUpdate); + } else { + return setStatus("ready").then(function () { + return updatedModules; + }); + } + }); + }); }); }); - }); } function hotApply(options) { @@ -312,58 +317,57 @@ module.exports = function () { .filter(Boolean); if (errors.length > 0) { - setStatus("abort"); - return Promise.resolve().then(function () { + return setStatus("abort").then(function () { throw errors[0]; }); } // Now in "dispose" phase - setStatus("dispose"); - - results.forEach(function (result) { - if (result.dispose) result.dispose(); - }); - - // Now in "apply" phase - setStatus("apply"); - - var error; - var reportError = function (err) { - if (!error) error = err; - }; + return setStatus("dispose").then(function () { + results.forEach(function (result) { + if (result.dispose) result.dispose(); + }); - var outdatedModules = []; - results.forEach(function (result) { - if (result.apply) { - var modules = result.apply(reportError); - if (modules) { - for (var i = 0; i < modules.length; i++) { - outdatedModules.push(modules[i]); + // Now in "apply" phase + return setStatus("apply").then(function () { + var error; + var reportError = function (err) { + if (!error) error = err; + }; + + var outdatedModules = []; + results.forEach(function (result) { + if (result.apply) { + var modules = result.apply(reportError); + if (modules) { + for (var i = 0; i < modules.length; i++) { + outdatedModules.push(modules[i]); + } + } } + }); + + // handle errors in accept handlers and self accepted module load + if (error) { + return setStatus("fail").then(function () { + throw error; + }); } - } - }); - // handle errors in accept handlers and self accepted module load - if (error) { - setStatus("fail"); - return Promise.resolve().then(function () { - throw error; - }); - } + if (queuedInvalidatedModules) { + return internalApply(options).then(function (list) { + outdatedModules.forEach(function (moduleId) { + if (list.indexOf(moduleId) < 0) list.push(moduleId); + }); + return list; + }); + } - if (queuedInvalidatedModules) { - return internalApply(options).then(function (list) { - outdatedModules.forEach(function (moduleId) { - if (list.indexOf(moduleId) < 0) list.push(moduleId); + return setStatus("idle").then(function () { + return outdatedModules; }); - return list; }); - } - - setStatus("idle"); - return Promise.resolve(outdatedModules); + }); } function applyInvalidatedModules() { diff --git a/test/HotTestCases.template.js b/test/HotTestCases.template.js index 48db82c06e1..81152fa4408 100644 --- a/test/HotTestCases.template.js +++ b/test/HotTestCases.template.js @@ -250,7 +250,7 @@ const describeCases = config => { return JSON.parse(fs.readFileSync(p, "utf-8")); } else { const fn = vm.runInThisContext( - "(function(require, module, exports, __dirname, __filename, it, beforeEach, afterEach, expect, self, window, fetch, document, importScripts, Worker, EventSource, NEXT, STATS) {" + + "(function(require, module, exports, __dirname, __filename, it, beforeEach, afterEach, expect, jest, self, window, fetch, document, importScripts, Worker, EventSource, NEXT, STATS) {" + "global.expect = expect;" + 'function nsObj(m) { Object.defineProperty(m, Symbol.toStringTag, { value: "Module" }); return m; }' + fs.readFileSync(p, "utf-8") + @@ -271,6 +271,7 @@ const describeCases = config => { _beforeEach, _afterEach, expect, + jest, window, window, window.fetch, diff --git a/test/hotCases/invalidate/during-idle/index.js b/test/hotCases/invalidate/during-idle/index.js index 1a406401b66..89a1da3b35d 100644 --- a/test/hotCases/invalidate/during-idle/index.js +++ b/test/hotCases/invalidate/during-idle/index.js @@ -11,9 +11,10 @@ it("should allow to invalidate and reload a file", () => { expect(module.hot.status()).toBe("ready"); c.invalidate(); expect(module.hot.status()).toBe("ready"); - module.hot.apply(); - expect(module.hot.status()).toBe("idle"); - expect(a.value).not.toBe(oldA); - expect(b.value).not.toBe(oldB); - expect(c.value).toBe(oldC); + module.hot.apply().then(function () { + expect(module.hot.status()).toBe("idle"); + expect(a.value).not.toBe(oldA); + expect(b.value).not.toBe(oldB); + expect(c.value).toBe(oldC); + }); }); diff --git a/test/hotCases/status/accept/file.js b/test/hotCases/status/accept/file.js new file mode 100644 index 00000000000..77e3c4ea564 --- /dev/null +++ b/test/hotCases/status/accept/file.js @@ -0,0 +1,3 @@ +module.exports = 1; +--- +module.exports = 2; diff --git a/test/hotCases/status/accept/index.js b/test/hotCases/status/accept/index.js new file mode 100644 index 00000000000..f58bf67f6e4 --- /dev/null +++ b/test/hotCases/status/accept/index.js @@ -0,0 +1,21 @@ +var value = require("./file"); + +it("should wait until promises returned by status handlers are fulfilled", (done) => { + var handler = jest.fn(status => { + return Promise.resolve().then(() => { + expect(status).toBe(module.hot.status()); + }); + }); + module.hot.addStatusHandler(handler); + module.hot.accept("./file", () => { + value = require("./file"); + done(); + }); + NEXT(require("../../update")(done, undefined, () => { + expect(module.hot.status()).toBe("idle"); + + expect(handler.mock.calls).toBe([['check'], ['prepare'], ['dispose'], ['apply'], ['idle']]); + done(); +})); +}); + From ef4aa56755d7a05f7caf048706fcd94c1bb9eda6 Mon Sep 17 00:00:00 2001 From: Nathan Summers Date: Mon, 21 Jun 2021 17:21:23 -0700 Subject: [PATCH 2/2] Dispose and apply must run atomically. --- lib/hmr/HotModuleReplacement.runtime.js | 76 +++++++++++++------------ test/hotCases/status/accept/index.js | 20 ++++--- 2 files changed, 52 insertions(+), 44 deletions(-) diff --git a/lib/hmr/HotModuleReplacement.runtime.js b/lib/hmr/HotModuleReplacement.runtime.js index 90ea15163df..a92d27d7f40 100644 --- a/lib/hmr/HotModuleReplacement.runtime.js +++ b/lib/hmr/HotModuleReplacement.runtime.js @@ -323,49 +323,51 @@ module.exports = function () { } // Now in "dispose" phase - return setStatus("dispose").then(function () { - results.forEach(function (result) { - if (result.dispose) result.dispose(); - }); + var disposePromise = setStatus("dispose"); - // Now in "apply" phase - return setStatus("apply").then(function () { - var error; - var reportError = function (err) { - if (!error) error = err; - }; - - var outdatedModules = []; - results.forEach(function (result) { - if (result.apply) { - var modules = result.apply(reportError); - if (modules) { - for (var i = 0; i < modules.length; i++) { - outdatedModules.push(modules[i]); - } - } - } - }); + results.forEach(function (result) { + if (result.dispose) result.dispose(); + }); - // handle errors in accept handlers and self accepted module load - if (error) { - return setStatus("fail").then(function () { - throw error; - }); - } + // Now in "apply" phase + var applyPromise = setStatus("apply"); - if (queuedInvalidatedModules) { - return internalApply(options).then(function (list) { - outdatedModules.forEach(function (moduleId) { - if (list.indexOf(moduleId) < 0) list.push(moduleId); - }); - return list; - }); + var error; + var reportError = function (err) { + if (!error) error = err; + }; + + var outdatedModules = []; + results.forEach(function (result) { + if (result.apply) { + var modules = result.apply(reportError); + if (modules) { + for (var i = 0; i < modules.length; i++) { + outdatedModules.push(modules[i]); + } } + } + }); + + return Promise.all([disposePromise, applyPromise]).then(function () { + // handle errors in accept handlers and self accepted module load + if (error) { + return setStatus("fail").then(function () { + throw error; + }); + } - return setStatus("idle").then(function () { - return outdatedModules; + if (queuedInvalidatedModules) { + return internalApply(options).then(function (list) { + outdatedModules.forEach(function (moduleId) { + if (list.indexOf(moduleId) < 0) list.push(moduleId); + }); + return list; }); + } + + return setStatus("idle").then(function () { + return outdatedModules; }); }); } diff --git a/test/hotCases/status/accept/index.js b/test/hotCases/status/accept/index.js index f58bf67f6e4..597a2bd0ec3 100644 --- a/test/hotCases/status/accept/index.js +++ b/test/hotCases/status/accept/index.js @@ -2,20 +2,26 @@ var value = require("./file"); it("should wait until promises returned by status handlers are fulfilled", (done) => { var handler = jest.fn(status => { - return Promise.resolve().then(() => { - expect(status).toBe(module.hot.status()); + var test = jest.fn(() => { + expect(module.hot.status()).toBe(status == "dispose" ? "apply" : status); }); + + var promise = Promise.resolve().then(test); + promise.test = test; + + return promise; }); module.hot.addStatusHandler(handler); module.hot.accept("./file", () => { value = require("./file"); - done(); }); NEXT(require("../../update")(done, undefined, () => { + expect(handler.mock.calls).toStrictEqual([['check'], ['prepare'], ['dispose'], ['apply'], ['idle']]); + for (let result of handler.mock.results) + expect(result.value.test).toHaveBeenCalledTimes(1); + expect(module.hot.status()).toBe("idle"); - - expect(handler.mock.calls).toBe([['check'], ['prepare'], ['dispose'], ['apply'], ['idle']]); + done(); -})); + })); }); -