Skip to content

Commit

Permalink
vm: add importModuleDynamically option to compileFunction
Browse files Browse the repository at this point in the history
Fixes: #31860

PR-URL: #32985
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
devsnek authored and BridgeAR committed Apr 28, 2020
1 parent fce8c4e commit 84fd829
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 30 deletions.
16 changes: 15 additions & 1 deletion doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ changes:
This option is part of the experimental modules API, and should not be
considered stable.
* `specifier` {string} specifier passed to `import()`
* `module` {vm.Module}
* `script` {vm.Script}
* 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.
Expand Down Expand Up @@ -773,6 +773,10 @@ const vm = require('vm');
## `vm.compileFunction(code[, params[, options]])`
<!-- YAML
added: v10.10.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/32985
description: The `importModuleDynamically` option is now supported.
-->

* `code` {string} The body of the function to compile.
Expand All @@ -795,6 +799,16 @@ added: v10.10.0
* `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
Expand Down
40 changes: 12 additions & 28 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
const manifest = getOptionValue('--experimental-policy') ?
require('internal/process/policy').manifest :
null;
const { compileFunction } = internalBinding('contextify');

// Whether any user-provided CJS modules had been loaded (executed).
// Used for internal assertions.
Expand Down Expand Up @@ -1045,40 +1044,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
Expand Down
17 changes: 17 additions & 0 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ function compileFunction(code, params, options = {}) {
produceCachedData = false,
parsingContext = undefined,
contextExtensions = [],
importModuleDynamically,
} = options;

validateString(filename, 'options.filename');
Expand Down Expand Up @@ -360,6 +361,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;
}

Expand Down
19 changes: 18 additions & 1 deletion test/parallel/test-vm-module-basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const {
Module,
SourceTextModule,
SyntheticModule,
createContext
createContext,
compileFunction,
} = require('vm');
const util = require('util');

Expand Down Expand Up @@ -147,3 +148,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);
});
}
1 change: 1 addition & 0 deletions tools/doc/type-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ const customTypesMap = {
'URLSearchParams': 'url.html#url_class_urlsearchparams',

'vm.Module': 'vm.html#vm_class_vm_module',
'vm.Script': 'vm.html#vm_class_vm_script',
'vm.SourceTextModule': 'vm.html#vm_class_vm_sourcetextmodule',

'MessagePort': 'worker_threads.html#worker_threads_class_messageport',
Expand Down

0 comments on commit 84fd829

Please sign in to comment.