Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce loader hook context #20761

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,25 @@ provided via a `--loader ./loader-name.mjs` argument to Node.js.
When hooks are used they only apply to ES module loading and not to any
CommonJS modules loaded.

### Hook Context

This context contains access to functions that are scoped to the current load
process.

A hook context consists of the following properties:

- `defaultResolve` {Function} A shortcut to the resolve algorithm that ships
with Node.js.
- `specifier` {string} The specifier of the module to import.
- `parentURL` {string} The URL of the module that requested the specifier.
- `resolve` {Function} A shortcut to the current resolve function. Especially
useful if resolve is not hooked.
- `specifier` {string} The specifier of the module to import.
- `parentURL` {string} The URL of the module that requested the specifier.
- `vmModuleLinkHook` {object} This value can be passed to [`module.link`][] to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`module.link` -> `module.link()`?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{object} -> {Object}.

allow linking the import requests of a [`vm.Module`][] instance to the
loader.

### Resolve hook

The resolve hook returns the resolved file URL and module format for a
Expand All @@ -153,7 +172,7 @@ baseURL.pathname = `${process.cwd()}/`;

export async function resolve(specifier,
parentModuleURL = baseURL,
defaultResolver) {
hookContext) {
return {
url: new URL(specifier, parentModuleURL).href,
format: 'esm'
Expand Down Expand Up @@ -195,7 +214,7 @@ const JS_EXTENSIONS = new Set(['.js', '.mjs']);
const baseURL = new URL('file://');
baseURL.pathname = `${process.cwd()}/`;

export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) {
export function resolve(specifier, parentModuleURL = baseURL, hookContext) {
if (builtins.includes(specifier)) {
return {
url: specifier,
Expand All @@ -204,7 +223,7 @@ export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) {
}
if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) {
// For node_modules support:
// return defaultResolve(specifier, parentModuleURL);
// return hookContext.defaultResolve(specifier, parentModuleURL);
throw new Error(
`imports must begin with '/', './', or '../'; '${specifier}' does not`);
}
Expand Down Expand Up @@ -238,7 +257,7 @@ This hook is called only for modules that return `format: 'dynamic'` from
the `resolve` hook.

```js
export async function dynamicInstantiate(url) {
export async function dynamicInstantiate(url, hookContext) {
return {
exports: ['customExportName'],
execute: (exports) => {
Expand All @@ -253,6 +272,8 @@ With the list of module exports provided upfront, the `execute` function will
then be called at the exact point of module evaluation order for that module
in the import tree.

[`module.link`]: vm.html#vm_module_link_linker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`module.link` -> `module.link()`?

[`vm.Module`]: vm.html#vm_class_vm_module
[Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md
[addons]: addons.html
[dynamic instantiate hook]: #esm_dynamic_instantiate_hook
3 changes: 3 additions & 0 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ that point all modules would have been fully linked already, the
[HostResolveImportedModule][] implementation is fully synchronous per
specification.

The linker may also be passed from [`hookContext.vmModuleLinkhook`][].

## Class: vm.Script
<!-- YAML
added: v0.3.1
Expand Down Expand Up @@ -893,6 +895,7 @@ associating it with the `sandbox` object is what this document refers to as
[`Error`]: errors.html#errors_class_error
[`URL`]: url.html#url_class_url
[`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval
[`hookContext.vmModuleLinkHook`]: esm.html#esm_hook_context
[`script.runInContext()`]: #vm_script_runincontext_contextifiedsandbox_options
[`script.runInThisContext()`]: #vm_script_runinthiscontext_options
[`url.origin`]: url.html#url_url_origin
Expand Down
16 changes: 14 additions & 2 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const defaultResolve = require('internal/modules/esm/default_resolve');
const createDynamicModule = require(
'internal/modules/esm/create_dynamic_module');
const translators = require('internal/modules/esm/translators');
const { vmModuleLinkHookMap } = require('internal/vm/module');

const FunctionBind = Function.call.bind(Function.prototype.bind);

Expand Down Expand Up @@ -45,6 +46,16 @@ class Loader {
// an object with the same keys as `exports`, whose values are get/set
// functions for the actual exported values.
this._dynamicInstantiate = undefined;

// Set up context passed to hooks
const k = Object.freeze(Object.create(null));
vmModuleLinkHookMap.set(k, this);

this.hookContext = Object.assign(Object.create(null), {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a different name since this has relations to vm and Context has a meaning of a Realm in the vm terminology.

This also seems to have some conflict with #18914 that I need to think on.

defaultResolve,
resolve: (specifier, parentURL) => this.resolve(specifier, parentURL),
vmModuleLinkHook: k,
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to deny the hook context access to outside world, you are not – the [[Prototype]] of these functions are the %FunctionPrototype% of the outside world. It would be better to create these functions in the context directly, if possible, rather than using Object/Reflect.setPrototypeOf().

}

async resolve(specifier, parentURL) {
Expand All @@ -53,7 +64,7 @@ class Loader {
throw new ERR_INVALID_ARG_TYPE('parentURL', 'string', parentURL);

const { url, format } =
await this._resolve(specifier, parentURL, defaultResolve);
await this._resolve(specifier, parentURL, this.hookContext);

if (typeof url !== 'string')
throw new ERR_INVALID_ARG_TYPE('url', 'string', url);
Expand Down Expand Up @@ -97,7 +108,8 @@ class Loader {

loaderInstance = async (url) => {
debug(`Translating dynamic ${url}`);
const { exports, execute } = await this._dynamicInstantiate(url);
const { exports, execute } =
await this._dynamicInstantiate(url, this.hookContext);
return createDynamicModule(exports, url, (reflect) => {
debug(`Loading dynamic ${url}`);
execute(reflect.exports);
Expand Down
17 changes: 16 additions & 1 deletion lib/internal/vm/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const {
const { SafePromise } = require('internal/safe_globals');
const { validateInt32, validateUint32 } = require('internal/validators');

const vmModuleLinkHookMap = new WeakMap();

const {
ModuleWrap,
kUninstantiated,
Expand Down Expand Up @@ -151,6 +153,15 @@ class Module {
}

async link(linker) {
const linkingFromLoader = vmModuleLinkHookMap.has(linker);
if (linkingFromLoader) {
const loader = vmModuleLinkHookMap.get(linker);
linker = (specifier, parent) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason not to use an async function here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

habits are hard to break :) nice catch

loader.getModuleJob(specifier, parent.url)
.then((j) => j.modulePromise)
.then((r) => r.module);
}

if (typeof linker !== 'function')
throw new ERR_INVALID_ARG_TYPE('linker', 'function', linker);
if (linkingStatusMap.get(this) !== 'unlinked')
Expand All @@ -163,6 +174,9 @@ class Module {

const promises = wrap.link(async (specifier) => {
const m = await linker(specifier, this);
if (linkingFromLoader) {
return m;
}
if (!m || !wrapMap.has(m))
throw new ERR_VM_MODULE_NOT_MODULE();
if (m.context !== this.context)
Expand Down Expand Up @@ -245,5 +259,6 @@ class Module {
module.exports = {
Module,
initImportMetaMap,
wrapToModuleMap
wrapToModuleMap,
vmModuleLinkHookMap,
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ export function dynamicInstantiate(url) {
};
}

export function resolve(specifier, base, defaultResolver) {
export function resolve(specifier, base, { defaultResolve }) {
if (builtins.has(specifier)) {
return {
url: `node:${specifier}`,
format: 'dynamic'
};
}
return defaultResolver(specifier, base);
return defaultResolve(specifier, base);
}
2 changes: 1 addition & 1 deletion test/fixtures/es-module-loaders/loader-shared-dep.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import dep from './loader-dep.js';
import assert from 'assert';

export function resolve(specifier, base, defaultResolve) {
export function resolve(specifier, base, { defaultResolve }) {
assert.strictEqual(dep.format, 'esm');
return defaultResolve(specifier, base);
}
2 changes: 1 addition & 1 deletion test/fixtures/es-module-loaders/loader-with-dep.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import dep from './loader-dep.js';
export function resolve (specifier, base, defaultResolve) {
export function resolve (specifier, base, { defaultResolve }) {
return {
url: defaultResolve(specifier, base).url,
format: dep.format
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import assert from 'assert';
// a loader that asserts that the defaultResolve will throw "not found"
// (skipping the top-level main of course)
let mainLoad = true;
export async function resolve (specifier, base, defaultResolve) {
export async function resolve (specifier, base, { defaultResolve }) {
if (mainLoad) {
mainLoad = false;
return defaultResolve(specifier, base);
Expand Down