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

loader: fix up #18394 · nodejs/node@b0f114d · GitHub

/ node Public

Commit b0f114d

Browse files
devsnek
authored and
TimothyGu
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 05e702d commit b0f114d

5 files changed

Lines changed: 54 additions & 19 deletions

File tree

‎lib/internal/loader/ModuleWrap.js‎

Lines 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;

‎lib/internal/vm/Module.js‎

Lines 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');

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
'lib/internal/loader/DefaultResolve.js',
108108
'lib/internal/loader/ModuleJob.js',
109109
'lib/internal/loader/ModuleMap.js',
110+
'lib/internal/loader/ModuleWrap.js',
110111
'lib/internal/loader/Translators.js',
111112
'lib/internal/safe_globals.js',
112113
'lib/internal/net.js',

‎src/module_wrap.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
209209
Local<Promise> resolve_promise = resolve_return_value.As<Promise>();
210210
obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise);
211211

212-
promises->Set(mod_context, specifier, resolve_promise).FromJust();
212+
promises->Set(mod_context, i, resolve_promise).FromJust();
213213
}
214214

215215
args.GetReturnValue().Set(promises);

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

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