vm: expose import attributes on SourceTextModule.moduleRequests

PR-URL: #58829
Backport-PR-URL: #59962
Refs: #37648
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>

Author: legendecas

doc/api/vm.md

@@ -573,16 +573,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`
 
 * Type: {any}
@@ -887,6 +877,72 @@ 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.
+
+For example, given a source text:
+
+<!-- eslint-disable no-duplicate-imports -->
+
+```mjs
+import foo from 'foo';
+import fooAlias from 'foo';
+import bar from './bar.js';
+import withAttrs from '../with-attrs.ts' with { arbitraryAttr: 'attr-val' };
+```
+
+<!-- eslint-enable no-duplicate-imports -->
+
+The value of the `sourceTextModule.moduleRequests` will be:
+
+```js
+[
+  {
+    specifier: 'foo',
+    attributes: {},
+  },
+  {
+    specifier: 'foo',
+    attributes: {},
+  },
+  {
+    specifier: './bar.js',
+    attributes: {},
+  },
+  {
+    specifier: '../with-attrs.ts',
+    attributes: { arbitraryAttr: 'attr-val' },
+  },
+];
+```
+
 ## Class: `vm.SyntheticModule`
 
 <!-- YAML
@@ -983,6 +1039,20 @@ 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.
+
+A `ModuleRequest` represents the request to import a module with given import attributes and phase.
+
 ## `vm.compileFunction(code[, params[, options]])`
 
 <!-- YAML
@@ -1953,12 +2023,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://tc39.es/ecma262/#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://tc39.es/ecma262/#sec-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
@@ -1968,6 +2040,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

@@ -64,6 +64,7 @@ const {
 } = binding;
 
 const STATUS_MAP = {
+  __proto__: null,
   [kUninstantiated]: 'unlinked',
   [kInstantiating]: 'linking',
   [kInstantiated]: 'linked',
@@ -251,13 +252,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');
@@ -298,20 +301,25 @@ 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,
+      });
+    }));
   }
 
   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,
@@ -349,16 +357,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';
     }
@@ -369,7 +377,6 @@ class SourceTextModule extends Module {
   }
 
   get error() {
-    validateThisInternalField(this, kDependencySpecifiers, 'SourceTextModule');
     if (this.#error !== kNoError) {
       return this.#error;
     }
@@ -432,8 +439,12 @@ class SyntheticModule extends Module {
 }
 
 function importModuleDynamicallyWrap(importModuleDynamically) {
-  const importModuleDynamicallyWrapper = async (...args) => {
-    const m = await ReflectApply(importModuleDynamically, this, args);
+  const importModuleDynamicallyWrapper = async (specifier, referrer, attributes) => {
+    const m = await ReflectApply(
+      importModuleDynamically,
+      this,
+      [specifier, referrer, attributes],
+    );
     if (isModuleNamespaceObject(m)) {
       return m;
     }

src/module_wrap.cc

@@ -435,12 +435,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++) {
@@ -467,11 +472,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,87 @@
+'use strict';
+
+// Flags: --experimental-vm-modules
+
+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' };
+    export { foo, bar, quz, quz2 };
+  `);
+
+  const requests = m.moduleRequests;
+  assert.strictEqual(requests.length, 4);
+  assert.deepStrictEqual(requests[0], {
+    __proto__: null,
+    specifier: './foo.js',
+    attributes: {
+      __proto__: null,
+    },
+  });
+  assert.deepStrictEqual(requests[1], {
+    __proto__: null,
+    specifier: './bar.json',
+    attributes: {
+      __proto__: null,
+      type: 'json'
+    },
+  });
+  assert.deepStrictEqual(requests[2], {
+    __proto__: null,
+    specifier: './quz.js',
+    attributes: {
+      __proto__: null,
+      attr1: 'quz',
+    },
+  });
+  assert.deepStrictEqual(requests[3], {
+    __proto__: null,
+    specifier: './quz.js',
+    attributes: {
+      __proto__: null,
+      attr2: 'quark',
+      attr3: 'baz',
+    },
+  });
+
+  // Check the deprecated dependencySpecifiers property.
+  // The dependencySpecifiers items are not unique.
+  assert.deepStrictEqual(m.dependencySpecifiers, [
+    './foo.js',
+    './bar.json',
+    './quz.js',
+    './quz.js',
+  ]);
+});
+
+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'];
+  for (const propertyName of propertyNames) {
+    assert.throws(() => {
+      requests[0][propertyName] = 'bar.js';
+    }, {
+      name: 'TypeError',
+    });
+  }
+  assert.throws(() => {
+    requests[0].attributes.type = 'json';
+  }, {
+    name: 'TypeError',
+  });
+});