Skip to content

Commit

Permalink
Dispose and apply must run atomically.
Browse files Browse the repository at this point in the history
  • Loading branch information
rockwalrus committed Jun 22, 2021
1 parent e852415 commit ef4aa56
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 44 deletions.
76 changes: 39 additions & 37 deletions lib/hmr/HotModuleReplacement.runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
});
}
Expand Down
20 changes: 13 additions & 7 deletions test/hotCases/status/accept/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}));
}));
});

2 comments on commit ef4aa56

@tiye
Copy link
Contributor

@tiye tiye commented on ef4aa56 Jul 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure but seeing from changes between 45.1 an 46, I think the bug #13877 is possibly related to this change. any ideas?

@alexander-akait
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug was fixed

Please sign in to comment.