From 085b12ce8cefabaadb184da710de6e9eafd1497c Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
Date: Sun, 5 Sep 2021 12:08:37 +0200
Subject: [PATCH] vm: add support for import assertions in dynamic imports

---
 doc/api/vm.md                                 | 46 +++++++++++++++++++
 lib/internal/process/esm_loader.js            | 10 +++-
 lib/vm.js                                     |  2 +-
 src/module_wrap.cc                            | 11 +++++
 .../parallel/test-vm-module-dynamic-import.js | 16 ++++++-
 5 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/doc/api/vm.md b/doc/api/vm.md
index b7cde0e3b69381..7a8dc946b1c824 100644
--- a/doc/api/vm.md
+++ b/doc/api/vm.md
@@ -54,6 +54,10 @@ executed in specific contexts.
 <!-- YAML
 added: v0.3.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/40249
+    description: Added support for import assertions to the
+                 `importModuleDynamically` parameter.
   - version: v10.6.0
     pr-url: https://github.com/nodejs/node/pull/20300
     description: The `produceCachedData` is deprecated in favour of
@@ -91,6 +95,9 @@ changes:
     using it in a production environment.
     * `specifier` {string} specifier passed to `import()`
     * `script` {vm.Script}
+    * `importAssertions` {Object} The `"assert"` value passed to the
+      [`optionsExpression`][] optional parameter, or an empty object if no value
+      was provided.
     * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
       recommended in order to take advantage of error tracking, and to avoid
       issues with namespaces that contain `then` function exports.
@@ -642,6 +649,13 @@ The `vm.SourceTextModule` class provides the [Source Text Module Record][] as
 defined in the ECMAScript specification.
 
 ### `new vm.SourceTextModule(code[, options])`
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/40249
+    description: Added support for import assertions to the
+                 `importModuleDynamically` parameter.
+-->
 
 * `code` {string} JavaScript Module code to parse
 * `options`
@@ -667,6 +681,9 @@ defined in the ECMAScript specification.
     `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
     * `specifier` {string} specifier passed to `import()`
     * `module` {vm.Module}
+    * `importAssertions` {Object} The `"assert"` value passed to the
+      [`optionsExpression`][] optional parameter, or an empty object if no value
+      was provided.
     * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
       recommended in order to take advantage of error tracking, and to avoid
       issues with namespaces that contain `then` function exports.
@@ -852,6 +869,10 @@ const vm = require('vm');
 <!-- YAML
 added: v10.10.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/40249
+    description: Added support for import assertions to the
+                 `importModuleDynamically` parameter.
   - version: v15.9.0
     pr-url: https://github.com/nodejs/node/pull/35431
     description: Added `importModuleDynamically` option again.
@@ -893,6 +914,9 @@ changes:
     considered stable.
     * `specifier` {string} specifier passed to `import()`
     * `function` {Function}
+    * `importAssertions` {Object} The `"assert"` value passed to the
+      [`optionsExpression`][] optional parameter, or an empty object if no value
+      was provided.
     * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
       recommended in order to take advantage of error tracking, and to avoid
       issues with namespaces that contain `then` function exports.
@@ -1068,6 +1092,10 @@ vm.measureMemory({ mode: 'detailed', execution: 'eager' })
 <!-- YAML
 added: v0.3.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/40249
+    description: Added support for import assertions to the
+                 `importModuleDynamically` parameter.
   - version: v6.3.0
     pr-url: https://github.com/nodejs/node/pull/6635
     description: The `breakOnSigint` option is supported now.
@@ -1113,6 +1141,9 @@ changes:
     using it in a production environment.
     * `specifier` {string} specifier passed to `import()`
     * `script` {vm.Script}
+    * `importAssertions` {Object} The `"assert"` value passed to the
+      [`optionsExpression`][] optional parameter, or an empty object if no value
+      was provided.
     * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
       recommended in order to take advantage of error tracking, and to avoid
       issues with namespaces that contain `then` function exports.
@@ -1145,6 +1176,10 @@ console.log(contextObject);
 <!-- YAML
 added: v0.3.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/40249
+    description: Added support for import assertions to the
+                 `importModuleDynamically` parameter.
   - version: v14.6.0
     pr-url: https://github.com/nodejs/node/pull/34023
     description: The `microtaskMode` option is supported now.
@@ -1211,6 +1246,9 @@ changes:
     using it in a production environment.
     * `specifier` {string} specifier passed to `import()`
     * `script` {vm.Script}
+    * `importAssertions` {Object} The `"assert"` value passed to the
+      [`optionsExpression`][] optional parameter, or an empty object if no value
+      was provided.
     * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
       recommended in order to take advantage of error tracking, and to avoid
       issues with namespaces that contain `then` function exports.
@@ -1247,6 +1285,10 @@ console.log(contextObject);
 <!-- YAML
 added: v0.3.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/40249
+    description: Added support for import assertions to the
+                 `importModuleDynamically` parameter.
   - version: v6.3.0
     pr-url: https://github.com/nodejs/node/pull/6635
     description: The `breakOnSigint` option is supported now.
@@ -1290,6 +1332,9 @@ changes:
     using it in a production environment.
     * `specifier` {string} specifier passed to `import()`
     * `script` {vm.Script}
+    * `importAssertions` {Object} The `"assert"` value passed to the
+      [`optionsExpression`][] optional parameter, or an empty object if no value
+      was provided.
     * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
       recommended in order to take advantage of error tracking, and to avoid
       issues with namespaces that contain `then` function exports.
@@ -1452,6 +1497,7 @@ are not controllable through the timeout either.
 [`Error`]: errors.md#class-error
 [`URL`]: url.md#class-url
 [`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval
