Skip to content

Commit

Permalink
esm: unflag Module.register and allow nested loader
Browse files Browse the repository at this point in the history
Major functional changes:

- Allow `import()` to work within loaders that require other loaders,
- Unflag the use of `Module.register`.

A new interface `Customizations` has been created in order to unify
`ModuleLoader` (previously `DefaultModuleLoader`), `Hooks` and
`CustomizedModuleLoader` all of which now implement it:

```ts
interface LoadResult {
  format: ModuleFormat;
  source: ModuleSource;
}

interface ResolveResult {
  format: string;
  url: URL['href'];
}

interface Customizations {
  allowImportMetaResolve: boolean;
  load(url: string, context: object): Promise<LoadResult>
  resolve(
    originalSpecifier:
    string, parentURL: string,
    importAssertions: Record<string, string>
  ): Promise<ResolveResult>
  resolveSync(
    originalSpecifier:
    string, parentURL: string,
    importAssertions: Record<string, string>
  ) ResolveResult;
  register(specifier: string, parentUrl: string): any;
  forceLoadHooks(): void;
  importMetaInitialize(meta, context, loader): void;
}
```

The `ModuleLoader` class now has `setCustomizations` which takes an
object of this shape and delegates its responsibilities to this object
if present.

Note that two properties `allowImportMetaResolve` and `resolveSync`
exist now as a mechanism for `import.meta.resolve` – since `Hooks`
does not implement `resolveSync` other loaders cannot use
`import.meta.resolve`; `allowImportMetaResolve` is a way of checking
for that case instead of invoking `resolveSync` and erroring.

Fixes nodejs/node#48515
Closes nodejs/node#48439

PR-URL: nodejs/node#48559
Backport-PR-URL: nodejs/node#50669
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
  • Loading branch information
sercher committed Apr 25, 2024
1 parent 29f9d1c commit c9eb494
Show file tree
Hide file tree
Showing 11 changed files with 265 additions and 157 deletions.
17 changes: 0 additions & 17 deletions graal-nodejs/doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1234,23 +1234,6 @@ provided.
Encoding provided to `TextDecoder()` API was not one of the
[WHATWG Supported Encodings][].

<a id="ERR_ESM_LOADER_REGISTRATION_UNAVAILABLE"></a>

### `ERR_ESM_LOADER_REGISTRATION_UNAVAILABLE`

<!-- YAML
added: REPLACEME
-->

Programmatically registering custom ESM loaders
currently requires at least one custom loader to have been
registered via the `--experimental-loader` flag. A no-op
loader registered via CLI is sufficient
(for example: `--experimental-loader data:text/javascript,`;
do not omit the necessary trailing comma).
A future version of Node.js will support the programmatic
registration of loaders without needing to also use the flag.

<a id="ERR_EVAL_ESM_CANNOT_PRINT"></a>

