Skip to content
Open
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
35 changes: 26 additions & 9 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,11 @@ function defineImportAssertionAlias(context) {
* Calling the asynchronous `load` hook asynchronously.
* @property {(url: string, context: object, defaultLoad: Function) => LoadResult} [loadSync]
* Calling the asynchronous `load` hook synchronously.
* @property {(originalSpecifier: string, parentURL: string,
* importAttributes: Record<string, string>) => Promise<ResolveResult>} resolve
* @property {(originalSpecifier: string, parentURL: string, importAttributes:
* Record<string, string>, conditions?: string[]) => Promise<ResolveResult>} resolve
* Calling the asynchronous `resolve` hook asynchronously.
* @property {(originalSpecifier: string, parentURL: string,
* importAttributes: Record<string, string>) => ResolveResult} [resolveSync]
* @property {(originalSpecifier: string, parentURL: string, importAttributes:
* Record<string, string>, conditions?: string[]) => ResolveResult} [resolveSync]
* Calling the asynchronous `resolve` hook synchronously.
* @property {(specifier: string, parentURL: string) => any} register Register asynchronous loader hooks
* @property {() => void} waitForLoaderHookInitialization Force loading of hooks.
Expand Down Expand Up @@ -219,18 +219,20 @@ class AsyncLoaderHooksOnLoaderHookWorker {
* @param {string} [parentURL] The URL path of the module's parent.
* @param {ImportAttributes} [importAttributes] Attributes from the import
* statement or expression.
* @param {string[]} [conditions] Package export conditions for this request.
* @returns {Promise<{ format: string, url: URL['href'] }>}
*/
async resolve(
originalSpecifier,
parentURL,
importAttributes = { __proto__: null },
conditions = getDefaultConditions(),
) {
throwIfInvalidParentURL(parentURL);

const chain = this.#chains.resolve;
const context = {
conditions: getDefaultConditions(),
conditions,
importAttributes,
parentURL,
};
Expand Down Expand Up @@ -834,15 +836,30 @@ class AsyncLoaderHooksProxiedToLoaderHookWorker {
* @param {string} [parentURL] The URL path of the module's parent.
* @param {ImportAttributes} importAttributes Attributes from the import
* statement or expression.
* @param {string[]} [conditions] Package export conditions for this request.
* @returns {{ format: string, url: URL['href'] }}
*/
resolve(originalSpecifier, parentURL, importAttributes) {
return asyncLoaderHookWorker.makeAsyncRequest('resolve', undefined, originalSpecifier, parentURL, importAttributes);
resolve(originalSpecifier, parentURL, importAttributes, conditions) {
return asyncLoaderHookWorker.makeAsyncRequest(
'resolve',
undefined,
originalSpecifier,
parentURL,
importAttributes,
conditions,
);
}

resolveSync(originalSpecifier, parentURL, importAttributes) {
resolveSync(originalSpecifier, parentURL, importAttributes, conditions) {
// This happens only as a result of `import.meta.resolve` calls, which must be sync per spec.
return asyncLoaderHookWorker.makeSyncRequest('resolve', undefined, originalSpecifier, parentURL, importAttributes);
return asyncLoaderHookWorker.makeSyncRequest(
'resolve',
undefined,
originalSpecifier,
parentURL,
importAttributes,
conditions,
);
}

/**
Expand Down
26 changes: 20 additions & 6 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const {
setImportMetaResolveInitializer,
} = internalBinding('module_wrap');
const {
getCjsConditionsArray,
urlToFilename,
} = require('internal/modules/helpers');
const {
Expand Down Expand Up @@ -600,7 +601,9 @@ class ModuleLoader {
let maybePromise;
if (requestType === kRequireInImportedCJS || requestType === kImportInRequiredESM) {
// In these two cases, resolution must be synchronous.
maybePromise = this.resolveSync(parentURL, request);
const conditions = requestType === kRequireInImportedCJS ?
getCjsConditionsArray() : this.#defaultConditions;
maybePromise = this.resolveSync(parentURL, request, false, conditions);
assert(!isPromise(maybePromise));
} else {
maybePromise = this.#resolve(parentURL, request);
Expand Down Expand Up @@ -680,7 +683,7 @@ class ModuleLoader {
const specifier = `${request.specifier}`;
const importAttributes = request.attributes ?? kEmptyObject;
// TODO(joyeecheung): invoke the synchronous hooks in the default step on loader thread.
return this.#asyncLoaderHooks.resolve(specifier, parentURL, importAttributes);
return this.#asyncLoaderHooks.resolve(specifier, parentURL, importAttributes, request.conditions);
}

return this.resolveSync(parentURL, request);
Expand Down Expand Up @@ -718,7 +721,12 @@ class ModuleLoader {
*/
#resolveAndMaybeBlockOnLoaderThread(out, specifier, context) {
if (this.#asyncLoaderHooks?.resolveSync) {
return this.#asyncLoaderHooks.resolveSync(specifier, context.parentURL, context.importAttributes);
return this.#asyncLoaderHooks.resolveSync(
specifier,
context.parentURL,
context.importAttributes,
context.conditions,
);
}
out.isResolvedByDefaultResolve = true;
return this.#cachedDefaultResolve(specifier, context);
Expand All @@ -735,9 +743,15 @@ class ModuleLoader {
* @param {boolean} [shouldSkipSyncHooks] Whether to skip the synchronous hooks registered by module.registerHooks().
* This is used to maintain compatibility for the re-invented require.resolve (in imported CJS customized
* by module.register()`) which invokes the CJS resolution separately from the hook chain.
* @param {string[]} [conditions] Package export conditions for this request.
* @returns {ResolveResult}
*/
resolveSync(parentURL, request, shouldSkipSyncHooks = false) {
resolveSync(
parentURL,
request,
shouldSkipSyncHooks = false,
conditions = request.conditions ?? this.#defaultConditions,
) {
const specifier = `${request.specifier}`;
const importAttributes = request.attributes ?? kEmptyObject;
// Use an output parameter to track the state and avoid polluting the user-visible resolve results.
Expand All @@ -747,14 +761,14 @@ class ModuleLoader {
let isResolvedBySyncHooks = false;
if (!shouldSkipSyncHooks && syncResolveHooks.length) {
// Has module.registerHooks() hooks, chain the asynchronous hooks in the default step.
result = resolveWithSyncHooks(specifier, parentURL, importAttributes, this.#defaultConditions,
result = resolveWithSyncHooks(specifier, parentURL, importAttributes, conditions,
this.#resolveAndMaybeBlockOnLoaderThread.bind(this, out));
// If the default step ran, sync hooks did not short-circuit the resolution.
isResolvedBySyncHooks = !out.isResolvedByDefaultResolve;
} else {
const context = {
...request,
conditions: this.#defaultConditions,
conditions,
parentURL,
importAttributes,
__proto__: null,
Expand Down
8 changes: 7 additions & 1 deletion lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const assert = require('internal/assert');
const { dirname, extname } = require('path');
const {
assertBufferSource,
getCjsConditionsArray,
loadBuiltinModule,
stringify,
stripBOM,
Expand Down Expand Up @@ -178,7 +179,12 @@ function loadCJSModuleWithSpecialRequire(module, source, url, filename, isMain,
}
}

const request = { specifier, __proto__: null, attributes: kEmptyObject };
const request = {
specifier,
__proto__: null,
attributes: kEmptyObject,
conditions: getCjsConditionsArray(),
};
// Skip sync hooks in resolveSync since resolveForCJSWithHooks already ran them above.
const { url: resolvedURL } = cascadedLoader.resolveSync(url, request, /* shouldSkipSyncHooks */ true);
return urlToFilename(resolvedURL);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import '../common/index.mjs';
import assert from 'node:assert';
import { register } from 'node:module';

const loader = `
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you put this in the fixtures folder instead of making it inline?

Also please add a test for module.registerHooks - module.register has been deprecated and will be removed soon.

import assert from 'node:assert';

function assertCjsConditions(context) {
assert.ok(
context.conditions.includes('require'),
\`Conditions should include "require": \${JSON.stringify(context.conditions)}\`,
);
assert.ok(
!context.conditions.includes('import'),
\`Conditions should not include "import": \${JSON.stringify(context.conditions)}\`,
);
}

export async function resolve(specifier, context, nextResolve) {
if (specifier === 'custom:cjs') {
return { url: 'custom:cjs', format: 'commonjs', shortCircuit: true };
}

if (specifier === 'node:target') {
assertCjsConditions(context);
return { url: 'node:fs', shortCircuit: true };
}

return nextResolve(specifier, context);
}

export async function load(url, context, nextLoad) {
if (url === 'custom:cjs') {
return {
format: 'commonjs',
source: \`
module.exports = require.resolve('node:target');
\`,
shortCircuit: true,
};
}

return nextLoad(url, context);
}
`;

register(`data:text/javascript,${encodeURIComponent(loader)}`);

const ns = await import('custom:cjs');
assert.strictEqual(ns.default, 'node:fs');
Loading