Skip to content

Commit 819f136

Browse files
committed
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.
1 parent 91d2400 commit 819f136

File tree

2 files changed

+80
-8
lines changed

2 files changed

+80
-8
lines changed

src/module_wrap.cc

+13-8
Original file line numberDiff line numberDiff line change
@@ -1020,16 +1020,21 @@ static MaybeLocal<Promise> ImportModuleDynamicallyWithPhase(
10201020
};
10211021

10221022
Local<Value> result;
1023-
if (import_callback->Call(
1024-
context,
1025-
Undefined(isolate),
1026-
arraysize(import_args),
1027-
import_args).ToLocal(&result)) {
1028-
CHECK(result->IsPromise());
1029-
return handle_scope.Escape(result.As<Promise>());
1023+
if (!import_callback
1024+
->Call(
1025+
context, Undefined(isolate), arraysize(import_args), import_args)
1026+
.ToLocal(&result)) {
1027+
return {};
10301028
}
10311029

1032-
return MaybeLocal<Promise>();
1030+
// Wrap the returned value in a promise created in the referrer context to avoid
1031+
// dynamic scopes.
1032+
Local<Promise::Resolver> resolver;
1033+
if (!Promise::Resolver::New(context).ToLocal(&resolver)) {
1034+
return {};
1035+
}
1036+
1037+
return handle_scope.Escape(resolver->GetPromise());
10331038
}
10341039

10351040
static MaybeLocal<Promise> ImportModuleDynamically(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Flags: --experimental-vm-modules
2+
'use strict';
3+
4+
const common = require('../common');
5+
6+
const assert = require('assert');
7+
const { createContext, Script, SourceTextModule } = require('vm');
8+
9+
// Verifies that a `import` call returns a promise created in the context
10+
// where the `import` was called, not the context of `importModuleDynamically`
11+
// callback.
12+
13+
async function testScript() {
14+
const ctx = createContext();
15+
16+
const mod1 = new SourceTextModule('export const a = 1;', {
17+
context: ctx,
18+
});
19+
// No import statements, so must not link statically.
20+
await mod1.link(common.mustNotCall());
21+
22+
const script2 = new Script(`
23+
const promise = import("mod1");
24+
if (Object.getPrototypeOf(promise) !== Promise.prototype) {
25+
throw new Error('Expected promise to be a Promise');
26+
}
27+
`, {
28+
importModuleDynamically: common.mustCall((specifier, referrer) => {
29+
assert.strictEqual(specifier, 'mod1');
30+
assert.strictEqual(referrer, script2);
31+
return mod1;
32+
}),
33+
});
34+
script2.runInContext(ctx);
35+
}
36+
37+
async function testModule() {
38+
const ctx = createContext();
39+
40+
const mod1 = new SourceTextModule('export const a = 1;', {
41+
context: ctx,
42+
});
43+
// No import statements, so must not link statically.
44+
await mod1.link(common.mustNotCall());
45+
46+
const mod2 = new SourceTextModule(`
47+
const promise = import("mod1");
48+
if (Object.getPrototypeOf(promise) !== Promise.prototype) {
49+
throw new Error('Expected promise to be a Promise');
50+
}
51+
`, {
52+
context: ctx,
53+
importModuleDynamically: common.mustCall((specifier, referrer) => {
54+
assert.strictEqual(specifier, 'mod1');
55+
assert.strictEqual(referrer, mod2);
56+
return mod1;
57+
}),
58+
});
59+
// No import statements, so must not link statically.
60+
await mod2.link(common.mustNotCall());
61+
await mod2.evaluate();
62+
}
63+
64+
Promise.all([
65+
testScript(),
66+
testModule(),
67+
]).then(common.mustCall());

0 commit comments

Comments
 (0)