GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — module: fix conditions override in synchronous resolve hooks · nodejs/node@fe0195f

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • lib/internal/modules
    • cjs
      • loader.js
    • esm
      • loader.js
    • helpers.js
  • test
    • fixtures/es-modules/custom-condition/node_modules/foo
      • default.cjs
      • foo-esm.mjs
      • foo.cjs
      • package.json
    • module-hooks
      • test-module-hooks-custom-conditions-cjs.js
      • test-module-hooks-custom-conditions-special-values.js
      • test-module-hooks-custom-conditions.mjs
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+22-6Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ const {
5151
ReflectSet,
5252
RegExpPrototypeExec,
5353
SafeMap,
54+
SafeSet,
5455
String,
5556
StringPrototypeCharAt,
5657
StringPrototypeCharCodeAt,
@@ -154,6 +155,7 @@ const internalFsBinding = internalBinding('fs');
154155
const { safeGetenv } = internalBinding('credentials');
155156
const {
156157
getCjsConditions,
158+
getCjsConditionsArray,
157159
initializeCjsConditions,
158160
loadBuiltinModule,
159161
makeRequireFunction,
@@ -653,7 +655,7 @@ const EXPORTS_PATTERN = /^((?:@[^/\\%]+\/)?[^./\\%][^/\\%]*)(\/.*)?$/;
653655
* Resolves the exports for a given module path and request.
654656
* @param {string} nmPath The path to the module.
655657
* @param {string} request The request for the module.
656-
* @param {unknown} conditions
658+
* @param {Set<string>} conditions The conditions to use for resolution.
657659
* @returns {undefined|string}
658660
*/
659661
function resolveExports(nmPath, request, conditions) {
@@ -1068,17 +1070,30 @@ function resolveForCJSWithHooks(specifier, parent, isMain) {
10681070
function defaultResolve(specifier, context) {
10691071
// TODO(joyeecheung): parent and isMain should be part of context, then we
10701072
// no longer need to use a different defaultResolve for every resolution.
1073+
// In the hooks, context.conditions is passed around as an array, but internally
1074+
// the resolution helpers expect a SafeSet. Do the conversion here.
1075+
let conditionSet;
1076+
const conditions = context.conditions;
1077+
if (conditions !== undefined && conditions !== getCjsConditionsArray()) {
1078+
if (!ArrayIsArray(conditions)) {
1079+
throw new ERR_INVALID_ARG_VALUE('context.conditions', conditions,
1080+
'expected an array');
1081+
}
1082+
conditionSet = new SafeSet(conditions);
1083+
} else {
1084+
conditionSet = getCjsConditions();
1085+
}
10711086
defaultResolvedFilename = defaultResolveImpl(specifier, parent, isMain, {
10721087
__proto__: null,
1073-
conditions: context.conditions,
1088+
conditions: conditionSet,
10741089
});
10751090

10761091
defaultResolvedURL = convertCJSFilenameToURL(defaultResolvedFilename);
10771092
return { __proto__: null, url: defaultResolvedURL };
10781093
}
10791094

10801095
const resolveResult = resolveWithHooks(specifier, parentURL, /* importAttributes */ undefined,
1081-
getCjsConditions(), defaultResolve);
1096+
getCjsConditionsArray(), defaultResolve);
10821097
const { url } = resolveResult;
10831098
format = resolveResult.format;
10841099

@@ -1154,7 +1169,7 @@ function loadBuiltinWithHooks(id, url, format) {
11541169
url ??= `node:${id}`;
11551170
// TODO(joyeecheung): do we really want to invoke the load hook for the builtins?
11561171
const loadResult = loadWithHooks(url, format || 'builtin', /* importAttributes */ undefined,
1157-
getCjsConditions(), getDefaultLoad(url, id));
1172+
getCjsConditionsArray(), getDefaultLoad(url, id));
11581173
if (loadResult.format && loadResult.format !== 'builtin') {
11591174
return undefined; // Format has been overridden, return undefined for the caller to continue loading.
11601175
}
@@ -1306,7 +1321,7 @@ Module._load = function(request, parent, isMain) {
13061321
* @param {ResolveFilenameOptions} options Options object
13071322
* @typedef {object} ResolveFilenameOptions
13081323
* @property {string[]} paths Paths to search for modules in
1309-
* @property {string[]} conditions Conditions used for resolution.
1324+
* @property {Set<string>?} conditions The conditions to use for resolution.
13101325
* @returns {void|string}
13111326
*/
13121327
Module._resolveFilename = function(request, parent, isMain, options) {
@@ -1755,7 +1770,8 @@ function loadSource(mod, filename, formatFromNode) {
17551770
mod[kURL] = convertCJSFilenameToURL(filename);
17561771
}
17571772

1758-
const loadResult = loadWithHooks(mod[kURL], mod[kFormat], /* importAttributes */ undefined, getCjsConditions(),
1773+
const loadResult = loadWithHooks(mod[kURL], mod[kFormat], /* importAttributes */ undefined,
1774+
getCjsConditionsArray(),
17591775
getDefaultLoad(mod[kURL], filename));
17601776

17611777
// Reset the module properties with load hook results.
Collapse file

‎lib/internal/modules/esm/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js
+17-24Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -709,24 +709,30 @@ class ModuleLoader {
709709
if (this.#customizations) { // Only has module.register hooks.
710710
return this.#customizations.resolve(specifier, parentURL, importAttributes);
711711
}
712-
return this.#cachedDefaultResolve(specifier, parentURL, importAttributes);
712+
return this.#cachedDefaultResolve(specifier, {
713+
__proto__: null,
714+
conditions: this.#defaultConditions,
715+
parentURL,
716+
importAttributes,
717+
});
713718
}
714719

715720
/**
716721
* Either return a cached resolution, or perform the default resolution which is synchronous, and
717722
* cache the result.
718723
* @param {string} specifier See {@link resolve}.
719-
* @param {string} [parentURL] See {@link resolve}.
720-
* @param {ImportAttributes} importAttributes See {@link resolve}.
724+
* @param {{ parentURL?: string, importAttributes: ImportAttributes, conditions?: string[]}} context
721725
* @returns {{ format: string, url: string }}
722726
*/
723-
#cachedDefaultResolve(specifier, parentURL, importAttributes) {
727+
#cachedDefaultResolve(specifier, context) {
728+
const { parentURL, importAttributes } = context;
724729
const requestKey = this.#resolveCache.serializeKey(specifier, importAttributes);
725730
const cachedResult = this.#resolveCache.get(requestKey, parentURL);
726731
if (cachedResult != null) {
727732
return cachedResult;
728733
}
729-
const result = this.defaultResolve(specifier, parentURL, importAttributes);
734+
defaultResolve ??= require('internal/modules/esm/resolve').defaultResolve;
735+
const result = defaultResolve(specifier, context);
730736
this.#resolveCache.set(requestKey, parentURL, result);
731737
return result;
732738
}
@@ -754,14 +760,15 @@ class ModuleLoader {
754760
* This is the default resolve step for module.registerHooks(), which incorporates asynchronous hooks
755761
* from module.register() which are run in a blocking fashion for it to be synchronous.
756762
* @param {string|URL} specifier See {@link resolveSync}.
757-
* @param {{ parentURL?: string, importAttributes: ImportAttributes}} context See {@link resolveSync}.
763+
* @param {{ parentURL?: string, importAttributes: ImportAttributes, conditions?: string[]}} context
764+
* See {@link resolveSync}.
758765
* @returns {{ format: string, url: string }}
759766
*/
760767
#resolveAndMaybeBlockOnLoaderThread(specifier, context) {
761768
if (this.#customizations) {
762769
return this.#customizations.resolveSync(specifier, context.parentURL, context.importAttributes);
763770
}
764-
return this.#cachedDefaultResolve(specifier, context.parentURL, context.importAttributes);
771+
return this.#cachedDefaultResolve(specifier, context);
765772
}
766773

767774
/**
@@ -784,26 +791,12 @@ class ModuleLoader {
784791
return resolveWithHooks(specifier, parentURL, importAttributes, this.#defaultConditions,
785792
this.#resolveAndMaybeBlockOnLoaderThread.bind(this));
786793
}
787-
return this.#resolveAndMaybeBlockOnLoaderThread(specifier, { parentURL, importAttributes });
788-
}
789-
790-
/**
791-
* Our `defaultResolve` is synchronous and can be used in both
792-
* `resolve` and `resolveSync`. This function is here just to avoid
793-
* repeating the same code block twice in those functions.
794-
* @returns {string}
795-
*/
796-
defaultResolve(originalSpecifier, parentURL, importAttributes) {
797-
defaultResolve ??= require('internal/modules/esm/resolve').defaultResolve;
798-
799-
const context = {
794+
return this.#resolveAndMaybeBlockOnLoaderThread(specifier, {
800795
__proto__: null,
801796
conditions: this.#defaultConditions,
802-
importAttributes,
803797
parentURL,
804-
};
805-
806-
return defaultResolve(originalSpecifier, context);
798+
importAttributes,
799+
});
807800
}
808801

809802
/**
Collapse file

‎lib/internal/modules/helpers.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/helpers.js
+16-3Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ function toRealPath(requestPath) {
6666

6767
/** @type {Set<string>} */
6868
let cjsConditions;
69+
/** @type {string[]} */
70+
let cjsConditionsArray;
71+
6972
/**
7073
* Define the conditions that apply to the CommonJS loader.
7174
* @returns {void}
@@ -75,15 +78,17 @@ function initializeCjsConditions() {
7578
const noAddons = getOptionValue('--no-addons');
7679
const addonConditions = noAddons ? [] : ['node-addons'];
7780
// TODO: Use this set when resolving pkg#exports conditions in loader.js.
78-
cjsConditions = new SafeSet([
81+
cjsConditionsArray = [
7982
'require',
8083
'node',
8184
...addonConditions,
8285
...userConditions,
83-
]);
86+
];
8487
if (getOptionValue('--experimental-require-module')) {
85-
cjsConditions.add('module-sync');
88+
cjsConditionsArray.push('module-sync');
8689
}
90+
ObjectFreeze(cjsConditionsArray);
91+
cjsConditions = new SafeSet(cjsConditionsArray);
8792
}
8893

8994
/**
@@ -97,6 +102,13 @@ function getCjsConditions() {
97102
return cjsConditions;
98103
}
99104

105+
function getCjsConditionsArray() {
106+
if (cjsConditionsArray === undefined) {
107+
initializeCjsConditions();
108+
}
109+
return cjsConditionsArray;
110+
}
111+
100112
/**
101113
* Provide one of Node.js' public modules to user code.
102114
* @param {string} id - The identifier/specifier of the builtin module to load
@@ -418,6 +430,7 @@ module.exports = {
418430
flushCompileCache,
419431
getBuiltinModule,
420432
getCjsConditions,
433+
getCjsConditionsArray,
421434
getCompileCacheDir,
422435
initializeCjsConditions,
423436
loadBuiltinModule,
Collapse file

‎test/fixtures/es-modules/custom-condition/node_modules/foo/default.cjs‎

Copy file name to clipboardExpand all lines: test/fixtures/es-modules/custom-condition/node_modules/foo/default.cjs
+2Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/es-modules/custom-condition/node_modules/foo/foo-esm.mjs‎

Copy file name to clipboardExpand all lines: test/fixtures/es-modules/custom-condition/node_modules/foo/foo-esm.mjs
+2Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/es-modules/custom-condition/node_modules/foo/foo.cjs‎

Copy file name to clipboardExpand all lines: test/fixtures/es-modules/custom-condition/node_modules/foo/foo.cjs
+2Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/es-modules/custom-condition/node_modules/foo/package.json‎

Copy file name to clipboardExpand all lines: test/fixtures/es-modules/custom-condition/node_modules/foo/package.json
+28Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/module-hooks/test-module-hooks-custom-conditions-cjs.js‎

Copy file name to clipboard
+57Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Similar to test-module-hooks-custom-conditions.mjs, but checking the
2+
// real require() instead of the re-invented one for imported CJS.
3+
'use strict';
4+
const common = require('../common');
5+
const { registerHooks } = require('node:module');
6+
const assert = require('node:assert');
7+
const { cjs, esm } = require('../fixtures/es-modules/custom-condition/load.cjs');
8+
9+
(async () => {
10+
// Without hooks, the default condition is used.
11+
assert.strictEqual(cjs('foo').result, 'default');
12+
assert.strictEqual((await esm('foo')).result, 'default');
13+
14+
// Prepending 'foo' to the conditions array in the resolve hook should
15+
// allow a CJS to be resolved with that condition.
16+
{
17+
const hooks = registerHooks({
18+
resolve(specifier, context, nextResolve) {
19+
assert(Array.isArray(context.conditions));
20+
context.conditions = ['foo', ...context.conditions];
21+
return nextResolve(specifier, context);
22+
},
23+
});
24+
assert.strictEqual(cjs('foo/second').result, 'foo');
25+
assert.strictEqual((await esm('foo/second')).result, 'foo');
26+
hooks.deregister();
27+
}
28+
29+
// Prepending 'foo-esm' to the conditions array in the resolve hook should
30+
// allow a ESM to be resolved with that condition.
31+
{
32+
const hooks = registerHooks({
33+
resolve(specifier, context, nextResolve) {
34+
assert(Array.isArray(context.conditions));
35+
context.conditions = ['foo-esm', ...context.conditions];
36+
return nextResolve(specifier, context);
37+
},
38+
});
39+
assert.strictEqual(cjs('foo/third').result, 'foo-esm');
40+
assert.strictEqual((await esm('foo/third')).result, 'foo-esm');
41+
hooks.deregister();
42+
}
43+
44+
// Duplicating the 'foo' condition in the resolve hook should not change the result.
45+
{
46+
const hooks = registerHooks({
47+
resolve(specifier, context, nextResolve) {
48+
assert(Array.isArray(context.conditions));
49+
context.conditions = ['foo', ...context.conditions, 'foo'];
50+
return nextResolve(specifier, context);
51+
},
52+
});
53+
assert.strictEqual(cjs('foo/fourth').result, 'foo');
54+
assert.strictEqual((await esm('foo/fourth')).result, 'foo');
55+
hooks.deregister();
56+
}
57+
})().then(common.mustCall());
Collapse file

‎test/module-hooks/test-module-hooks-custom-conditions-special-values.js‎

Copy file name to clipboard
+70Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Check various special values of `conditions` in the context object
2+
// when using synchronous module hooks to override the loaders in a
3+
// CJS module.
4+
'use strict';
5+
const common = require('../common');
6+
const { registerHooks } = require('node:module');
7+
const assert = require('node:assert');
8+
const { cjs, esm } = require('../fixtures/es-modules/custom-condition/load.cjs');
9+
10+
(async () => {
11+
// Setting it to undefined would lead to the default conditions being used.
12+
{
13+
const hooks = registerHooks({
14+
resolve(specifier, context, nextResolve) {
15+
context.conditions = undefined;
16+
return nextResolve(specifier, context);
17+
},
18+
});
19+
assert.strictEqual(cjs('foo').result, 'default');
20+
assert.strictEqual((await esm('foo')).result, 'default');
21+
hooks.deregister();
22+
}
23+
24+
// Setting it to an empty array would lead to the default conditions being used.
25+
{
26+
const hooks = registerHooks({
27+
resolve(specifier, context, nextResolve) {
28+
context.conditions = [];
29+
return nextResolve(specifier, context);
30+
},
31+
});
32+
assert.strictEqual(cjs('foo/second').result, 'default');
33+
assert.strictEqual((await esm('foo/second')).result, 'default');
34+
hooks.deregister();
35+
}
36+
37+
// If the exports have no default export, it should error.
38+
{
39+
const hooks = registerHooks({
40+
resolve(specifier, context, nextResolve) {
41+
context.conditions = [];
42+
return nextResolve(specifier, context);
43+
},
44+
});
45+
assert.throws(() => cjs('foo/no-default'), {
46+
code: 'ERR_PACKAGE_PATH_NOT_EXPORTED',
47+
});
48+
await assert.rejects(esm('foo/no-default'), {
49+
code: 'ERR_PACKAGE_PATH_NOT_EXPORTED',
50+
});
51+
hooks.deregister();
52+
}
53+
54+
// If the exports have no default export, it should error.
55+
{
56+
const hooks = registerHooks({
57+
resolve(specifier, context, nextResolve) {
58+
context.conditions = 'invalid';
59+
return nextResolve(specifier, context);
60+
},
61+
});
62+
assert.throws(() => cjs('foo/third'), {
63+
code: 'ERR_INVALID_ARG_VALUE',
64+
});
65+
await assert.rejects(esm('foo/third'), {
66+
code: 'ERR_INVALID_ARG_VALUE',
67+
});
68+
hooks.deregister();
69+
}
70+
})().then(common.mustCall());

0 commit comments

Comments
0 (0)
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.