FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

vm: import call should return a promise in the current context · nodejs/node@9f2e5aa · GitHub

/ node Public

Commit 9f2e5aa

Browse files
legendecas
authored and
aduh95
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. PR-URL: #58309 Fixes: #53575 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
1 parent 72fac71 commit 9f2e5aa

2 files changed

Lines changed: 150 additions & 8 deletions

File tree

‎src/module_wrap.cc‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -905,16 +905,23 @@ static MaybeLocal<Promise> ImportModuleDynamically(
905905
};
906906

907907
Local<Value> result;
908-
if (import_callback->Call(
909-
context,
910-
Undefined(isolate),
911-
arraysize(import_args),
912-
import_args).ToLocal(&result)) {
913-
CHECK(result->IsPromise());
914-
return handle_scope.Escape(result.As<Promise>());
908+
if (!import_callback
909+
->Call(
910+
context, Undefined(isolate), arraysize(import_args), import_args)
911+
.ToLocal(&result)) {
912+
return {};
915913
}
916914

917-
return MaybeLocal<Promise>();
915+
// Wrap the returned value in a promise created in the referrer context to
916+
// avoid dynamic scopes.
917+
Local<Promise::Resolver> resolver;
918+
if (!Promise::Resolver::New(context).ToLocal(&resolver)) {
919+
return {};
920+
}
921+
if (resolver->Resolve(context, result).IsNothing()) {
922+
return {};
923+
}
924+
return handle_scope.Escape(resolver->GetPromise());
918925
}
919926

920927
void ModuleWrap::SetImportModuleDynamicallyCallback(

‎test/parallel/test-vm-module-dynamic-import-promise.js‎

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)
Back | FazBrowse Home | New Git URL