Skip to content

Commit 2d5a1ef

Browse files
legendecastargos
authored andcommitted
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. PR-URL: #58309 Fixes: #53575 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
1 parent 3b1e4bd commit 2d5a1ef

File tree

2 files changed

+150
-8
lines changed

2 files changed

+150
-8
lines changed

src/module_wrap.cc

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,16 +1020,23 @@ 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
1031+
// avoid dynamic scopes.
1032+
Local<Promise::Resolver> resolver;
1033+
if (!Promise::Resolver::New(context).ToLocal(&resolver)) {
1034+
return {};
1035+
}
1036+
if (resolver->Resolve(context, result).IsNothing()) {
1037+
return {};
1038+
}
1039+
return handle_scope.Escape(resolver->GetPromise());
10331040
}
10341041

10351042
static MaybeLocal<Promise> ImportModuleDynamically(
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
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 created in the current context');
26+
}
27+
globalThis.__result = promise;
28+
`, {
29+
importModuleDynamically: common.mustCall((specifier, referrer) => {
30+
assert.strictEqual(specifier, 'mod1');
31+
assert.strictEqual(referrer, script2);
32+
return mod1;
33+
}),
34+
});
35+
script2.runInContext(ctx);
36+
37+
// Wait for the promise to resolve.
38+
await ctx.__result;
39+
}
40+
41+
async function testScriptImportFailed() {
42+
const ctx = createContext();
43+
44+
const mod1 = new SourceTextModule('export const a = 1;', {
45+
context: ctx,
46+
});
47+
// No import statements, so must not link statically.
48+
await mod1.link(common.mustNotCall());
49+
50+
const err = new Error('import failed');
51+
const script2 = new Script(`
52+
const promise = import("mod1");
53+
if (Object.getPrototypeOf(promise) !== Promise.prototype) {
54+
throw new Error('Expected promise to be created in the current context');
55+
}
56+
globalThis.__result = promise;
57+
`, {
58+
importModuleDynamically: common.mustCall((specifier, referrer) => {
59+
throw err;
60+
}),
61+
});
62+
script2.runInContext(ctx);
63+
64+
// Wait for the promise to reject.
65+
await assert.rejects(ctx.__result, err);
66+
}
67+
68+
async function testModule() {
69+
const ctx = createContext();
70+
71+
const mod1 = new SourceTextModule('export const a = 1;', {
72+
context: ctx,
73+
});
74+
// No import statements, so must not link statically.
75+
await mod1.link(common.mustNotCall());
76+
77+
const mod2 = new SourceTextModule(`
78+
const promise = import("mod1");
79+
if (Object.getPrototypeOf(promise) !== Promise.prototype) {
80+
throw new Error('Expected promise to be created in the current context');
81+
}
82+
await promise;
83+
`, {
84+
context: ctx,
85+
importModuleDynamically: common.mustCall((specifier, referrer) => {
86+
assert.strictEqual(specifier, 'mod1');
87+
assert.strictEqual(referrer, mod2);
88+
return mod1;
89+
}),
90+
});
91+
// No import statements, so must not link statically.
92+
await mod2.link(common.mustNotCall());
93+
await mod2.evaluate();
94+
}
95+
96+
async function testModuleImportFailed() {
97+
const ctx = createContext();
98+
99+
const mod1 = new SourceTextModule('export const a = 1;', {
100+
context: ctx,
101+
});
102+
// No import statements, so must not link statically.
103+
await mod1.link(common.mustNotCall());
104+
105+
const err = new Error('import failed');
106+
ctx.__err = err;
107+
const mod2 = new SourceTextModule(`
108+
const promise = import("mod1");
109+
if (Object.getPrototypeOf(promise) !== Promise.prototype) {
110+
throw new Error('Expected promise to be created in the current context');
111+
}
112+
await promise.then(() => {
113+
throw new Error('Expected promise to be rejected');
114+
}, (e) => {
115+
if (e !== globalThis.__err) {
116+
throw new Error('Expected promise to be rejected with "import failed"');
117+
}
118+
});
119+
`, {
120+
context: ctx,
121+
importModuleDynamically: common.mustCall((specifier, referrer) => {
122+
throw err;
123+
}),
124+
});
125+
// No import statements, so must not link statically.
126+
await mod2.link(common.mustNotCall());
127+
await mod2.evaluate();
128+
}
129+
130+
Promise.all([
131+
testScript(),
132+
testScriptImportFailed(),
133+
testModule(),
134+
testModuleImportFailed(),
135+
]).then(common.mustCall());

0 commit comments

Comments
 (0)