vm: expose import phase on SourceTextModule.moduleRequests

Author: legendecas

doc/api/vm.md

@@ -575,16 +575,6 @@ const contextifiedObject = vm.createContext({
 })();
 ```
 
-### `module.dependencySpecifiers`
-
-* {string\[]}
-
-The specifiers of all dependencies of this module. The returned array is frozen
-to disallow any changes to it.
-
-Corresponds to the `[[RequestedModules]]` field of [Cyclic Module Record][]s in
-the ECMAScript specification.
-
 ### `module.error`
 
 * {any}
@@ -889,6 +879,36 @@ const cachedData = module.createCachedData();
 const module2 = new vm.SourceTextModule('const a = 1;', { cachedData });
 ```
 
+### `sourceTextModule.dependencySpecifiers`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/20300
+    description: This is deprecated in favour of `sourceTextModule.moduleRequests`.
+-->
+
+> Stability: 0 - Deprecated: Use [`sourceTextModule.moduleRequests`][] instead.
+
+* {string\[]}
+
+The specifiers of all dependencies of this module. The returned array is frozen
+to disallow any changes to it.
+
+Corresponds to the `[[RequestedModules]]` field of [Cyclic Module Record][]s in
+the ECMAScript specification.
+
+### `sourceTextModule.moduleRequests`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* {ModuleRequest\[]} Dependencies of this module.
+
+The requested import dependencies of this module. The returned array is frozen
+to disallow any changes to it.
+
 ## Class: `vm.SyntheticModule`
 
 <!-- YAML
