Skip to content

Commit 7535aa1

Browse files
joyeecheungnodejs-github-bot
authored andcommitted
esm: link modules synchronously when no async loader hooks are used
When no async loader hooks are registered, perform the linking as synchronously as possible to reduce the chance of races from the the shared module loading cache. PR-URL: #59519 Fixes: #59366 Refs: https://github.com/abejfehr/node-22.18-issue-repro Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent db70ceb commit 7535aa1

File tree

5 files changed

+25
-15
lines changed

5 files changed

+25
-15
lines changed

lib/internal/modules/esm/loader.js

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
6565
debug = fn;
6666
});
6767

68+
const { isPromise } = require('internal/util/types');
69+
6870
/**
6971
* @typedef {import('./hooks.js').HooksProxy} HooksProxy
7072
* @typedef {import('./module_job.js').ModuleJobBase} ModuleJobBase
@@ -592,15 +594,21 @@ class ModuleLoader {
592594

593595
/**
594596
* Load a module and translate it into a ModuleWrap for ordinary imported ESM.
595-
* This is run asynchronously.
597+
* This may be run asynchronously if there are asynchronous module loader hooks registered.
596598
* @param {string} url URL of the module to be translated.
597599
* @param {object} loadContext See {@link load}
598600
* @param {boolean} isMain Whether the module to be translated is the entry point.
599-
* @returns {Promise<ModuleWrap>}
601+
* @returns {Promise<ModuleWrap>|ModuleWrap}
600602
*/
601-
async loadAndTranslate(url, loadContext, isMain) {
602-
const { format, source } = await this.load(url, loadContext);
603-
return this.#translate(url, format, source, isMain);
603+
loadAndTranslate(url, loadContext, isMain) {
604+
const maybePromise = this.load(url, loadContext);
605+
const afterLoad = ({ format, source }) => {
606+
return this.#translate(url, format, source, isMain);
607+
};
608+
if (isPromise(maybePromise)) {
609+
return maybePromise.then(afterLoad);
610+
}
611+
return afterLoad(maybePromise);
604612
}
605613

606614
/**

lib/internal/modules/esm/module_job.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const {
3737
},
3838
} = internalBinding('util');
3939
const { decorateErrorStack, kEmptyObject } = require('internal/util');
40+
const { isPromise } = require('internal/util/types');
4041
const {
4142
getSourceMapsSupport,
4243
} = require('internal/source_map/source_map_cache');
@@ -138,12 +139,11 @@ class ModuleJob extends ModuleJobBase {
138139
this.#loader = loader;
139140

140141
// Expose the promise to the ModuleWrap directly for linking below.
141-
if (isForRequireInImportedCJS) {
142-
this.module = moduleOrModulePromise;
143-
assert(this.module instanceof ModuleWrap);
144-
this.modulePromise = PromiseResolve(this.module);
145-
} else {
142+
if (isPromise(moduleOrModulePromise)) {
146143
this.modulePromise = moduleOrModulePromise;
144+
} else {
145+
this.module = moduleOrModulePromise;
146+
this.modulePromise = PromiseResolve(moduleOrModulePromise);
147147
}
148148

149149
if (this.phase === kEvaluationPhase) {

test/es-module/test-esm-error-cache.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ let error;
1919
await assert.rejects(
2020
() => import(file),
2121
(e) => {
22-
assert.strictEqual(error, e);
22+
// The module may be compiled again and a new SyntaxError would be thrown but
23+
// with the same content.
24+
assert.deepStrictEqual(error, e);
2325
return true;
2426
}
2527
);

test/es-module/test-esm-import-attributes-errors.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ async function test() {
2828

2929
await rejects(
3030
import(jsModuleDataUrl, { with: { type: 'json', other: 'unsupported' } }),
31-
{ code: 'ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE' }
31+
{ code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' }
3232
);
3333

3434
await rejects(
@@ -48,7 +48,7 @@ async function test() {
4848

4949
await rejects(
5050
import(jsonModuleDataUrl, { with: { foo: 'bar' } }),
51-
{ code: 'ERR_IMPORT_ATTRIBUTE_MISSING' }
51+
{ code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' }
5252
);
5353

5454
await rejects(

test/es-module/test-esm-import-attributes-errors.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ await rejects(
2323

2424
await rejects(
2525
import(jsModuleDataUrl, { with: { type: 'json', other: 'unsupported' } }),
26-
{ code: 'ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE' }
26+
{ code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' }
2727
);
2828

2929
await rejects(
@@ -43,7 +43,7 @@ await rejects(
4343

4444
await rejects(
4545
import(jsonModuleDataUrl, { with: { foo: 'bar' } }),
46-
{ code: 'ERR_IMPORT_ATTRIBUTE_MISSING' }
46+
{ code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' }
4747
);
4848

4949
await rejects(

0 commit comments

Comments
 (0)