Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit cb36b67

Browse filesBrowse files
devsnekMylesBorins
devsnek
authored and
MylesBorins
committed
loader: fix up #18394
This commit fixes up some issues in #18394. * Switch vm.Module internals to use the new link method properly * Fix bug with ModuleWrap::Link * Add tests for ModuleWrap::Link PR-URL: #18509 Fixes: #18249 Refs: #18394 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
1 parent 02afdbc commit cb36b67
Copy full SHA for cb36b67

File tree

Expand file treeCollapse file tree

5 files changed

+54
-19
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+54
-19
lines changed
Open diff view settings
Collapse file

‎lib/internal/loader/ModuleWrap.js‎

Copy file name to clipboard
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
'use strict';
2+
3+
// exposes ModuleWrap for testing
4+
5+
module.exports = internalBinding('module_wrap').ModuleWrap;
Collapse file

‎lib/internal/vm/Module.js‎

Copy file name to clipboardExpand all lines: lib/internal/vm/Module.js
+18-18Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88
getConstructorOf,
99
customInspectSymbol,
1010
} = require('internal/util');
11+
const { SafePromise } = require('internal/safe_globals');
1112

1213
const {
1314
ModuleWrap,
@@ -131,27 +132,26 @@ class Module {
131132
const wrap = wrapMap.get(this);
132133
if (wrap.getStatus() !== kUninstantiated)
133134
throw new errors.Error('ERR_VM_MODULE_STATUS', 'must be uninstantiated');
135+
134136
linkingStatusMap.set(this, 'linking');
135-
const promises = [];
136-
wrap.link((specifier) => {
137-
const p = (async () => {
138-
const m = await linker(specifier, this);
139-
if (!m || !wrapMap.has(m))
140-
throw new errors.Error('ERR_VM_MODULE_NOT_MODULE');
141-
if (m.context !== this.context)
142-
throw new errors.Error('ERR_VM_MODULE_DIFFERENT_CONTEXT');
143-
const childLinkingStatus = linkingStatusMap.get(m);
144-
if (childLinkingStatus === 'errored')
145-
throw new errors.Error('ERR_VM_MODULE_LINKING_ERRORED');
146-
if (childLinkingStatus === 'unlinked')
147-
await m.link(linker);
148-
return wrapMap.get(m);
149-
})();
150-
promises.push(p);
151-
return p;
137+
138+
const promises = wrap.link(async (specifier) => {
139+
const m = await linker(specifier, this);
140+
if (!m || !wrapMap.has(m))
141+
throw new errors.Error('ERR_VM_MODULE_NOT_MODULE');
142+
if (m.context !== this.context)
143+
throw new errors.Error('ERR_VM_MODULE_DIFFERENT_CONTEXT');
144+
const childLinkingStatus = linkingStatusMap.get(m);
145+
if (childLinkingStatus === 'errored')
146+
throw new errors.Error('ERR_VM_MODULE_LINKING_ERRORED');
147+
if (childLinkingStatus === 'unlinked')
148+
await m.link(linker);
149+
return wrapMap.get(m);
152150
});
151+
153152
try {
154-
await Promise.all(promises);
153+
if (promises !== undefined)
154+
await SafePromise.all(promises);
155155
linkingStatusMap.set(this, 'linked');
156156
} catch (err) {
157157
linkingStatusMap.set(this, 'errored');
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@
109109
'lib/internal/loader/DefaultResolve.js',
110110
'lib/internal/loader/ModuleJob.js',
111111
'lib/internal/loader/ModuleMap.js',
112+
'lib/internal/loader/ModuleWrap.js',
112113
'lib/internal/loader/Translators.js',
113114
'lib/internal/safe_globals.js',
114115
'lib/internal/net.js',
Collapse file

‎src/module_wrap.cc‎

Copy file name to clipboardExpand all lines: src/module_wrap.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
197197
Local<Promise> resolve_promise = resolve_return_value.As<Promise>();
198198
obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise);
199199

200-
promises->Set(mod_context, specifier, resolve_promise).FromJust();
200+
promises->Set(mod_context, i, resolve_promise).FromJust();
201201
}
202202

203203
args.GetReturnValue().Set(promises);
Collapse file

‎test/parallel/test-internal-module-wrap.js‎

Copy file name to clipboard
+29Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
'use strict';
2+
3+
// Flags: --expose-internals
4+
5+
const common = require('../common');
6+
common.crashOnUnhandledRejection();
7+
const assert = require('assert');
8+
9+
const ModuleWrap = require('internal/loader/ModuleWrap');
10+
const { getPromiseDetails, isPromise } = process.binding('util');
11+
const setTimeoutAsync = require('util').promisify(setTimeout);
12+
13+
const foo = new ModuleWrap('export * from "bar"; 6;', 'foo');
14+
const bar = new ModuleWrap('export const five = 5', 'bar');
15+
16+
(async () => {
17+
const promises = foo.link(() => setTimeoutAsync(1000).then(() => bar));
18+
assert.strictEqual(promises.length, 1);
19+
assert(isPromise(promises[0]));
20+
21+
await Promise.all(promises);
22+
23+
assert.strictEqual(getPromiseDetails(promises[0])[1], bar);
24+
25+
foo.instantiate();
26+
27+
assert.strictEqual(await foo.evaluate(), 6);
28+
assert.strictEqual(foo.namespace().five, 5);
29+
})();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.