+[`optionsExpression`]: https://tc39.es/proposal-import-assertions/#sec-evaluate-import-call
 [`script.runInContext()`]: #scriptrunincontextcontextifiedobject-options
 [`script.runInThisContext()`]: #scriptruninthiscontextoptions
 [`url.origin`]: url.md#urlorigin
diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js
index 0f04ca97751ef6..73385a85b4e106 100644
--- a/lib/internal/process/esm_loader.js
+++ b/lib/internal/process/esm_loader.js
@@ -1,5 +1,9 @@
 'use strict';
 
+const {
+  ObjectCreate,
+} = primordials;
+
 const {
   ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING,
 } = require('internal/errors').codes;
@@ -22,13 +26,14 @@ exports.initializeImportMetaObject = function(wrap, meta) {
   }
 };
 
-exports.importModuleDynamicallyCallback = async function(wrap, specifier) {
+exports.importModuleDynamicallyCallback =
+async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
   const { callbackMap } = internalBinding('module_wrap');
   if (callbackMap.has(wrap)) {
     const { importModuleDynamically } = callbackMap.get(wrap);
     if (importModuleDynamically !== undefined) {
       return importModuleDynamically(
-        specifier, getModuleFromWrap(wrap) || wrap);
+        specifier, getModuleFromWrap(wrap) || wrap, assertions);
     }
   }
   throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
@@ -69,6 +74,7 @@ async function initializeLoader() {
   const exports = await internalEsmLoader.import(
     customLoaders,
     pathToFileURL(cwd).href,
+    ObjectCreate(null),
   );
 
   // Hooks must then be added to external/public loader
diff --git a/lib/vm.js b/lib/vm.js
index d8e3d6586036f3..92afa9f6964acf 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -378,7 +378,7 @@ function compileFunction(code, params, options = {}) {
     const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
     const func = result.function;
     callbackMap.set(result.cacheKey, {
-      importModuleDynamically: (s, _k) => wrapped(s, func),
+      importModuleDynamically: (s, _k, i) => wrapped(s, func, i),
     });
   }
 
diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index f3ea419df1ebc0..1e7fb959a6ea8e 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -602,9 +602,20 @@ static MaybeLocal<Promise> ImportModuleDynamically(
     UNREACHABLE();
   }
 
+  Local<Object> assertions =
+    Object::New(isolate, v8::Null(env->isolate()), nullptr, nullptr, 0);
+  for (int i = 0; i < import_assertions->Length(); i += 2) {
+    assertions
+        ->Set(env->context(),
+              Local<String>::Cast(import_assertions->Get(env->context(), i)),
+              Local<Value>::Cast(import_assertions->Get(env->context(), i + 1)))
+        .ToChecked();
+  }
+
   Local<Value> import_args[] = {
     object,
     Local<Value>(specifier),
+    assertions,
   };
 
   Local<Value> result;
diff --git a/test/parallel/test-vm-module-dynamic-import.js b/test/parallel/test-vm-module-dynamic-import.js
index 9d04cf96e6f979..2273497d27677c 100644
--- a/test/parallel/test-vm-module-dynamic-import.js
+++ b/test/parallel/test-vm-module-dynamic-import.js
@@ -1,6 +1,6 @@
 'use strict';
 
-// Flags: --experimental-vm-modules
+// Flags: --experimental-vm-modules --harmony-import-assertions
 
 const common = require('../common');
 
@@ -56,6 +56,20 @@ async function test() {
     assert.strictEqual(foo.namespace, await globalThis.fooResult);
     delete globalThis.fooResult;
   }
+
+  {
+    const s = new Script('import("foo", { assert: { key: "value" } })', {
+      importModuleDynamically: common.mustCall((specifier, wrap, assertion) => {
+        assert.strictEqual(specifier, 'foo');
+        assert.strictEqual(wrap, s);
+        assert.deepStrictEqual(assertion, { __proto__: null, key: 'value' });
+        return foo;
+      }),
+    });
+
+    const result = s.runInThisContext();
+    assert.strictEqual(foo.namespace, await result);
+  }
 }
 
 async function testInvalid() {