From 5e7946fe79193633e9a78aae48548635cbc11023 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Mon, 9 Sep 2019 12:20:16 -0500 Subject: vm: refactor SourceTextModule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Removes redundant `instantiate` method - Refactors `link` to match the spec linking steps more accurately - Removes URL validation from SourceTextModule specifiers - DRYs some dynamic import logic Closes: https://github.com/nodejs/node/issues/29030 Co-Authored-By: Michaël Zasso PR-URL: https://github.com/nodejs/node/pull/29776 Reviewed-By: James M Snell Reviewed-By: Michaël Zasso Reviewed-By: Minwoo Jung --- test/parallel/test-internal-module-wrap.js | 2 +- test/parallel/test-util-inspect-namespace.js | 1 - test/parallel/test-util-types.js | 1 - test/parallel/test-vm-module-basic.js | 29 +++++---- test/parallel/test-vm-module-dynamic-import.js | 4 -- test/parallel/test-vm-module-dynamic-namespace.js | 17 ++--- test/parallel/test-vm-module-errors.js | 75 +++-------------------- test/parallel/test-vm-module-import-meta.js | 1 - test/parallel/test-vm-module-link.js | 35 +++++------ test/parallel/test-vm-module-reevaluate.js | 2 - 10 files changed, 49 insertions(+), 118 deletions(-) (limited to 'test') diff --git a/test/parallel/test-internal-module-wrap.js b/test/parallel/test-internal-module-wrap.js index ee0b722d45..116a300543 100644 --- a/test/parallel/test-internal-module-wrap.js +++ b/test/parallel/test-internal-module-wrap.js @@ -25,5 +25,5 @@ const bar = new ModuleWrap('export const five = 5', 'bar'); foo.instantiate(); assert.strictEqual(await foo.evaluate(-1, false), 6); - assert.strictEqual(foo.namespace().five, 5); + assert.strictEqual(foo.getNamespace().five, 5); })(); diff --git a/test/parallel/test-util-inspect-namespace.js b/test/parallel/test-util-inspect-namespace.js index f2b6e57178..89b26fcdbc 100644 --- a/test/parallel/test-util-inspect-namespace.js +++ b/test/parallel/test-util-inspect-namespace.js @@ -11,7 +11,6 @@ const { inspect } = require('util'); (async () => { const m = new SourceTextModule('export const a = 1; export var b = 2'); await m.link(() => 0); - m.instantiate(); assert.strictEqual( inspect(m.namespace), '[Module] { a: , b: undefined }'); diff --git a/test/parallel/test-util-types.js b/test/parallel/test-util-types.js index 69ebaaba12..6a9bad0169 100644 --- a/test/parallel/test-util-types.js +++ b/test/parallel/test-util-types.js @@ -281,7 +281,6 @@ for (const [ value, _method ] of [ (async () => { const m = new vm.SourceTextModule(''); await m.link(() => 0); - m.instantiate(); await m.evaluate(); assert.ok(types.isModuleNamespaceObject(m.namespace)); })(); diff --git a/test/parallel/test-vm-module-basic.js b/test/parallel/test-vm-module-basic.js index 76a8e8a3d2..cbe5e29d3c 100644 --- a/test/parallel/test-vm-module-basic.js +++ b/test/parallel/test-vm-module-basic.js @@ -17,10 +17,9 @@ const util = require('util'); 'baz = foo; typeofProcess = typeof process; typeof Object;', { context } ); - assert.strictEqual(m.status, 'uninstantiated'); + assert.strictEqual(m.status, 'unlinked'); await m.link(common.mustNotCall()); - m.instantiate(); - assert.strictEqual(m.status, 'instantiated'); + assert.strictEqual(m.status, 'linked'); const result = await m.evaluate(); assert.strictEqual(m.status, 'evaluated'); assert.strictEqual(Object.getPrototypeOf(result), null); @@ -37,7 +36,6 @@ const util = require('util'); 'global.vmResult = "foo"; Object.prototype.toString.call(process);' ); await m.link(common.mustNotCall()); - m.instantiate(); const { result } = await m.evaluate(); assert.strictEqual(global.vmResult, 'foo'); assert.strictEqual(result, '[object process]'); @@ -47,22 +45,21 @@ const util = require('util'); (async () => { const m = new SourceTextModule('while (true) {}'); await m.link(common.mustNotCall()); - m.instantiate(); await m.evaluate({ timeout: 500 }) .then(() => assert(false), () => {}); })(); -// Check the generated url for each module +// Check the generated identifier for each module (async () => { const context1 = createContext({ }); const context2 = createContext({ }); const m1 = new SourceTextModule('1', { context: context1 }); - assert.strictEqual(m1.url, 'vm:module(0)'); + assert.strictEqual(m1.identifier, 'vm:module(0)'); const m2 = new SourceTextModule('2', { context: context1 }); - assert.strictEqual(m2.url, 'vm:module(1)'); + assert.strictEqual(m2.identifier, 'vm:module(1)'); const m3 = new SourceTextModule('3', { context: context2 }); - assert.strictEqual(m3.url, 'vm:module(0)'); + assert.strictEqual(m3.identifier, 'vm:module(0)'); })(); // Check inspection of the instance @@ -72,13 +69,19 @@ const util = require('util'); assert.strictEqual( util.inspect(m), - "SourceTextModule {\n status: 'uninstantiated',\n linkingStatus:" + - " 'unlinked',\n url: 'vm:module(0)',\n context: { foo: 'bar' }\n}" + `SourceTextModule { + status: 'unlinked', + identifier: 'vm:module(0)', + context: { foo: 'bar' } +}` ); assert.strictEqual( m[util.inspect.custom].call(Object.create(null)), - 'SourceTextModule {\n status: undefined,\n linkingStatus: undefined,' + - '\n url: undefined,\n context: undefined\n}' + `SourceTextModule { + status: undefined, + identifier: undefined, + context: undefined +}`, ); assert.strictEqual(util.inspect(m, { depth: -1 }), '[SourceTextModule]'); } diff --git a/test/parallel/test-vm-module-dynamic-import.js b/test/parallel/test-vm-module-dynamic-import.js index 600f455e93..897d9f27d7 100644 --- a/test/parallel/test-vm-module-dynamic-import.js +++ b/test/parallel/test-vm-module-dynamic-import.js @@ -10,7 +10,6 @@ const { Script, SourceTextModule, createContext } = require('vm'); async function testNoCallback() { const m = new SourceTextModule('import("foo")', { context: createContext() }); await m.link(common.mustNotCall()); - m.instantiate(); const { result } = await m.evaluate(); let threw = false; try { @@ -25,7 +24,6 @@ async function testNoCallback() { async function test() { const foo = new SourceTextModule('export const a = 1;'); await foo.link(common.mustNotCall()); - foo.instantiate(); await foo.evaluate(); { @@ -50,7 +48,6 @@ async function test() { }), }); await m.link(common.mustNotCall()); - m.instantiate(); const { result } = await m.evaluate(); assert.strictEqual(foo.namespace, await result); } @@ -63,7 +60,6 @@ async function testInvalid() { }), }); await m.link(common.mustNotCall()); - m.instantiate(); const { result } = await m.evaluate(); await result.catch(common.mustCall((e) => { assert.strictEqual(e.code, 'ERR_VM_MODULE_NOT_MODULE'); diff --git a/test/parallel/test-vm-module-dynamic-namespace.js b/test/parallel/test-vm-module-dynamic-namespace.js index bcd91daebd..987b21f2ec 100644 --- a/test/parallel/test-vm-module-dynamic-namespace.js +++ b/test/parallel/test-vm-module-dynamic-namespace.js @@ -1,35 +1,30 @@ 'use strict'; -// Flags: --experimental-vm-modules --expose-internals -// +// Flags: --experimental-vm-modules + const common = require('../common'); const assert = require('assert'); const { types } = require('util'); -const { SourceTextModule, wrapMap } = require('internal/vm/source_text_module'); - -const { importModuleDynamicallyCallback } = - require('internal/process/esm_loader'); +const { SourceTextModule } = require('vm'); async function getNamespace() { const m = new SourceTextModule(''); await m.link(() => 0); - m.instantiate(); await m.evaluate(); return m.namespace; } (async () => { const namespace = await getNamespace(); - const m = new SourceTextModule('export const A = "A";', { + const m = new SourceTextModule('export const A = "A"; import("");', { importModuleDynamically: common.mustCall((specifier, wrap) => { return namespace; }) }); await m.link(() => 0); - m.instantiate(); - await m.evaluate(); - const ns = await importModuleDynamicallyCallback(wrapMap.get(m)); + const { result } = await m.evaluate(); + const ns = await result; assert.ok(types.isModuleNamespaceObject(ns)); })().then(common.mustCall()); diff --git a/test/parallel/test-vm-module-errors.js b/test/parallel/test-vm-module-errors.js index a92863b125..e76bbbb0e1 100644 --- a/test/parallel/test-vm-module-errors.js +++ b/test/parallel/test-vm-module-errors.js @@ -23,7 +23,7 @@ async function checkArgType() { }); for (const invalidOptions of [ - 0, 1, null, true, 'str', () => {}, { url: 0 }, Symbol.iterator, + 0, 1, null, true, 'str', () => {}, { identifier: 0 }, Symbol.iterator, { context: null }, { context: 'hucairz' }, { context: {} } ]) { common.expectsError(() => { @@ -52,7 +52,7 @@ async function checkModuleState() { await assert.rejects(async () => { const m = new SourceTextModule(''); await m.link(common.mustNotCall()); - assert.strictEqual(m.linkingStatus, 'linked'); + assert.strictEqual(m.status, 'linked'); await m.link(common.mustNotCall()); }, { code: 'ERR_VM_MODULE_ALREADY_LINKED' @@ -61,54 +61,18 @@ async function checkModuleState() { await assert.rejects(async () => { const m = new SourceTextModule(''); m.link(common.mustNotCall()); - assert.strictEqual(m.linkingStatus, 'linking'); + assert.strictEqual(m.status, 'linking'); await m.link(common.mustNotCall()); }, { code: 'ERR_VM_MODULE_ALREADY_LINKED' }); - common.expectsError(() => { - const m = new SourceTextModule(''); - m.instantiate(); - }, { - code: 'ERR_VM_MODULE_NOT_LINKED' - }); - - await assert.rejects(async () => { - const m = new SourceTextModule('import "foo";'); - try { - await m.link(common.mustCall(() => ({}))); - } catch { - assert.strictEqual(m.linkingStatus, 'errored'); - m.instantiate(); - } - }, { - code: 'ERR_VM_MODULE_NOT_LINKED' - }); - - { - const m = new SourceTextModule('import "foo";'); - await m.link(common.mustCall(async (specifier, module) => { - assert.strictEqual(module, m); - assert.strictEqual(specifier, 'foo'); - assert.strictEqual(m.linkingStatus, 'linking'); - common.expectsError(() => { - m.instantiate(); - }, { - code: 'ERR_VM_MODULE_NOT_LINKED' - }); - return new SourceTextModule(''); - })); - m.instantiate(); - await m.evaluate(); - } - await assert.rejects(async () => { const m = new SourceTextModule(''); await m.evaluate(); }, { code: 'ERR_VM_MODULE_STATUS', - message: 'Module status must be one of instantiated, evaluated, and errored' + message: 'Module status must be one of linked, evaluated, or errored' }); await assert.rejects(async () => { @@ -120,14 +84,6 @@ async function checkModuleState() { 'Received type boolean' }); - await assert.rejects(async () => { - const m = await createEmptyLinkedModule(); - await m.evaluate(); - }, { - code: 'ERR_VM_MODULE_STATUS', - message: 'Module status must be one of instantiated, evaluated, and errored' - }); - common.expectsError(() => { const m = new SourceTextModule(''); m.error; @@ -138,7 +94,6 @@ async function checkModuleState() { await assert.rejects(async () => { const m = await createEmptyLinkedModule(); - m.instantiate(); await m.evaluate(); m.error; }, { @@ -151,15 +106,7 @@ async function checkModuleState() { m.namespace; }, { code: 'ERR_VM_MODULE_STATUS', - message: 'Module status must not be uninstantiated or instantiating' - }); - - await assert.rejects(async () => { - const m = await createEmptyLinkedModule(); - m.namespace; - }, { - code: 'ERR_VM_MODULE_STATUS', - message: 'Module status must not be uninstantiated or instantiating' + message: 'Module status must not be unlinked or linking' }); } @@ -170,7 +117,7 @@ async function checkLinking() { try { await m.link(common.mustCall(() => ({}))); } catch (err) { - assert.strictEqual(m.linkingStatus, 'errored'); + assert.strictEqual(m.status, 'errored'); throw err; } }, { @@ -185,7 +132,7 @@ async function checkLinking() { try { await bar.link(common.mustCall(() => foo)); } catch (err) { - assert.strictEqual(bar.linkingStatus, 'errored'); + assert.strictEqual(bar.status, 'errored'); throw err; } }, { @@ -199,7 +146,7 @@ async function checkLinking() { } catch { // ignored } finally { - assert.strictEqual(erroredModule.linkingStatus, 'errored'); + assert.strictEqual(erroredModule.status, 'errored'); } const rootModule = new SourceTextModule('import "errored";'); @@ -224,19 +171,17 @@ common.expectsError(() => { async function checkExecution() { await (async () => { const m = new SourceTextModule('import { nonexistent } from "module";'); - await m.link(common.mustCall(() => new SourceTextModule(''))); // There is no code for this exception since it is thrown by the JavaScript // engine. - assert.throws(() => { - m.instantiate(); + await assert.rejects(() => { + return m.link(common.mustCall(() => new SourceTextModule(''))); }, SyntaxError); })(); await (async () => { const m = new SourceTextModule('throw new Error();'); await m.link(common.mustNotCall()); - m.instantiate(); const evaluatePromise = m.evaluate(); await evaluatePromise.catch(() => {}); assert.strictEqual(m.status, 'errored'); diff --git a/test/parallel/test-vm-module-import-meta.js b/test/parallel/test-vm-module-import-meta.js index 4886464f34..1db4467b08 100644 --- a/test/parallel/test-vm-module-import-meta.js +++ b/test/parallel/test-vm-module-import-meta.js @@ -14,7 +14,6 @@ async function testBasic() { }) }); await m.link(common.mustNotCall()); - m.instantiate(); const { result } = await m.evaluate(); assert.strictEqual(typeof result, 'object'); assert.strictEqual(Object.getPrototypeOf(result), null); diff --git a/test/parallel/test-vm-module-link.js b/test/parallel/test-vm-module-link.js index 20518c4054..9678d13737 100644 --- a/test/parallel/test-vm-module-link.js +++ b/test/parallel/test-vm-module-link.js @@ -23,8 +23,6 @@ async function simple() { return foo; })); - bar.instantiate(); - assert.strictEqual((await bar.evaluate()).result, 5); } @@ -49,7 +47,6 @@ async function depth() { const baz = await getProxy('bar', bar); const barz = await getProxy('baz', baz); - barz.instantiate(); await barz.evaluate(); assert.strictEqual(barz.namespace.default, 5); @@ -67,20 +64,19 @@ async function circular() { return foo; } `); - await foo.link(common.mustCall(async (fooSpecifier, fooModule) => { - assert.strictEqual(fooModule, foo); - assert.strictEqual(fooSpecifier, 'bar'); - await bar.link(common.mustCall((barSpecifier, barModule) => { - assert.strictEqual(barModule, bar); - assert.strictEqual(barSpecifier, 'foo'); - assert.strictEqual(foo.linkingStatus, 'linking'); - return foo; - })); - assert.strictEqual(bar.linkingStatus, 'linked'); - return bar; - })); + await foo.link(common.mustCall(async (specifier, module) => { + if (specifier === 'bar') { + assert.strictEqual(module, foo); + return bar; + } + assert.strictEqual(specifier, 'foo'); + assert.strictEqual(module, bar); + assert.strictEqual(foo.status, 'linking'); + return foo; + }, 2)); + + assert.strictEqual(bar.status, 'linked'); - foo.instantiate(); await foo.evaluate(); assert.strictEqual(foo.namespace.default, 42); } @@ -109,19 +105,20 @@ async function circular2() { ` }; const moduleMap = new Map(); - const rootModule = new SourceTextModule(sourceMap.root, { url: 'vm:root' }); + const rootModule = new SourceTextModule(sourceMap.root, { + identifier: 'vm:root', + }); async function link(specifier, referencingModule) { if (moduleMap.has(specifier)) { return moduleMap.get(specifier); } const mod = new SourceTextModule(sourceMap[specifier], { - url: new URL(specifier, 'file:///').href, + identifier: new URL(specifier, 'file:///').href, }); moduleMap.set(specifier, mod); return mod; } await rootModule.link(link); - rootModule.instantiate(); await rootModule.evaluate(); } diff --git a/test/parallel/test-vm-module-reevaluate.js b/test/parallel/test-vm-module-reevaluate.js index c3914f362f..4767541dd2 100644 --- a/test/parallel/test-vm-module-reevaluate.js +++ b/test/parallel/test-vm-module-reevaluate.js @@ -14,7 +14,6 @@ const finished = common.mustCall(); { const m = new SourceTextModule('1'); await m.link(common.mustNotCall()); - m.instantiate(); assert.strictEqual((await m.evaluate()).result, 1); assert.strictEqual((await m.evaluate()).result, undefined); assert.strictEqual((await m.evaluate()).result, undefined); @@ -23,7 +22,6 @@ const finished = common.mustCall(); { const m = new SourceTextModule('throw new Error()'); await m.link(common.mustNotCall()); - m.instantiate(); let threw = false; try { -- cgit v1.2.3