Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix HMR when module evaluation throws #4151

Merged
merged 2 commits into from
Mar 10, 2023
Merged

fix HMR when module evaluation throws #4151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df18cf4
fix HMR when module evaluation throws
sokra Mar 10, 2023
fbc9031
review
sokra Mar 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 76 additions & 42 deletions crates/turbopack-ecmascript/js/src/runtime.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
/** @typedef {import('../types').GetFirstModuleChunk} GetFirstModuleChunk */

/** @typedef {import('../types').Module} Module */
/** @typedef {import('../types').SourceInfo} SourceInfo */
/** @typedef {import('../types').SourceType} SourceType */
/** @typedef {import('../types').SourceType.Runtime} SourceTypeRuntime */
/** @typedef {import('../types').SourceType.Parent} SourceTypeParent */
/** @typedef {import('../types').SourceType.Update} SourceTypeUpdate */
/** @typedef {import('../types').Exports} Exports */
/** @typedef {import('../types').EsmInteropNamespace} EsmInteropNamespace */
/** @typedef {import('../types').Runnable} Runnable */
Expand Down Expand Up @@ -286,48 +291,34 @@ function getOrCreateChunkLoader(chunkPath, from) {
return chunkLoader;
}

/**
* @enum {number}
*/
const SourceType = {
/**
* The module was instantiated because it was included in an evaluated chunk's
* runtime.
*/
Runtime: 0,
/**
* The module was instantiated because a parent module imported it.
*/
Parent: 1,
/**
* The module was instantiated because it was included in a chunk's hot module
* update.
*/
Update: 2,
};
/** @type {SourceTypeRuntime} */
const SourceTypeRuntime = 0;
/** @type {SourceTypeParent} */
const SourceTypeParent = 1;
/** @type {SourceTypeUpdate} */
const SourceTypeUpdate = 2;

