Skip to content

Commit

Permalink
esm: refactor DefaultModuleLoader
Browse files Browse the repository at this point in the history
Fixes #48515
Fixes #48439
  • Loading branch information
izaakschroeder committed Jul 4, 2023
1 parent 951da52 commit 7170a68
Show file tree
Hide file tree
Showing 9 changed files with 237 additions and 170 deletions.
65 changes: 32 additions & 33 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {

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


class Hooks {
#chains = {
/**
Expand Down Expand Up @@ -126,16 +125,18 @@ class Hooks {
*/
async register(urlOrSpecifier, parentURL) {
const moduleLoader = require('internal/process/esm_loader').esmLoader;

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

this.addCustomLoader(urlOrSpecifier, keyedExports);
}

allowImportMetaResolve() {
return false;
}

/**
* Collect custom/user-defined module loader hook(s).
* After all hooks have been collected, the global preload hook(s) must be initialized.
Expand All @@ -150,13 +151,16 @@ class Hooks {
} = pluckHooks(exports);

if (globalPreload) {
ArrayPrototypePush(this.#chains.globalPreload, { fn: globalPreload, url });
const next = this.#chains.globalPreload[this.#chains.globalPreload.length - 1];
ArrayPrototypePush(this.#chains.globalPreload, { fn: globalPreload, url, next });
}
if (resolve) {
ArrayPrototypePush(this.#chains.resolve, { fn: resolve, url });
const next = this.#chains.resolve[this.#chains.resolve.length - 1];
ArrayPrototypePush(this.#chains.resolve, { 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, { fn: load, url, next });
}
}

Expand Down Expand Up @@ -233,7 +237,6 @@ class Hooks {
chainFinished: null,
context,
hookErrIdentifier: '',
hookIndex: chain.length - 1,
hookName: 'resolve',
shortCircuited: false,
};
Expand All @@ -256,7 +259,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 @@ -349,7 +352,6 @@ class Hooks {
chainFinished: null,
context,
hookErrIdentifier: '',
hookIndex: chain.length - 1,
hookName: 'load',
shortCircuited: false,
};
Expand Down Expand Up @@ -391,7 +393,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 @@ -528,7 +530,17 @@ class HooksProxy {
debug('wait for signal from worker');
AtomicsWait(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 0);
const response = this.#worker.receiveMessageSync();
if (response.message.status === 'exit') { return; }
if (response.message.status === 'exit') {
// TODO: I do not understand why this is necessary.
// node \
// --no-warnings --experimental-loader 'data:text/javascript,process.exit(42)'
// ./test/fixtures/empty.js
// Does not trigger `this.#worker.on('exit', process.exit);`.
// I think it is because `makeSyncRequest` keeps waiting to see another
// message and blocks the thread from ANY other activity including the exit.
process.exit(response.message.body);
return;
}
const { preloadScripts } = this.#unwrapMessage(response);
this.#executePreloadScripts(preloadScripts);
}
Expand Down Expand Up @@ -684,46 +696,34 @@ 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} hook The first hook in the chain.
* @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(hook, meta, { validateArgs, validateOutput }) {
// First, prepare the current
const { hookName } = meta;
const {
fn: hook,
url: hookFilePath,
} = chain[meta.hookIndex];
const { fn, url, next } = hook;

// 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 @@ -736,21 +736,20 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
return ObjectDefineProperty(
async (arg0 = undefined, context) => {
// Update only when hook is invoked to avoid fingering the wrong filePath
meta.hookErrIdentifier = `${hookFilePath} '${hookName}'`;
meta.hookErrIdentifier = `${url} '${hookName}'`;

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

const outputErrIdentifier = `${chain[generatedHookIndex].url} '${hookName}' hook's ${nextHookName}()`;
const outputErrIdentifier = `${url} '${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);

const output = await fn(arg0, meta.context, nextNextHook);
validateOutput(outputErrIdentifier, output);

if (output?.shortCircuit === true) { meta.shortCircuited = true; }
Expand Down
12 changes: 10 additions & 2 deletions lib/internal/modules/esm/initialize_import_meta.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
'use strict';

const { Symbol } = primordials;

const { getOptionValue } = require('internal/options');
const experimentalImportMetaResolve = getOptionValue('--experimental-import-meta-resolve');
const kResolveSync = Symbol('sync');

const importAssertions = {
[kResolveSync]: true,
};

/**
* Generate a function to be used as import.meta.resolve for a particular module.
Expand All @@ -14,7 +21,7 @@ function createImportMetaResolve(defaultParentUrl, loader) {
let url;

try {
({ url } = loader.resolve(specifier, parentUrl));
({ url } = loader.resolve(specifier, parentUrl, importAssertions));
} catch (error) {
if (error?.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
({ url } = error);
Expand All @@ -38,7 +45,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 All @@ -49,4 +56,5 @@ function initializeImportMeta(meta, context, loader) {

module.exports = {
initializeImportMeta,
kResolveSync,
};
Loading

0 comments on commit 7170a68

Please sign in to comment.