module: ensure successful dynamic import returns the same result

benchmark/esm/esm-loader-import.js

@@ -0,0 +1,45 @@
+// Tests the impact on eager operations required for policies affecting
+// general startup, does not test lazy operations
+'use strict';
+const fs = require('node:fs');
+const path = require('node:path');
+const common = require('../common.js');
+
+const tmpdir = require('../../test/common/tmpdir.js');
+const { pathToFileURL } = require('node:url');
+
+const benchmarkDirectory = pathToFileURL(path.resolve(tmpdir.path, 'benchmark-import'));
+
+const configs = {
+ n: [1e3],
+ specifier: [
+ 'data:text/javascript,{i}',
+ './relative-existing.js',
+ './relative-nonexistent.js',
+ 'node:prefixed-nonexistent',
+ 'node:os',
+ ],
+};
+
+const options = {
+ flags: ['--expose-internals'],
+};
+
+const bench = common.createBenchmark(main, configs, options);
+
+async function main(conf) {
+ tmpdir.refresh();
+
+ fs.mkdirSync(benchmarkDirectory, { recursive: true });
+ fs.writeFileSync(new URL('./relative-existing.js', benchmarkDirectory), '\n');
+
+ bench.start();
+
+ for (let i = 0; i < conf.n; i++) {
+ try {
+ await import(new URL(conf.specifier.replace('{i}', i), benchmarkDirectory));
+ } catch { /* empty */ }
+ }
+
+ bench.end(conf.n);
+}

lib/internal/modules/esm/loader.js

@@ -20,9 +20,14 @@ const {
 } = require('internal/modules/esm/utils');
 let defaultResolve, defaultLoad, importMetaInitializer;
 
