Skip to content

Commit

Permalink
Revert "Revert "vm: add importModuleDynamically option to compileFunc…
Browse files Browse the repository at this point in the history
…tion""

This reverts commit 2d5d773.

See: #32985
See: #33364
See: #33166
Fixes: #31860
  • Loading branch information
devsnek committed Feb 5, 2021
1 parent b5f5c46 commit 28f5ec8
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 29 deletions.
13 changes: 13 additions & 0 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
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 @@ -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.
Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ function compileFunction(code, params, options = {}) {
produceCachedData = false,
parsingContext = undefined,
contextExtensions = [],
importModuleDynamically,
} = options;

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

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 @@ -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);
});
}

0 comments on commit 28f5ec8

Please sign in to comment.