vm: refactor SourceTextModule

- 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 <targos@protonmail.com>

PR-URL: https://github.com/nodejs/node/pull/29776
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>

10 files changed, 49 insertions, 118 deletions

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: <uninitialized>, 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 {