From bf2f2b74faeeb5ad7fc8e483a80bae3064e3e717 Mon Sep 17 00:00:00 2001
From: Gus Caplan <me@gus.host>
Date: Fri, 25 Sep 2020 15:22:52 -0500
Subject: [PATCH] vm: add importModuleDynamically option to compileFunction

This reverts commit 2d5d77306f6dff9110c1f77fefab25f973415770.

See: https://github.com/nodejs/node/pull/32985
See: https://github.com/nodejs/node/pull/33364
See: https://github.com/nodejs/node/issues/33166
Fixes: https://github.com/nodejs/node/issues/31860

PR-URL: https://github.com/nodejs/node/pull/35431
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
---
 doc/api/vm.md                         | 13 +++++++++
 lib/internal/modules/cjs/loader.js    | 40 ++++++++-------------------
 lib/vm.js                             | 17 ++++++++++++
 test/parallel/test-vm-module-basic.js | 19 ++++++++++++-
 4 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/doc/api/vm.md b/doc/api/vm.md
index 1bd9303fc4ef0d..cb97cda7d791c0 100644
--- a/doc/api/vm.md
+++ b/doc/api/vm.md
@@ -813,6 +813,9 @@ const vm = require('vm');
 <!-- YAML
 added: v10.10.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/35431
+    description: Added `importModuleDynamically` option again.
   - version: v14.3.0
     pr-url: https://github.com/nodejs/node/pull/33364
     description: Removal of `importModuleDynamically` due to compatibility
@@ -844,6 +847,16 @@ changes:
   * `contextExtensions` {Object[]} An array containing a collection of context
     extensions (objects wrapping the current scope) to be applied while
     compiling. **Default:** `[]`.
+  * `importModuleDynamically` {Function} Called during evaluation of this module
+    when `import()` is called. If this option is not specified, calls to
+    `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
+    This option is part of the experimental modules API, and should not be
+    considered stable.
+    * `specifier` {string} specifier passed to `import()`
+    * `function` {Function}
+    * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
+      recommended in order to take advantage of error tracking, and to avoid
+      issues with namespaces that contain `then` function exports.
 * Returns: {Function}
 
 Compiles the given code into the provided context (if no context is
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index 0c69d547d40b9a..5abfa465e0407d 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -101,7 +101,6 @@ const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
 const policy = getOptionValue('--experimental-policy') ?
   require('internal/process/policy') :
   null;
-const { compileFunction } = internalBinding('contextify');
 
 // Whether any user-provided CJS modules had been loaded (executed).
 // Used for internal assertions.
@@ -1019,40 +1018,25 @@ function wrapSafe(filename, content, cjsModuleInstance) {
       },
     });
   }
-  let compiled;
   try {
-    compiled = compileFunction(
-      content,
+    return vm.compileFunction(content, [
+      'exports',
+      'require',
+      'module',
+      '__filename',
+      '__dirname',
+    ], {
       filename,
-      0,
-      0,
-      undefined,
-      false,
-      undefined,
-      [],
-      [
-        'exports',
-        'require',
-        'module',
-        '__filename',
-        '__dirname',
-      ]
-    );
+      importModuleDynamically(specifier) {
+        const loader = asyncESM.ESMLoader;
+        return loader.import(specifier, normalizeReferrerURL(filename));
+      },
+    });
   } catch (err) {
     if (process.mainModule === cjsModuleInstance)
       enrichCJSError(err);
     throw err;
   }
-
-  const { callbackMap } = internalBinding('module_wrap');
-  callbackMap.set(compiled.cacheKey, {
-    importModuleDynamically: async (specifier) => {
-      const loader = asyncESM.ESMLoader;
-      return loader.import(specifier, normalizeReferrerURL(filename));
-    }
-  });
-
-  return compiled.function;
 }
 
 // Run the file contents in the correct scope or sandbox. Expose
diff --git a/lib/vm.js b/lib/vm.js
index 56f4a7c3af1875..c873842c434c30 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -324,6 +324,7 @@ function compileFunction(code, params, options = {}) {
     produceCachedData = false,
     parsingContext = undefined,
     contextExtensions = [],
+    importModuleDynamically,
   } = options;
 
   validateString(filename, 'options.filename');
@@ -371,6 +372,22 @@ function compileFunction(code, params, options = {}) {
     result.function.cachedData = result.cachedData;
   }
 
+  if (importModuleDynamically !== undefined) {
+    if (typeof importModuleDynamically !== 'function') {
+      throw new ERR_INVALID_ARG_TYPE('options.importModuleDynamically',
+                                     'function',
+                                     importModuleDynamically);
+    }
+    const { importModuleDynamicallyWrap } =
+      require('internal/vm/module');
+    const { callbackMap } = internalBinding('module_wrap');
+    const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
+    const func = result.function;
+    callbackMap.set(result.cacheKey, {
+      importModuleDynamically: (s, _k) => wrapped(s, func),
+    });
+  }
+
   return result.function;
 }
 
diff --git a/test/parallel/test-vm-module-basic.js b/test/parallel/test-vm-module-basic.js
index 0376473bf7123c..d162f1bebd744c 100644
--- a/test/parallel/test-vm-module-basic.js
+++ b/test/parallel/test-vm-module-basic.js
@@ -8,7 +8,8 @@ const {
   Module,
   SourceTextModule,
   SyntheticModule,
-  createContext
+  createContext,
+  compileFunction,
 } = require('vm');
 const util = require('util');
 
@@ -160,3 +161,19 @@ const util = require('util');
     name: 'TypeError'
   });
 }
+
+// Test compileFunction importModuleDynamically
+{
+  const module = new SyntheticModule([], () => {});
+  module.link(() => {});
+  const f = compileFunction('return import("x")', [], {
+    importModuleDynamically(specifier, referrer) {
+      assert.strictEqual(specifier, 'x');
+      assert.strictEqual(referrer, f);
+      return module;
+    },
+  });
+  f().then((ns) => {
+    assert.strictEqual(ns, module.namespace);
+  });
+}