/**
*
* @param {ModuleId} id
* @param {SourceType} sourceType
* @param {ModuleId} [sourceId]
* @param {SourceInfo} source
* @returns {Module}
*/
function instantiateModule(id, sourceType, sourceId) {
function instantiateModule(id, source) {
const moduleFactory = moduleFactories[id];
if (typeof moduleFactory !== "function") {
// This can happen if modules incorrectly handle HMR disposes/updates,
// e.g. when they keep a `setTimeout` around which still executes old code
// and contains e.g. a `require("something")` call.
let instantiationReason;
switch (sourceType) {
case SourceType.Runtime:
switch (source.type) {
case SourceTypeRuntime:
instantiationReason = "as a runtime entry";
break;
case SourceType.Parent:
instantiationReason = `because it was required from module ${sourceId}`;
case SourceTypeParent:
instantiationReason = `because it was required from module ${source.parentId}`;
break;
case SourceType.Update:
case SourceTypeUpdate:
instantiationReason = "because of an HMR update";
break;
}
Expand All @@ -344,21 +335,27 @@ function instantiateModule(id, sourceType, sourceId) {
exports: {},
loaded: false,
id,
parents: [],
parents: undefined,
children: [],
interopNamespace: undefined,
hot,
};
moduleCache[id] = module;
moduleHotState.set(module, hotState);

if (sourceType === SourceType.Runtime) {
runtimeModules.add(id);
} else if (sourceType === SourceType.Parent) {
module.parents.push(sourceId);

// No need to add this module as a child of the parent module here, this
// has already been taken care of in `getOrInstantiateModuleFromParent`.
switch (source.type) {
case SourceTypeRuntime:
runtimeModules.add(id);
module.parents = [];
break;
case SourceTypeParent:
// No need to add this module as a child of the parent module here, this
// has already been taken care of in `getOrInstantiateModuleFromParent`.
module.parents = [source.parentId];
break;
case SourceTypeUpdate:
module.parents = source.parents || [];
break;
}

runModuleExecutionHooks(module, () => {
Expand Down Expand Up @@ -445,7 +442,10 @@ function getOrInstantiateModuleFromParent(id, sourceModule) {
return module;
}

return instantiateModule(id, SourceType.Parent, sourceModule.id);
return instantiateModule(id, {
type: SourceTypeParent,
parentId: sourceModule.id,
});
}

/**
Expand Down Expand Up @@ -618,6 +618,7 @@ function updateChunksPhase(chunksAddedModules, chunksDeletedModules) {
/**
* @param {Iterable<ModuleId>} outdatedModules
* @param {Set<ModuleId>} disposedModules
* @return {{ outdatedModuleParents: Map<ModuleId, Array<ModuleId>> }}
*/
function disposePhase(outdatedModules, disposedModules) {
for (const moduleId of outdatedModules) {
Expand All @@ -628,8 +629,19 @@ function disposePhase(outdatedModules, disposedModules) {
disposeModule(moduleId, "clear");
}

// Removing modules from the module cache is a separate step.
// We also want to keep track of previous parents of the outdated modules.
const outdatedModuleParents = new Map();
for (const moduleId of outdatedModules) {
const oldModule = moduleCache[moduleId];
outdatedModuleParents.set(moduleId, oldModule?.parents);
delete moduleCache[moduleId];
sokra marked this conversation as resolved.
Show resolved Hide resolved
}

// TODO(alexkirsz) Dependencies: remove outdated dependency from module
// children.

return { outdatedModuleParents };
}

/**
Expand All @@ -638,6 +650,13 @@ function disposePhase(outdatedModules, disposedModules) {
* Returns the persistent hot data that should be kept for the next module
* instance.
*
* NOTE: mode = "replace" will not remove modules from the moduleCache.
* This must be done in a separate step afterwards.
* This is important because all modules need to be diposed to update the
* parent/child relationships before they are actually removed from the moduleCache.
* If this would be done in this method, following disposeModulecalls won't find
* the module from the module id in the cache.
*
* @param {ModuleId} moduleId
* @param {"clear" | "replace"} mode
*/
Expand All @@ -660,7 +679,6 @@ function disposeModule(moduleId, mode) {
// module is still importing other modules.
module.hot.active = false;

delete moduleCache[module.id];
moduleHotState.delete(module);

// TODO(alexkirsz) Dependencies: delete the module from outdated deps.
Expand All @@ -682,6 +700,7 @@ function disposeModule(moduleId, mode) {

switch (mode) {
case "clear":
delete moduleCache[module.id];
moduleHotData.delete(module.id);
break;
case "replace":
Expand All @@ -696,8 +715,13 @@ function disposeModule(moduleId, mode) {
*
* @param {{ moduleId: ModuleId, errorHandler: true | Function }[]} outdatedSelfAcceptedModules
* @param {Map<ModuleId, ModuleFactory>} newModuleFactories
* @param {Map<ModuleId, Array<ModuleId>>} outdatedModuleParents
*/
function applyPhase(outdatedSelfAcceptedModules, newModuleFactories) {
function applyPhase(
outdatedSelfAcceptedModules,
newModuleFactories,
outdatedModuleParents
) {
// Update module factories.
for (const [moduleId, factory] of newModuleFactories.entries()) {
moduleFactories[moduleId] = factory;
Expand All @@ -710,7 +734,10 @@ function applyPhase(outdatedSelfAcceptedModules, newModuleFactories) {
// Re-instantiate all outdated self-accepted modules.
for (const { moduleId, errorHandler } of outdatedSelfAcceptedModules) {
try {
instantiateModule(moduleId, SourceType.Update);
instantiateModule(moduleId, {
type: SourceTypeUpdate,
parents: outdatedModuleParents.get(moduleId),
});
} catch (err) {
if (typeof errorHandler === "function") {
try {
Expand Down Expand Up @@ -811,8 +838,15 @@ function applyEcmascriptMergedUpdate(chunkPath, update) {
const outdatedSelfAcceptedModules =
computeOutdatedSelfAcceptedModules(outdatedModules);
const { disposedModules } = updateChunksPhase(chunksAdded, chunksDeleted);
disposePhase(outdatedModules, disposedModules);
applyPhase(outdatedSelfAcceptedModules, newModuleFactories);
const { outdatedModuleParents } = disposePhase(
outdatedModules,
disposedModules
);
applyPhase(
outdatedSelfAcceptedModules,
newModuleFactories,
outdatedModuleParents
);
}

/**
Expand Down Expand Up @@ -1234,7 +1268,7 @@ function disposeChunk(chunkPath) {
* @returns {Module}
*/
function instantiateRuntimeModule(moduleId) {
return instantiateModule(moduleId, SourceType.Runtime);
return instantiateModule(moduleId, { type: SourceTypeRuntime });
}

/**
Expand Down
30 changes: 30 additions & 0 deletions crates/turbopack-ecmascript/js/types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,36 @@ interface Module {
interopNamespace?: EsmInteropNamespace;
}

enum SourceType {
/**
* The module was instantiated because it was included in an evaluated chunk's
* runtime.
*/
Runtime = 0,
/**
* The module was instantiated because a parent module imported it.
*/
Parent = 1,
/**
* The module was instantiated because it was included in a chunk's hot module
* update.
*/
Update = 2,
}

type SourceInfo =
| {
type: SourceType.Runtime;
}
| {
type: SourceType.Parent;
parentId: ModuleId;
}
| {
type: SourceType.Update;
parents?: ModuleId[];
};

type ModuleCache = Record<ModuleId, Module>;

type CommonJsRequire = (moduleId: ModuleId) => Exports;
Expand Down
Loading