-function newModuleMap() {
- const ModuleMap = require('internal/modules/esm/module_map');
- return new ModuleMap();
+function newLoadCache() {
+ const { LoadCache } = require('internal/modules/esm/module_map');
+ return new LoadCache();
+}
+
+function newResolveCache() {
+ const { ResolveCache } = require('internal/modules/esm/module_map');
+ return new ResolveCache();
 }
 
 function getTranslators() {
@@ -74,7 +79,12 @@ class ModuleLoader {
  /**
  * Registry of loaded modules, akin to `require.cache`
  */
- moduleMap = newModuleMap();
+ loadCache = newLoadCache();
+
+ /**
+ * Registry of resolved URLs
+ */
+ #resolveCache = newResolveCache();
 
  /**
  * Methods which translate input code or other information into ES modules
@@ -183,7 +193,7 @@ class ModuleLoader {
  const ModuleJob = require('internal/modules/esm/module_job');
  const job = new ModuleJob(
  this, url, undefined, evalInstance, false, false);
- this.moduleMap.set(url, undefined, job);
+ this.loadCache.set(url, undefined, job);
  const { module } = await job.run();
 
  return {
@@ -213,11 +223,11 @@ class ModuleLoader {
  getJobFromResolveResult(resolveResult, parentURL, importAssertions) {
  const { url, format } = resolveResult;
  const resolvedImportAssertions = resolveResult.importAssertions ?? importAssertions;
- let job = this.moduleMap.get(url, resolvedImportAssertions.type);
+ let job = this.loadCache.get(url, resolvedImportAssertions.type);
 
  // CommonJS will set functions for lazy job evaluation.
  if (typeof job === 'function') {
- this.moduleMap.set(url, undefined, job = job());
+ this.loadCache.set(url, undefined, job = job());
  }
 
  if (job === undefined) {
@@ -277,7 +287,7 @@ class ModuleLoader {
  inspectBrk,
  );
 
- this.moduleMap.set(url, importAssertions.type, job);
+ this.loadCache.set(url, importAssertions.type, job);
 
  return job;
  }
@@ -315,13 +325,20 @@ class ModuleLoader {
  * @param {string} [parentURL] The URL path of the module's parent.
  * @param {ImportAssertions} importAssertions Assertions from the import
  * statement or expression.
- * @returns {Promise<{ format: string, url: URL['href'] }>}
+ * @returns {{ format: string, url: URL['href'] }}
  */
  resolve(originalSpecifier, parentURL, importAssertions) {
  if (this.#customizations) {
  return this.#customizations.resolve(originalSpecifier, parentURL, importAssertions);
  }
- return this.defaultResolve(originalSpecifier, parentURL, importAssertions);
+ const requestKey = this.#resolveCache.serializeKey(originalSpecifier, importAssertions);
+ const cachedResult = this.#resolveCache.get(requestKey, parentURL);
+ if (cachedResult != null) {
+ return cachedResult;
+ }
+ const result = this.defaultResolve(originalSpecifier, parentURL, importAssertions);
+ this.#resolveCache.set(requestKey, parentURL, result);
+ return result;
  }
 
  /**

lib/internal/modules/esm/module_job.js

@@ -21,7 +21,7 @@ const {
 
 const { ModuleWrap } = internalBinding('module_wrap');
 
-const { decorateErrorStack } = require('internal/util');
+const { decorateErrorStack, kEmptyObject } = require('internal/util');
 const {
  getSourceMapsEnabled,
 } = require('internal/source_map/source_map_cache');
@@ -140,7 +140,7 @@ class ModuleJob {
  /module '(.*)' does not provide an export named '(.+)'/,
  e.message);
  const { url: childFileURL } = await this.loader.resolve(
- childSpecifier, parentFileUrl,
+ childSpecifier, parentFileUrl, kEmptyObject,
  );
  let format;
  try {

lib/internal/modules/esm/module_map.js

@@ -1,17 +1,89 @@
 'use strict';
 
-const { kImplicitAssertType } = require('internal/modules/esm/assert');
 const {
+ ArrayPrototypeJoin,
+ ArrayPrototypeMap,
+ ArrayPrototypeSort,
+ JSONStringify,
+ ObjectKeys,
  SafeMap,
 } = primordials;
+const { kImplicitAssertType } = require('internal/modules/esm/assert');
 let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
  debug = fn;
 });
 const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
 const { validateString } = require('internal/validators');
 
-// Tracks the state of the loader-level module cache
-class ModuleMap extends SafeMap {
+/**
+ * Cache the results of the `resolve` step of the module resolution and loading process.
+ * Future resolutions of the same input (specifier, parent URL and import assertions)
+ * must return the same result if the first attempt was successful, per
+ * https://tc39.es/ecma262/#sec-HostLoadImportedModule.
+ */
+class ResolveCache extends SafeMap {
+ constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
+
+ /**
+ * Generates the internal serialized cache key and returns it along the actual cache object.
+ *
+ * It is exposed to allow more efficient read and overwrite a cache entry.
+ * @param {string} specifier
+ * @param {Record<string,string>} importAssertions
+ * @returns {string}
+ */
+ serializeKey(specifier, importAssertions) {
+ // To serialize the ModuleRequest (specifier + list of import attributes),
+ // we need to sort the attributes by key, then stringifying,
+ // so that different import statements with the same attributes are always treated
+ // as identical.
+ return specifier + '::' + ArrayPrototypeJoin(
+ ArrayPrototypeMap(
+ ArrayPrototypeSort(ObjectKeys(importAssertions)),
+ (key) => JSONStringify(key) + JSONStringify(importAssertions[key])),
+ ',');
+ }
+
+ /**
+ * @param {string} serializedKey
+ * @param {string} parentURL
+ * @returns {import('./loader').ModuleExports | Promise<import('./loader').ModuleExports>}
+ */
+ get(serializedKey, parentURL) {
+ let internalCache = super.get(parentURL);
+ if (internalCache == null) {
+ super.set(parentURL, internalCache = { __proto__: null });
+ }
+ return internalCache[serializedKey];
+ }
+
+ /**
+ * @param {string} serializedKey
+ * @param {string} parentURL
+ * @param {{ format: string, url: URL['href'] }} result
+ */
+ set(serializedKey, parentURL, result) {
+ let internalCache = super.get(parentURL);
+ if (internalCache == null) {
+ super.set(parentURL, internalCache = { __proto__: null });
+ }
+ internalCache[serializedKey] = result;
+ return this;
+ }
+
+ has(serializedKey, parentURL) {
+ let internalCache = super.get(parentURL);
+ if (internalCache == null) {
+ super.set(parentURL, internalCache = { __proto__: null });
+ }
+ return serializedKey in internalCache;
+ }
+}
+
+/**
+ * Cache the results of the `load` step of the module resolution and loading process.
+ */
+class LoadCache extends SafeMap {
  constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
  get(url, type = kImplicitAssertType) {
  validateString(url, 'url');
@@ -29,7 +101,7 @@ class ModuleMap extends SafeMap {
  }
  debug(`Storing ${url} (${
  type === kImplicitAssertType ? 'implicit type' : type
- }) in ModuleMap`);
+ }) in ModuleLoadMap`);
  const cachedJobsForUrl = super.get(url) ?? { __proto__: null };
  cachedJobsForUrl[type] = job;
  return super.set(url, cachedJobsForUrl);
@@ -40,4 +112,8 @@ class ModuleMap extends SafeMap {
  return super.get(url)?.[type] !== undefined;
  }
 }
-module.exports = ModuleMap;
+
+module.exports = {
+ LoadCache,
+ ResolveCache,
+};

test/es-module/test-esm-dynamic-import-mutating-fs.js

@@ -0,0 +1,25 @@
+'use strict';
+const common = require('../common');
+const tmpdir = require('../common/tmpdir');
+
+const assert = require('node:assert');
+const fs = require('node:fs/promises');
+const { pathToFileURL } = require('node:url');
+
+tmpdir.refresh();
+const tmpDir = pathToFileURL(tmpdir.path);
+
+const target = new URL(`./${Math.random()}.mjs`, tmpDir);
+
+(async () => {
+
+ await assert.rejects(import(target), { code: 'ERR_MODULE_NOT_FOUND' });
+
+ await fs.writeFile(target, 'export default "actual target"\n');
+
+ const moduleRecord = await import(target);
+
+ await fs.rm(target);
+
+ assert.strictEqual(await import(target), moduleRecord);
+})().then(common.mustCall());

test/es-module/test-esm-dynamic-import-mutating-fs.mjs

@@ -0,0 +1,42 @@
+import { spawnPromisified } from '../common/index.mjs';
+import tmpdir from '../common/tmpdir.js';
+
+import assert from 'node:assert';
+import fs from 'node:fs/promises';
+import { execPath } from 'node:process';
+import { pathToFileURL } from 'node:url';
+
+tmpdir.refresh();
+const tmpDir = pathToFileURL(tmpdir.path);
+
+const target = new URL(`./${Math.random()}.mjs`, tmpDir);
+
+await assert.rejects(import(target), { code: 'ERR_MODULE_NOT_FOUND' });
+
+await fs.writeFile(target, 'export default "actual target"\n');
+
+const moduleRecord = await import(target);
+
+await fs.rm(target);
+
+assert.strictEqual(await import(target), moduleRecord);
+
+// Add the file back, it should be deleted by the subprocess.
+await fs.writeFile(target, 'export default "actual target"\n');
+
+assert.deepStrictEqual(
+ await spawnPromisified(execPath, [
+ '--input-type=module',
+ '--eval',
+ [`import * as d from${JSON.stringify(target)};`,
+ 'import{rm}from"node:fs/promises";',
+ `await rm(new URL(${JSON.stringify(target)}));`,
+ 'import{strictEqual}from"node:assert";',
+ `strictEqual(JSON.stringify(await import(${JSON.stringify(target)})),JSON.stringify(d));`].join(''),
+ ]),
+ {
+ code: 0,
+ signal: null,
+ stderr: '',
+ stdout: '',
+ });