Repository: nodejs/node
Pull request: vm: import call should return a promise in the current context

A `import` call should returns a promise created in the context where
the `import` was called, not the context of `importModuleDynamically`
callback.

This is also a requirement in ECMA262 https://tc39.es/ecma262/#sec-evaluate-import-call
step 7 and 14:

```
13.3.10.2 EvaluateImportCall ( specifierExpression [ , optionsExpression ] )
  ...
  7. Let promiseCapability be ! NewPromiseCapability(%Promise%).
  ...
  14. Return promiseCapability.[[Promise]].
```

Fixes: https://github.com/nodejs/node/issues/53575

Changed files:
filename: src/module_wrap.cc
patch: @@ -1020,16 +1020,23 @@ static MaybeLocal<Promise> ImportModuleDynamicallyWithPhase(
   };
 
   Local<Value> result;
-  if (import_callback->Call(
-        context,
-        Undefined(isolate),
-        arraysize(import_args),
-        import_args).ToLocal(&result)) {
-    CHECK(result->IsPromise());
-    return handle_scope.Escape(result.As<Promise>());
+  if (!import_callback
+           ->Call(
+               context, Undefined(isolate), arraysize(import_args), import_args)
+           .ToLocal(&result)) {
+    return {};
   }
 
-  return MaybeLocal<Promise>();
+  // Wrap the returned value in a promise created in the referrer context to
+  // avoid dynamic scopes.
+  Local<Promise::Resolver> resolver;
+  if (!Promise::Resolver::New(context).ToLocal(&resolver)) {
+    return {};
+  }
+  if (resolver->Resolve(context, result).IsNothing()) {
+    return {};
+  }
+  return handle_scope.Escape(resolver->GetPromise());
 }
 
 static MaybeLocal<Promise> ImportModuleDynamically(

filename: test/parallel/test-vm-module-dynamic-import-promise.js
patch: @@ -0,0 +1,135 @@
+// Flags: --experimental-vm-modules
+'use strict';
+
+const common = require('../common');
+
+const assert = require('assert');
+const { createContext, Script, SourceTextModule } = require('vm');
+
+// Verifies that a `import` call returns a promise created in the context
+// where the `import` was called, not the context of `importModuleDynamically`
+// callback.
+
+async function testScript() {
+  const ctx = createContext();
+
+  const mod1 = new SourceTextModule('export const a = 1;', {
+    context: ctx,
+  });
+  // No import statements, so must not link statically.
+  await mod1.link(common.mustNotCall());
+
+  const script2 = new Script(`
+    const promise = import("mod1");
+    if (Object.getPrototypeOf(promise) !== Promise.prototype) {
+      throw new Error('Expected promise to be created in the current context');
+    }
+    globalThis.__result = promise;
+  `, {
+    importModuleDynamically: common.mustCall((specifier, referrer) => {
+      assert.strictEqual(specifier, 'mod1');
+      assert.strictEqual(referrer, script2);
+      return mod1;
+    }),
+  });
+  script2.runInContext(ctx);
+
+  // Wait for the promise to resolve.
+  await ctx.__result;
+}
+
+async function testScriptImportFailed() {
+  const ctx = createContext();
+
+  const mod1 = new SourceTextModule('export const a = 1;', {
+    context: ctx,
+  });
+  // No import statements, so must not link statically.
+  await mod1.link(common.mustNotCall());
+
+  const err = new Error('import failed');
+  const script2 = new Script(`
+    const promise = import("mod1");
+    if (Object.getPrototypeOf(promise) !== Promise.prototype) {
+      throw new Error('Expected promise to be created in the current context');
+    }
+    globalThis.__result = promise;
+  `, {
+    importModuleDynamically: common.mustCall((specifier, referrer) => {
+      throw err;
+    }),
+  });
+  script2.runInContext(ctx);
+
+  // Wait for the promise to reject.
+  await assert.rejects(ctx.__result, err);
+}
+
+async function testModule() {
+  const ctx = createContext();
+
+  const mod1 = new SourceTextModule('export const a = 1;', {
+    context: ctx,
+  });
+  // No import statements, so must not link statically.
+  await mod1.link(common.mustNotCall());
+
+  const mod2 = new SourceTextModule(`
+    const promise = import("mod1");
+    if (Object.getPrototypeOf(promise) !== Promise.prototype) {
+      throw new Error('Expected promise to be created in the current context');
+    }
+    await promise;
+  `, {
+    context: ctx,
+    importModuleDynamically: common.mustCall((specifier, referrer) => {
+      assert.strictEqual(specifier, 'mod1');
+      assert.strictEqual(referrer, mod2);
+      return mod1;
+    }),
+  });
+  // No import statements, so must not link statically.
+  await mod2.link(common.mustNotCall());
+  await mod2.evaluate();
+}
+
+async function testModuleImportFailed() {
+  const ctx = createContext();
+
+  const mod1 = new SourceTextModule('export const a = 1;', {
+    context: ctx,
+  });
+  // No import statements, so must not link statically.
+  await mod1.link(common.mustNotCall());
+
+  const err = new Error('import failed');
+  ctx.__err = err;
+  const mod2 = new SourceTextModule(`
+    const promise = import("mod1");
+    if (Object.getPrototypeOf(promise) !== Promise.prototype) {
+      throw new Error('Expected promise to be created in the current context');
+    }
+    await promise.then(() => {
+      throw new Error('Expected promise to be rejected');
+    }, (e) => {
+      if (e !== globalThis.__err) {
+        throw new Error('Expected promise to be rejected with "import failed"');
+      }
+    });
+  `, {
+    context: ctx,
+    importModuleDynamically: common.mustCall((specifier, referrer) => {
+      throw err;
+    }),
+  });
+  // No import statements, so must not link statically.
+  await mod2.link(common.mustNotCall());
+  await mod2.evaluate();
+}
+
+Promise.all([
+  testScript(),
+  testScriptImportFailed(),
+  testModule(),
+  testModuleImportFailed(),
+]).then(common.mustCall());