@@ -985,6 +1005,21 @@ const vm = require('node:vm');
 })();
 ```
 
+## Type: `ModuleRequest`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* {Object}
+  * `specifier` {string} The specifier of the requested module.
+  * `attributes` {Object} The `"with"` value passed to the
+    [WithClause][] in a [ImportDeclaration][], or an empty object if no value was
+    provided.
+  * `phase` {string} The phase of the requested module (`"source"` or `"evaluation"`).
+
+A `ModuleRequest` represents the request to import a module with given import attributes and phase.
+
 ## `vm.compileFunction(code[, params[, options]])`
 
 <!-- YAML
@@ -1958,12 +1993,14 @@ const { Script, SyntheticModule } = require('node:vm');
 [Evaluate() concrete method]: https://tc39.es/ecma262/#sec-moduleevaluation
 [GetModuleNamespace]: https://tc39.es/ecma262/#sec-getmodulenamespace
 [HostResolveImportedModule]: https://tc39.es/ecma262/#sec-hostresolveimportedmodule
+[ImportDeclaration]: https://tc39.es/ecma262/#prod-ImportDeclaration
 [Link() concrete method]: https://tc39.es/ecma262/#sec-moduledeclarationlinking
 [Module Record]: https://262.ecma-international.org/14.0/#sec-abstract-module-records
 [Source Text Module Record]: https://tc39.es/ecma262/#sec-source-text-module-records
 [Support of dynamic `import()` in compilation APIs]: #support-of-dynamic-import-in-compilation-apis
 [Synthetic Module Record]: https://heycam.github.io/webidl/#synthetic-module-records
 [V8 Embedder's Guide]: https://v8.dev/docs/embed#contexts
+[WithClause]: https://tc39.es/ecma262/#prod-WithClause
 [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`]: errors.md#err_vm_dynamic_import_callback_missing_flag
 [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.md#err_vm_dynamic_import_callback_missing
 [`ERR_VM_MODULE_STATUS`]: errors.md#err_vm_module_status
@@ -1973,6 +2010,7 @@ const { Script, SyntheticModule } = require('node:vm');
 [`optionsExpression`]: https://tc39.es/proposal-import-attributes/#sec-evaluate-import-call
 [`script.runInContext()`]: #scriptrunincontextcontextifiedobject-options
 [`script.runInThisContext()`]: #scriptruninthiscontextoptions
+[`sourceTextModule.moduleRequests`]: #sourcetextmodulemodulerequests
 [`url.origin`]: url.md#urlorigin
 [`vm.compileFunction()`]: #vmcompilefunctioncode-params-options
 [`vm.constants.DONT_CONTEXTIFY`]: #vmconstantsdont_contextify

lib/internal/vm/module.js

@@ -62,6 +62,7 @@ const {
   kEvaluated,
   kErrored,
   kSourcePhase,
+  kEvaluationPhase,
 } = binding;
 
 const STATUS_MAP = {
@@ -90,6 +91,15 @@ function isModule(object) {
   return true;
 }
 
+function phaseEnumToPhaseName(phase) {
+  if (phase === kSourcePhase) {
+    return 'source';
+  } else if (phase === kEvaluationPhase) {
+    return 'evaluation';
+  }
+  assert.fail(`Invalid phase: ${phase}`);
+}
+
 class Module {
   constructor(options) {
     emitExperimentalWarning('VM Modules');
@@ -252,13 +262,15 @@ class Module {
   }
 }
 
-const kDependencySpecifiers = Symbol('kDependencySpecifiers');
 const kNoError = Symbol('kNoError');
 
 class SourceTextModule extends Module {
   #error = kNoError;
   #statusOverride;
 
+  #moduleRequests;
+  #dependencySpecifiers;
+
   constructor(sourceText, options = kEmptyObject) {
     validateString(sourceText, 'sourceText');
     validateObject(options, 'options');
@@ -299,20 +311,26 @@ class SourceTextModule extends Module {
       importModuleDynamically,
     });
 
-    this[kDependencySpecifiers] = undefined;
+    this.#moduleRequests = ObjectFreeze(ArrayPrototypeMap(this[kWrap].getModuleRequests(), (request) => {
+      return ObjectFreeze({
+        __proto__: null,
+        specifier: request.specifier,
+        attributes: request.attributes,
+        phase: phaseEnumToPhaseName(request.phase),
+      });
+    }));
   }
 
   async [kLink](linker) {
     this.#statusOverride = 'linking';
 
-    const moduleRequests = this[kWrap].getModuleRequests();
     // Iterates the module requests and links with the linker.
     // Specifiers should be aligned with the moduleRequests array in order.
-    const specifiers = Array(moduleRequests.length);
-    const modulePromises = Array(moduleRequests.length);
+    const specifiers = Array(this.#moduleRequests.length);
+    const modulePromises = Array(this.#moduleRequests.length);
     // Iterates with index to avoid calling into userspace with `Symbol.iterator`.
-    for (let idx = 0; idx < moduleRequests.length; idx++) {
-      const { specifier, attributes } = moduleRequests[idx];
+    for (let idx = 0; idx < this.#moduleRequests.length; idx++) {
+      const { specifier, attributes } = this.#moduleRequests[idx];
 
       const linkerResult = linker(specifier, this, {
         attributes,
@@ -350,16 +368,16 @@ class SourceTextModule extends Module {
   }
 
   get dependencySpecifiers() {
-    validateThisInternalField(this, kDependencySpecifiers, 'SourceTextModule');
-    // TODO(legendecas): add a new getter to expose the import attributes as the value type
-    // of [[RequestedModules]] is changed in https://tc39.es/proposal-import-attributes/#table-cyclic-module-fields.
-    this[kDependencySpecifiers] ??= ObjectFreeze(
-      ArrayPrototypeMap(this[kWrap].getModuleRequests(), (request) => request.specifier));
-    return this[kDependencySpecifiers];
+    this.#dependencySpecifiers ??= ObjectFreeze(
+      ArrayPrototypeMap(this.#moduleRequests, (request) => request.specifier));
+    return this.#dependencySpecifiers;
+  }
+
+  get moduleRequests() {
+    return this.#moduleRequests;
   }
 
   get status() {
-    validateThisInternalField(this, kDependencySpecifiers, 'SourceTextModule');
     if (this.#error !== kNoError) {
       return 'errored';
     }
@@ -370,7 +388,6 @@ class SourceTextModule extends Module {
   }
 
   get error() {
-    validateThisInternalField(this, kDependencySpecifiers, 'SourceTextModule');
     if (this.#error !== kNoError) {
       return this.#error;
     }
@@ -447,9 +464,12 @@ class SyntheticModule extends Module {
  */
 function importModuleDynamicallyWrap(importModuleDynamically) {
   const importModuleDynamicallyWrapper = async (specifier, referrer, attributes, phase) => {
-    const phaseString = phase === kSourcePhase ? 'source' : 'evaluation';
-    const m = await ReflectApply(importModuleDynamically, this, [specifier, referrer, attributes,
-                                                                 phaseString]);
+    const phaseName = phaseEnumToPhaseName(phase);
+    const m = await ReflectApply(
+      importModuleDynamically,
+      this,
+      [specifier, referrer, attributes, phaseName],
+    );
     if (isModuleNamespaceObject(m)) {
       if (phase === kSourcePhase) throw new ERR_VM_MODULE_NOT_MODULE();
       return m;

src/module_wrap.cc

@@ -448,12 +448,17 @@ static Local<Object> createImportAttributesContainer(
     values[idx] = raw_attributes->Get(realm->context(), i + 1).As<Value>();
   }
 
-  return Object::New(
+  Local<Object> attributes = Object::New(
       isolate, Null(isolate), names.data(), values.data(), num_attributes);
+  attributes->SetIntegrityLevel(realm->context(), v8::IntegrityLevel::kFrozen)
+      .Check();
+  return attributes;
 }
 
 static Local<Array> createModuleRequestsContainer(
     Realm* realm, Isolate* isolate, Local<FixedArray> raw_requests) {
+  EscapableHandleScope scope(isolate);
+  Local<Context> context = realm->context();
   LocalVector<Value> requests(isolate, raw_requests->Length());
 
   for (int i = 0; i < raw_requests->Length(); i++) {
@@ -483,11 +488,12 @@ static Local<Array> createModuleRequestsContainer(
 
     Local<Object> request =
         Object::New(isolate, Null(isolate), names, values, arraysize(names));
+    request->SetIntegrityLevel(context, v8::IntegrityLevel::kFrozen).Check();
 
     requests[i] = request;
   }
 
-  return Array::New(isolate, requests.data(), requests.size());
+  return scope.Escape(Array::New(isolate, requests.data(), requests.size()));
 }
 
 void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {

test/parallel/test-vm-module-errors.js

@@ -237,23 +237,29 @@ function checkInvalidCachedData() {
 }
 
 function checkGettersErrors() {
-  const expectedError = { code: 'ERR_INVALID_THIS' };
+  const expectedError = { name: 'TypeError' };
   const getters = ['identifier', 'context', 'namespace', 'status', 'error'];
   getters.forEach((getter) => {
     assert.throws(() => {
       // eslint-disable-next-line no-unused-expressions
       Module.prototype[getter];
-    }, expectedError);
+    }, expectedError, `Module.prototype.${getter} should throw`);
     assert.throws(() => {
       // eslint-disable-next-line no-unused-expressions
       SourceTextModule.prototype[getter];
-    }, expectedError);
+    }, expectedError, `SourceTextModule.prototype.${getter} should throw`);
+  });
+
+  const sourceTextModuleGetters = [
+    'moduleRequests',
+    'dependencySpecifiers',
+  ];
+  sourceTextModuleGetters.forEach((getter) => {
+    assert.throws(() => {
+      // eslint-disable-next-line no-unused-expressions
+      SourceTextModule.prototype[getter];
+    }, expectedError, `SourceTextModule.prototype.${getter} should throw`);
   });
-  // `dependencySpecifiers` getter is just part of SourceTextModule
-  assert.throws(() => {
-    // eslint-disable-next-line no-unused-expressions
-    SourceTextModule.prototype.dependencySpecifiers;
-  }, expectedError);
 }
 
 const finished = common.mustCall();

test/parallel/test-vm-module-modulerequests.js

@@ -0,0 +1,101 @@
+'use strict';
+
+// Flags: --experimental-vm-modules --js-source-phase-imports
+
+require('../common');
+const assert = require('node:assert');
+const {
+  SourceTextModule,
+} = require('node:vm');
+const test = require('node:test');
+
+test('SourceTextModule.moduleRequests should return module requests', (t) => {
+  const m = new SourceTextModule(`
+    import { foo } from './foo.js';
+    import { bar } from './bar.json' with { type: 'json' };
+    import { quz } from './quz.js' with { attr1: 'quz' };
+    import { quz as quz2 } from './quz.js' with { attr2: 'quark', attr3: 'baz' };
+    import source Module from './source-module';
+    export { foo, bar, quz, quz2 };
+  `);
+
+  const requests = m.moduleRequests;
+  assert.strictEqual(requests.length, 5);
+  assert.deepStrictEqual(requests[0], {
+    __proto__: null,
+    specifier: './foo.js',
+    attributes: {
+      __proto__: null,
+    },
+    phase: 'evaluation',
+  });
+  assert.deepStrictEqual(requests[1], {
+    __proto__: null,
+    specifier: './bar.json',
+    attributes: {
+      __proto__: null,
+      type: 'json'
+    },
+    phase: 'evaluation',
+  });
+  assert.deepStrictEqual(requests[2], {
+    __proto__: null,
+    specifier: './quz.js',
+    attributes: {
+      __proto__: null,
+      attr1: 'quz',
+    },
+    phase: 'evaluation',
+  });
+  assert.deepStrictEqual(requests[3], {
+    __proto__: null,
+    specifier: './quz.js',
+    attributes: {
+      __proto__: null,
+      attr2: 'quark',
+      attr3: 'baz',
+    },
+    phase: 'evaluation',
+  });
+  assert.deepStrictEqual(requests[4], {
+    __proto__: null,
+    specifier: './source-module',
+    attributes: {
+      __proto__: null,
+    },
+    phase: 'source',
+  });
+
+  // Check the deprecated dependencySpecifiers property.
+  // The dependencySpecifiers items are not unique.
+  assert.deepStrictEqual(m.dependencySpecifiers, [
+    './foo.js',
+    './bar.json',
+    './quz.js',
+    './quz.js',
+    './source-module',
+  ]);
+});
+
+test('SourceTextModule.moduleRequests items are frozen', (t) => {
+  const m = new SourceTextModule(`
+    import { foo } from './foo.js';
+  `);
+
+  const requests = m.moduleRequests;
+  assert.strictEqual(requests.length, 1);
+
+  const propertyNames = ['specifier', 'attributes', 'phase'];
+  for (const propertyName of propertyNames) {
+    assert.throws(() => {
+      requests[0][propertyName] = 'bar.js';
+    }, {
+      name: 'TypeError',
+    });
+  }
+  assert.throws(() => {
+    requests[0].attributes.type = 'json';
+  }, {
+    name: 'TypeError',
+  });
+});