diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 80d6af29871..3b16099848a 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -51,6 +51,7 @@ const { ReflectSet, RegExpPrototypeExec, SafeMap, + SafeSet, String, StringPrototypeCharAt, StringPrototypeCharCodeAt, @@ -154,6 +155,7 @@ const internalFsBinding = internalBinding('fs'); const { safeGetenv } = internalBinding('credentials'); const { getCjsConditions, + getCjsConditionsArray, initializeCjsConditions, loadBuiltinModule, makeRequireFunction, @@ -652,7 +654,7 @@ const EXPORTS_PATTERN = /^((?:@[^/\\%]+\/)?[^./\\%][^/\\%]*)(\/.*)?$/; * Resolves the exports for a given module path and request. * @param {string} nmPath The path to the module. * @param {string} request The request for the module. - * @param {unknown} conditions + * @param {Set} conditions The conditions to use for resolution. * @returns {undefined|string} */ function resolveExports(nmPath, request, conditions) { @@ -1067,9 +1069,22 @@ function resolveForCJSWithHooks(specifier, parent, isMain) { function defaultResolve(specifier, context) { // TODO(joyeecheung): parent and isMain should be part of context, then we // no longer need to use a different defaultResolve for every resolution. + // In the hooks, context.conditions is passed around as an array, but internally + // the resolution helpers expect a SafeSet. Do the conversion here. + let conditionSet; + const conditions = context.conditions; + if (conditions !== undefined && conditions !== getCjsConditionsArray()) { + if (!ArrayIsArray(conditions)) { + throw new ERR_INVALID_ARG_VALUE('context.conditions', conditions, + 'expected an array'); + } + conditionSet = new SafeSet(conditions); + } else { + conditionSet = getCjsConditions(); + } defaultResolvedFilename = defaultResolveImpl(specifier, parent, isMain, { __proto__: null, - conditions: context.conditions, + conditions: conditionSet, }); defaultResolvedURL = convertCJSFilenameToURL(defaultResolvedFilename); @@ -1077,7 +1092,7 @@ function resolveForCJSWithHooks(specifier, parent, isMain) { } const resolveResult = resolveWithHooks(specifier, parentURL, /* importAttributes */ undefined, - getCjsConditions(), defaultResolve); + getCjsConditionsArray(), defaultResolve); const { url } = resolveResult; format = resolveResult.format; @@ -1153,7 +1168,7 @@ function loadBuiltinWithHooks(id, url, format) { url ??= `node:${id}`; // TODO(joyeecheung): do we really want to invoke the load hook for the builtins? const loadResult = loadWithHooks(url, format || 'builtin', /* importAttributes */ undefined, - getCjsConditions(), getDefaultLoad(url, id)); + getCjsConditionsArray(), getDefaultLoad(url, id)); if (loadResult.format && loadResult.format !== 'builtin') { return undefined; // Format has been overridden, return undefined for the caller to continue loading. } @@ -1305,7 +1320,7 @@ Module._load = function(request, parent, isMain) { * @param {ResolveFilenameOptions} options Options object * @typedef {object} ResolveFilenameOptions * @property {string[]} paths Paths to search for modules in - * @property {string[]} conditions Conditions used for resolution. + * @property {Set?} conditions The conditions to use for resolution. * @returns {void|string} */ Module._resolveFilename = function(request, parent, isMain, options) { @@ -1754,7 +1769,8 @@ function loadSource(mod, filename, formatFromNode) { mod[kURL] = convertCJSFilenameToURL(filename); } - const loadResult = loadWithHooks(mod[kURL], mod[kFormat], /* importAttributes */ undefined, getCjsConditions(), + const loadResult = loadWithHooks(mod[kURL], mod[kFormat], /* importAttributes */ undefined, + getCjsConditionsArray(), getDefaultLoad(mod[kURL], filename)); // Reset the module properties with load hook results. diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 19904ec4b36..bc36fa2e26b 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -709,24 +709,30 @@ class ModuleLoader { if (this.#customizations) { // Only has module.register hooks. return this.#customizations.resolve(specifier, parentURL, importAttributes); } - return this.#cachedDefaultResolve(specifier, parentURL, importAttributes); + return this.#cachedDefaultResolve(specifier, { + __proto__: null, + conditions: this.#defaultConditions, + parentURL, + importAttributes, + }); } /** * Either return a cached resolution, or perform the default resolution which is synchronous, and * cache the result. * @param {string} specifier See {@link resolve}. - * @param {string} [parentURL] See {@link resolve}. - * @param {ImportAttributes} importAttributes See {@link resolve}. + * @param {{ parentURL?: string, importAttributes: ImportAttributes, conditions?: string[]}} context * @returns {{ format: string, url: string }} */ - #cachedDefaultResolve(specifier, parentURL, importAttributes) { + #cachedDefaultResolve(specifier, context) { + const { parentURL, importAttributes } = context; const requestKey = this.#resolveCache.serializeKey(specifier, importAttributes); const cachedResult = this.#resolveCache.get(requestKey, parentURL); if (cachedResult != null) { return cachedResult; } - const result = this.defaultResolve(specifier, parentURL, importAttributes); + defaultResolve ??= require('internal/modules/esm/resolve').defaultResolve; + const result = defaultResolve(specifier, context); this.#resolveCache.set(requestKey, parentURL, result); return result; } @@ -754,14 +760,15 @@ class ModuleLoader { * This is the default resolve step for module.registerHooks(), which incorporates asynchronous hooks * from module.register() which are run in a blocking fashion for it to be synchronous. * @param {string|URL} specifier See {@link resolveSync}. - * @param {{ parentURL?: string, importAttributes: ImportAttributes}} context See {@link resolveSync}. + * @param {{ parentURL?: string, importAttributes: ImportAttributes, conditions?: string[]}} context + * See {@link resolveSync}. * @returns {{ format: string, url: string }} */ #resolveAndMaybeBlockOnLoaderThread(specifier, context) { if (this.#customizations) { return this.#customizations.resolveSync(specifier, context.parentURL, context.importAttributes); } - return this.#cachedDefaultResolve(specifier, context.parentURL, context.importAttributes); + return this.#cachedDefaultResolve(specifier, context); } /** @@ -784,26 +791,12 @@ class ModuleLoader { return resolveWithHooks(specifier, parentURL, importAttributes, this.#defaultConditions, this.#resolveAndMaybeBlockOnLoaderThread.bind(this)); } - return this.#resolveAndMaybeBlockOnLoaderThread(specifier, { parentURL, importAttributes }); - } - - /** - * Our `defaultResolve` is synchronous and can be used in both - * `resolve` and `resolveSync`. This function is here just to avoid - * repeating the same code block twice in those functions. - * @returns {string} - */ - defaultResolve(originalSpecifier, parentURL, importAttributes) { - defaultResolve ??= require('internal/modules/esm/resolve').defaultResolve; - - const context = { + return this.#resolveAndMaybeBlockOnLoaderThread(specifier, { __proto__: null, conditions: this.#defaultConditions, - importAttributes, parentURL, - }; - - return defaultResolve(originalSpecifier, context); + importAttributes, + }); } /** diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index 17a1c3d1f01..9a47dc92d9c 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -66,6 +66,9 @@ function toRealPath(requestPath) { /** @type {Set} */ let cjsConditions; +/** @type {string[]} */ +let cjsConditionsArray; + /** * Define the conditions that apply to the CommonJS loader. * @returns {void} @@ -75,15 +78,17 @@ function initializeCjsConditions() { const noAddons = getOptionValue('--no-addons'); const addonConditions = noAddons ? [] : ['node-addons']; // TODO: Use this set when resolving pkg#exports conditions in loader.js. - cjsConditions = new SafeSet([ + cjsConditionsArray = [ 'require', 'node', ...addonConditions, ...userConditions, - ]); + ]; if (getOptionValue('--experimental-require-module')) { - cjsConditions.add('module-sync'); + cjsConditionsArray.push('module-sync'); } + ObjectFreeze(cjsConditionsArray); + cjsConditions = new SafeSet(cjsConditionsArray); } /** @@ -97,6 +102,13 @@ function getCjsConditions() { return cjsConditions; } +function getCjsConditionsArray() { + if (cjsConditionsArray === undefined) { + initializeCjsConditions(); + } + return cjsConditionsArray; +} + /** * Provide one of Node.js' public modules to user code. * @param {string} id - The identifier/specifier of the builtin module to load @@ -418,6 +430,7 @@ module.exports = { flushCompileCache, getBuiltinModule, getCjsConditions, + getCjsConditionsArray, getCompileCacheDir, initializeCjsConditions, loadBuiltinModule, diff --git a/test/fixtures/es-modules/custom-condition/node_modules/foo/default.cjs b/test/fixtures/es-modules/custom-condition/node_modules/foo/default.cjs new file mode 100644 index 00000000000..fd595339e01 --- /dev/null +++ b/test/fixtures/es-modules/custom-condition/node_modules/foo/default.cjs @@ -0,0 +1,2 @@ +exports.result = 'default'; + diff --git a/test/fixtures/es-modules/custom-condition/node_modules/foo/foo-esm.mjs b/test/fixtures/es-modules/custom-condition/node_modules/foo/foo-esm.mjs new file mode 100644 index 00000000000..87499b2e3f3 --- /dev/null +++ b/test/fixtures/es-modules/custom-condition/node_modules/foo/foo-esm.mjs @@ -0,0 +1,2 @@ +export const result = 'foo-esm'; + diff --git a/test/fixtures/es-modules/custom-condition/node_modules/foo/foo.cjs b/test/fixtures/es-modules/custom-condition/node_modules/foo/foo.cjs new file mode 100644 index 00000000000..a948034aa47 --- /dev/null +++ b/test/fixtures/es-modules/custom-condition/node_modules/foo/foo.cjs @@ -0,0 +1,2 @@ +exports.result = 'foo'; + diff --git a/test/fixtures/es-modules/custom-condition/node_modules/foo/package.json b/test/fixtures/es-modules/custom-condition/node_modules/foo/package.json new file mode 100644 index 00000000000..ee7eab9fc74 --- /dev/null +++ b/test/fixtures/es-modules/custom-condition/node_modules/foo/package.json @@ -0,0 +1,28 @@ +{ + "exports": { + ".": { + "foo": "./foo.cjs", + "foo-esm": "./foo-esm.mjs", + "default": "./default.cjs" + }, + "./second": { + "foo": "./foo.cjs", + "foo-esm": "./foo-esm.mjs", + "default": "./default.cjs" + }, + "./third": { + "foo": "./foo.cjs", + "foo-esm": "./foo-esm.mjs", + "default": "./default.cjs" + }, + "./fourth": { + "foo": "./foo.cjs", + "foo-esm": "./foo-esm.mjs", + "default": "./default.cjs" + }, + "./no-default": { + "foo": "./foo.cjs", + "foo-esm": "./foo-esm.mjs" + } + } +} \ No newline at end of file diff --git a/test/module-hooks/test-module-hooks-custom-conditions-cjs.js b/test/module-hooks/test-module-hooks-custom-conditions-cjs.js new file mode 100644 index 00000000000..4dcb9c3a20f --- /dev/null +++ b/test/module-hooks/test-module-hooks-custom-conditions-cjs.js @@ -0,0 +1,57 @@ +// Similar to test-module-hooks-custom-conditions.mjs, but checking the +// real require() instead of the re-invented one for imported CJS. +'use strict'; +const common = require('../common'); +const { registerHooks } = require('node:module'); +const assert = require('node:assert'); +const { cjs, esm } = require('../fixtures/es-modules/custom-condition/load.cjs'); + +(async () => { + // Without hooks, the default condition is used. + assert.strictEqual(cjs('foo').result, 'default'); + assert.strictEqual((await esm('foo')).result, 'default'); + + // Prepending 'foo' to the conditions array in the resolve hook should + // allow a CJS to be resolved with that condition. + { + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + assert(Array.isArray(context.conditions)); + context.conditions = ['foo', ...context.conditions]; + return nextResolve(specifier, context); + }, + }); + assert.strictEqual(cjs('foo/second').result, 'foo'); + assert.strictEqual((await esm('foo/second')).result, 'foo'); + hooks.deregister(); + } + + // Prepending 'foo-esm' to the conditions array in the resolve hook should + // allow a ESM to be resolved with that condition. + { + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + assert(Array.isArray(context.conditions)); + context.conditions = ['foo-esm', ...context.conditions]; + return nextResolve(specifier, context); + }, + }); + assert.strictEqual(cjs('foo/third').result, 'foo-esm'); + assert.strictEqual((await esm('foo/third')).result, 'foo-esm'); + hooks.deregister(); + } + + // Duplicating the 'foo' condition in the resolve hook should not change the result. + { + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + assert(Array.isArray(context.conditions)); + context.conditions = ['foo', ...context.conditions, 'foo']; + return nextResolve(specifier, context); + }, + }); + assert.strictEqual(cjs('foo/fourth').result, 'foo'); + assert.strictEqual((await esm('foo/fourth')).result, 'foo'); + hooks.deregister(); + } +})().then(common.mustCall()); diff --git a/test/module-hooks/test-module-hooks-custom-conditions-special-values.js b/test/module-hooks/test-module-hooks-custom-conditions-special-values.js new file mode 100644 index 00000000000..9a84c788c77 --- /dev/null +++ b/test/module-hooks/test-module-hooks-custom-conditions-special-values.js @@ -0,0 +1,70 @@ +// Check various special values of `conditions` in the context object +// when using synchronous module hooks to override the loaders in a +// CJS module. +'use strict'; +const common = require('../common'); +const { registerHooks } = require('node:module'); +const assert = require('node:assert'); +const { cjs, esm } = require('../fixtures/es-modules/custom-condition/load.cjs'); + +(async () => { + // Setting it to undefined would lead to the default conditions being used. + { + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + context.conditions = undefined; + return nextResolve(specifier, context); + }, + }); + assert.strictEqual(cjs('foo').result, 'default'); + assert.strictEqual((await esm('foo')).result, 'default'); + hooks.deregister(); + } + + // Setting it to an empty array would lead to the default conditions being used. + { + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + context.conditions = []; + return nextResolve(specifier, context); + }, + }); + assert.strictEqual(cjs('foo/second').result, 'default'); + assert.strictEqual((await esm('foo/second')).result, 'default'); + hooks.deregister(); + } + + // If the exports have no default export, it should error. + { + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + context.conditions = []; + return nextResolve(specifier, context); + }, + }); + assert.throws(() => cjs('foo/no-default'), { + code: 'ERR_PACKAGE_PATH_NOT_EXPORTED', + }); + await assert.rejects(esm('foo/no-default'), { + code: 'ERR_PACKAGE_PATH_NOT_EXPORTED', + }); + hooks.deregister(); + } + + // If the exports have no default export, it should error. + { + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + context.conditions = 'invalid'; + return nextResolve(specifier, context); + }, + }); + assert.throws(() => cjs('foo/third'), { + code: 'ERR_INVALID_ARG_VALUE', + }); + await assert.rejects(esm('foo/third'), { + code: 'ERR_INVALID_ARG_VALUE', + }); + hooks.deregister(); + } +})().then(common.mustCall()); diff --git a/test/module-hooks/test-module-hooks-custom-conditions.mjs b/test/module-hooks/test-module-hooks-custom-conditions.mjs new file mode 100644 index 00000000000..ef2022cf190 --- /dev/null +++ b/test/module-hooks/test-module-hooks-custom-conditions.mjs @@ -0,0 +1,53 @@ +// This tests that custom conditions can be used in module resolution hooks. +import '../common/index.mjs'; +import { registerHooks } from 'node:module'; +import assert from 'node:assert'; +import { cjs, esm } from '../fixtures/es-modules/custom-condition/load.cjs'; + +// Without hooks, the default condition is used. +assert.strictEqual(cjs('foo').result, 'default'); +assert.strictEqual((await esm('foo')).result, 'default'); + +// Prepending 'foo' to the conditions array in the resolve hook should +// allow a CJS to be resolved with that condition. +{ + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + assert(Array.isArray(context.conditions)); + context.conditions = ['foo', ...context.conditions]; + return nextResolve(specifier, context); + }, + }); + assert.strictEqual(cjs('foo/second').result, 'foo'); + assert.strictEqual((await esm('foo/second')).result, 'foo'); + hooks.deregister(); +} + +// Prepending 'foo-esm' to the conditions array in the resolve hook should +// allow a ESM to be resolved with that condition. +{ + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + assert(Array.isArray(context.conditions)); + context.conditions = ['foo-esm', ...context.conditions]; + return nextResolve(specifier, context); + }, + }); + assert.strictEqual(cjs('foo/third').result, 'foo-esm'); + assert.strictEqual((await esm('foo/third')).result, 'foo-esm'); + hooks.deregister(); +} + +// Duplicating the 'foo' condition in the resolve hook should not change the result. +{ + const hooks = registerHooks({ + resolve(specifier, context, nextResolve) { + assert(Array.isArray(context.conditions)); + context.conditions = ['foo', ...context.conditions, 'foo']; + return nextResolve(specifier, context); + }, + }); + assert.strictEqual(cjs('foo/fourth').result, 'foo'); + assert.strictEqual((await esm('foo/fourth')).result, 'foo'); + hooks.deregister(); +}