### `ERR_EVAL_ESM_CANNOT_PRINT`
Expand Down
5 changes: 0 additions & 5 deletions graal-nodejs/lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1039,11 +1039,6 @@ E('ERR_ENCODING_INVALID_ENCODED_DATA', function(encoding, ret) {
}, TypeError);
E('ERR_ENCODING_NOT_SUPPORTED', 'The "%s" encoding is not supported',
RangeError);
E('ERR_ESM_LOADER_REGISTRATION_UNAVAILABLE', 'Programmatically registering custom ESM loaders ' +
'currently requires at least one custom loader to have been registered via the --experimental-loader ' +
'flag. A no-op loader registered via CLI is sufficient (for example: `--experimental-loader ' +
'"data:text/javascript,"` with the necessary trailing comma). A future version of Node.js ' +
'will remove this requirement.', Error);
E('ERR_EVAL_ESM_CANNOT_PRINT', '--print cannot be used with ESM input', Error);
E('ERR_EVENT_RECURSION', 'The event "%s" is already being dispatched', Error);
E('ERR_FALSY_VALUE_REJECTION', function(reason) {
Expand Down
62 changes: 34 additions & 28 deletions graal-nodejs/lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const {
ERR_INVALID_RETURN_PROPERTY_VALUE,
ERR_INVALID_RETURN_VALUE,
ERR_LOADER_CHAIN_INCOMPLETE,
ERR_METHOD_NOT_IMPLEMENTED,
ERR_UNKNOWN_BUILTIN_MODULE,
ERR_WORKER_UNSERIALIZABLE_ERROR,
} = require('internal/errors').codes;
Expand Down Expand Up @@ -64,7 +65,7 @@ const {
let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
debug = fn;
});

let importMetaInitializer;

/**
* @typedef {object} ExportedHooks
Expand All @@ -81,7 +82,6 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {

// [2] `validate...()`s throw the wrong error


class Hooks {
#chains = {
/**
Expand Down Expand Up @@ -120,20 +120,20 @@ class Hooks {
// Cache URLs we've already validated to avoid repeated validation
#validatedUrls = new SafeSet();

allowImportMetaResolve = false;

/**
* Import and register custom/user-defined module loader hook(s).
* @param {string} urlOrSpecifier
* @param {string} parentURL
*/
async register(urlOrSpecifier, parentURL) {
const moduleLoader = require('internal/process/esm_loader').esmLoader;

const keyedExports = await moduleLoader.import(
urlOrSpecifier,
parentURL,
kEmptyObject,
);

this.addCustomLoader(urlOrSpecifier, keyedExports);
}

Expand All @@ -151,13 +151,15 @@ class Hooks {
} = pluckHooks(exports);

if (globalPreload) {
ArrayPrototypePush(this.#chains.globalPreload, { fn: globalPreload, url });
ArrayPrototypePush(this.#chains.globalPreload, { __proto__: null, fn: globalPreload, url });
}
if (resolve) {
ArrayPrototypePush(this.#chains.resolve, { fn: resolve, url });
const next = this.#chains.resolve[this.#chains.resolve.length - 1];
ArrayPrototypePush(this.#chains.resolve, { __proto__: null, fn: resolve, url, next });
}
if (load) {
ArrayPrototypePush(this.#chains.load, { fn: load, url });
const next = this.#chains.load[this.#chains.load.length - 1];
ArrayPrototypePush(this.#chains.load, { __proto__: null, fn: load, url, next });
}
}

Expand Down Expand Up @@ -234,7 +236,6 @@ class Hooks {
chainFinished: null,
context,
hookErrIdentifier: '',
hookIndex: chain.length - 1,
hookName: 'resolve',
shortCircuited: false,
};
Expand All @@ -257,7 +258,7 @@ class Hooks {
}
};

const nextResolve = nextHookFactory(chain, meta, { validateArgs, validateOutput });
const nextResolve = nextHookFactory(chain[chain.length - 1], meta, { validateArgs, validateOutput });

const resolution = await nextResolve(originalSpecifier, context);
const { hookErrIdentifier } = meta; // Retrieve the value after all settled
Expand Down Expand Up @@ -334,6 +335,10 @@ class Hooks {
};
}

resolveSync(_originalSpecifier, _parentURL, _importAssertions) {
throw new ERR_METHOD_NOT_IMPLEMENTED('resolveSync()');
}

/**
* Provide source that is understood by one of Node's translators.
*
Expand All @@ -350,7 +355,6 @@ class Hooks {
chainFinished: null,
context,
hookErrIdentifier: '',
hookIndex: chain.length - 1,
hookName: 'load',
shortCircuited: false,
};
Expand Down Expand Up @@ -392,7 +396,7 @@ class Hooks {
}
};

const nextLoad = nextHookFactory(chain, meta, { validateArgs, validateOutput });
const nextLoad = nextHookFactory(chain[chain.length - 1], meta, { validateArgs, validateOutput });

const loaded = await nextLoad(url, context);
const { hookErrIdentifier } = meta; // Retrieve the value after all settled
Expand Down Expand Up @@ -467,6 +471,16 @@ class Hooks {
source,
};
}

forceLoadHooks() {
// No-op
}

importMetaInitialize(meta, context, loader) {
importMetaInitializer ??= require('internal/modules/esm/initialize_import_meta').initializeImportMeta;
meta = importMetaInitializer(meta, context, loader);
return meta;
}
}
ObjectSetPrototypeOf(Hooks.prototype, null);

Expand Down Expand Up @@ -716,46 +730,39 @@ function pluckHooks({
* A utility function to iterate through a hook chain, track advancement in the
* chain, and generate and supply the `next<HookName>` argument to the custom
* hook.
* @param {KeyedHook[]} chain The whole hook chain.
* @param {Hook} current The (currently) first hook in the chain (this shifts
* on every call).
* @param {object} meta Properties that change as the current hook advances
* along the chain.
* @param {boolean} meta.chainFinished Whether the end of the chain has been
* reached AND invoked.
* @param {string} meta.hookErrIdentifier A user-facing identifier to help
* pinpoint where an error occurred. Ex "file:///foo.mjs 'resolve'".
* @param {number} meta.hookIndex A non-negative integer tracking the current
* position in the hook chain.
* @param {string} meta.hookName The kind of hook the chain is (ex 'resolve')
* @param {boolean} meta.shortCircuited Whether a hook signaled a short-circuit.
* @param {(hookErrIdentifier, hookArgs) => void} validate A wrapper function
* containing all validation of a custom loader hook's intermediary output. Any
* validation within MUST throw.
* @returns {function next<HookName>(...hookArgs)} The next hook in the chain.
*/
function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
function nextHookFactory(current, meta, { validateArgs, validateOutput }) {
// First, prepare the current
const { hookName } = meta;
const {
fn: hook,
url: hookFilePath,
} = chain[meta.hookIndex];
next,
} = current;

// ex 'nextResolve'
const nextHookName = `next${
StringPrototypeToUpperCase(hookName[0]) +
StringPrototypeSlice(hookName, 1)
}`;

// When hookIndex is 0, it's reached the default, which does not call next()
// so feed it a noop that blows up if called, so the problem is obvious.
const generatedHookIndex = meta.hookIndex;
let nextNextHook;
if (meta.hookIndex > 0) {
// Now, prepare the next: decrement the pointer so the next call to the
// factory generates the next link in the chain.
meta.hookIndex--;

nextNextHook = nextHookFactory(chain, meta, { validateArgs, validateOutput });
if (next) {
nextNextHook = nextHookFactory(next, meta, { validateArgs, validateOutput });
} else {
// eslint-disable-next-line func-name-matching
nextNextHook = function chainAdvancedTooFar() {
Expand All @@ -772,17 +779,16 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {

validateArgs(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, arg0, context);

const outputErrIdentifier = `${chain[generatedHookIndex].url} '${hookName}' hook's ${nextHookName}()`;
const outputErrIdentifier = `${hookFilePath} '${hookName}' hook's ${nextHookName}()`;

// Set when next<HookName> is actually called, not just generated.
if (generatedHookIndex === 0) { meta.chainFinished = true; }
if (!next) { meta.chainFinished = true; }

if (context) { // `context` has already been validated, so no fancy check needed.
ObjectAssign(meta.context, context);
}

const output = await hook(arg0, meta.context, nextNextHook);

validateOutput(outputErrIdentifier, output);

if (output?.shortCircuit === true) { meta.shortCircuited = true; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function createImportMetaResolve(defaultParentUrl, loader) {
let url;

try {
({ url } = loader.resolve(specifier, parentUrl));
({ url } = loader.resolveSync(specifier, parentUrl));
} catch (error) {
if (error?.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
({ url } = error);
Expand All @@ -38,7 +38,7 @@ function initializeImportMeta(meta, context, loader) {
const { url } = context;

// Alphabetical
if (experimentalImportMetaResolve && loader.loaderType !== 'internal') {
if (experimentalImportMetaResolve && loader.allowImportMetaResolve) {
meta.resolve = createImportMetaResolve(url, loader);
}

Expand Down
Loading

0 comments on commit c9eb494

Please sign in to comment.