Skip to content

Feature: ESM configuration file #5353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Feature: ESM configuration file #5353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
639904b
feat(cli): make configuration loader asynchronous
ByteB4rb1e May 1, 2025
14ac77d
feat(cli): add ESM config loader
ByteB4rb1e May 1, 2025
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
208 changes: 106 additions & 102 deletions bin/mocha.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,119 +24,123 @@ const mochaArgs = {};
const nodeArgs = {};
let hasInspect = false;

const opts = loadOptions(process.argv.slice(2));
debug('loaded opts', opts);

/**
* Given option/command `value`, disable timeouts if applicable
* @param {string} [value] - Value to check
* @ignore
*/
const disableTimeouts = value => {
if (impliesNoTimeouts(value)) {
debug('option %s disabled timeouts', value);
mochaArgs.timeout = 0;
}
};

/**
* If `value` begins with `v8-` and is not explicitly `v8-options`, remove prefix
* @param {string} [value] - Value to check
* @returns {string} `value` with prefix (maybe) removed
* @ignore
*/
const trimV8Option = value =>
value !== 'v8-options' && /^v8-/.test(value) ? value.slice(3) : value;
async function main() {
Copy link
Member

Choose a reason for hiding this comment

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

main is part of Mocha's public API: https://mochajs.org/api/module-lib_cli#.main. Switching it to return a Promise would be a pretty major breaking change. We're really, really hesitant to tackle those.

OTOH, Node.js ^20.19.0 || >=22.12.0 supports require(ESM). So the code could still all be synchronous! I think it'd be fine and reasonable to tell users that ESM config files are only newly supported on Node.js versions with require(ESM). WDYT?

Aside: Node.js 18 just went EOL this week and it's looking likely that Mocha 12 will drop raise Mocha's minimum supported Node.js version to something at least as recent as 20.19.0. That means Mocha 12 might be able to fully rely on require(ESM). Very exciting.

Copy link
Author

Choose a reason for hiding this comment

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

main is part of Mocha's public API

I share your reservations... It seemed to me that somebody needed to get the conversation on the feature request going and yes, this is also a pretty painful PR for me... The diff on bin/mocha.js looks horrendous... If the PR leads to no resolution, this is fine with me as well.

Node.js ^20.19.0 || >=22.12.0 supports require(ESM). So the code could still all be synchronous!

Under the historic context of discussions around the Node.js ecosystem, I'm not quite sure where the compatibility layer between CommonJS and ESM in Node.js will be heading in the future... Migrating towards ESM, to me, seems all or nothing. Once migrated towards ESM, CommonJS backward-compatibility is straightforward, but the forward compatibility solution provided by Node.js, which inherently changes the behavior of require, seems like an unstable solution, catering towards CommonJS proponents. It's not ideal and also hinders the adoption of ECMAScript conventionality, IMO. Keeping in mind that mocha might find broader adoption with other runtimes, such as Deno, depending on a feature, which is essentially a workaround, might cause bigger issues in the future.

This is a very interesting read on the historic origins of this subject matter by one of the core contributors to Node.js.

...It would be far too disruptive a change to the ecosystem for us to modify the semantics of require() to allow it to do asynchronous loading...

Yet here we are...

https://github.com/nodejs/node/blob/f552c86fecd6c2ba9e832ea129b731dd63abdbe2/lib/internal/modules/esm/loader.js#L362

All in all, I'm not sold on the approach provided by Node.js. There are a few caveats, still some to-dos left - as far as I can tell from the commit history and comments. I'd rather encourage users to commit to ESM fully, though I can see the challenge in doing so as a popular project such as mocha.

I think it'd be fine and reasonable to tell users that ESM config files are only newly supported on Node.js versions with require(ESM). WDYT?

I agree, though I don't know your userbase, so can't really have an informed opinion on that. According to Repology, adoption of Node.js ver. >= 20 is good, though e.g. Ubuntu 24.04 LTS is still on v18 and some popular CI services use that Ubuntu version for their container images...

const opts = await loadOptions(process.argv.slice(2));
debug('loaded opts', opts);

/**
* Given option/command `value`, disable timeouts if applicable
* @param {string} [value] - Value to check
* @ignore
*/
const disableTimeouts = value => {
if (impliesNoTimeouts(value)) {
debug('option %s disabled timeouts', value);
mochaArgs.timeout = 0;
}
};

/**
* If `value` begins with `v8-` and is not explicitly `v8-options`, remove prefix
* @param {string} [value] - Value to check
* @returns {string} `value` with prefix (maybe) removed
* @ignore
*/
const trimV8Option = value =>
value !== 'v8-options' && /^v8-/.test(value) ? value.slice(3) : value;

// sort options into "node" and "mocha" buckets
Object.keys(opts).forEach(opt => {
if (isNodeFlag(opt)) {
nodeArgs[trimV8Option(opt)] = opts[opt];
} else {
mochaArgs[opt] = opts[opt];
}
});

// sort options into "node" and "mocha" buckets
Object.keys(opts).forEach(opt => {
if (isNodeFlag(opt)) {
nodeArgs[trimV8Option(opt)] = opts[opt];
} else {
mochaArgs[opt] = opts[opt];
}
});

// disable 'timeout' for debugFlags
Object.keys(nodeArgs).forEach(opt => disableTimeouts(opt));
mochaArgs['node-option'] &&
mochaArgs['node-option'].forEach(opt => disableTimeouts(opt));

// Native debugger handling
// see https://nodejs.org/api/debugger.html#debugger_debugger
// look for 'inspect' that would launch this debugger,
// remove it from Mocha's opts and prepend it to Node's opts.
// A deprecation warning will be printed by node, if applicable.
// (mochaArgs._ are "positional" arguments, not prefixed with - or --)
if (mochaArgs._) {
const i = mochaArgs._.findIndex(val => val === 'inspect');
if (i > -1) {
mochaArgs._.splice(i, 1);
disableTimeouts('inspect');
hasInspect = true;
// disable 'timeout' for debugFlags
Object.keys(nodeArgs).forEach(opt => disableTimeouts(opt));
mochaArgs['node-option'] &&
mochaArgs['node-option'].forEach(opt => disableTimeouts(opt));

// Native debugger handling
// see https://nodejs.org/api/debugger.html#debugger_debugger
// look for 'inspect' that would launch this debugger,
// remove it from Mocha's opts and prepend it to Node's opts.
// A deprecation warning will be printed by node, if applicable.
// (mochaArgs._ are "positional" arguments, not prefixed with - or --)
if (mochaArgs._) {
const i = mochaArgs._.findIndex(val => val === 'inspect');
if (i > -1) {
mochaArgs._.splice(i, 1);
disableTimeouts('inspect');
hasInspect = true;
}
}
}

if (mochaArgs['node-option'] || Object.keys(nodeArgs).length || hasInspect) {
const {spawn} = require('node:child_process');
const mochaPath = require.resolve('../lib/cli/cli.js');
if (mochaArgs['node-option'] || Object.keys(nodeArgs).length || hasInspect) {
const {spawn} = require('node:child_process');
const mochaPath = require.resolve('../lib/cli/cli.js');

const nodeArgv =
(mochaArgs['node-option'] && mochaArgs['node-option'].map(v => '--' + v)) ||
unparseNodeFlags(nodeArgs);
const nodeArgv =
(mochaArgs['node-option'] && mochaArgs['node-option'].map(v => '--' + v)) ||
unparseNodeFlags(nodeArgs);

if (hasInspect) nodeArgv.unshift('inspect');
delete mochaArgs['node-option'];
if (hasInspect) nodeArgv.unshift('inspect');
delete mochaArgs['node-option'];

debug('final node argv', nodeArgv);
debug('final node argv', nodeArgv);

const args = [].concat(
nodeArgv,
mochaPath,
unparse(mochaArgs, {alias: aliases})
);
const args = [].concat(
nodeArgv,
mochaPath,
unparse(mochaArgs, {alias: aliases})
);

debug(
'forking child process via command: %s %s',
process.execPath,
args.join(' ')
);
debug(
'forking child process via command: %s %s',
process.execPath,
args.join(' ')
);

const proc = spawn(process.execPath, args, {
stdio: 'inherit'
});
const proc = spawn(process.execPath, args, {
stdio: 'inherit'
});

proc.on('exit', (code, signal) => {
process.on('exit', () => {
if (signal) {
process.kill(process.pid, signal);
} else {
process.exit(code);
}
proc.on('exit', (code, signal) => {
process.on('exit', () => {
if (signal) {
process.kill(process.pid, signal);
} else {
process.exit(code);
}
});
});
});

// terminate children.
process.on('SIGINT', () => {
// XXX: a previous comment said this would abort the runner, but I can't see that it does
// anything with the default runner.
debug('main process caught SIGINT');
proc.kill('SIGINT');
// if running in parallel mode, we will have a proper SIGINT handler, so the below won't
// be needed.
if (!args.parallel || args.jobs < 2) {
// win32 does not support SIGTERM, so use next best thing.
if (require('node:os').platform() === 'win32') {
proc.kill('SIGKILL');
} else {
// using SIGKILL won't cleanly close the output streams, which can result
// in cut-off text or a befouled terminal.
debug('sending SIGTERM to child process');
proc.kill('SIGTERM');
// terminate children.
process.on('SIGINT', () => {
// XXX: a previous comment said this would abort the runner, but I can't see that it does
// anything with the default runner.
debug('main process caught SIGINT');
proc.kill('SIGINT');
// if running in parallel mode, we will have a proper SIGINT handler, so the below won't
// be needed.
if (!args.parallel || args.jobs < 2) {
// win32 does not support SIGTERM, so use next best thing.
if (require('node:os').platform() === 'win32') {
proc.kill('SIGKILL');
} else {
// using SIGKILL won't cleanly close the output streams, which can result
// in cut-off text or a befouled terminal.
debug('sending SIGTERM to child process');
proc.kill('SIGTERM');
}
}
}
});
} else {
debug('running Mocha in-process');
require('../lib/cli/cli').main([], mochaArgs);
});
} else {
debug('running Mocha in-process');
await require('../lib/cli/cli').main([], mochaArgs);
}
}

main()
4 changes: 2 additions & 2 deletions lib/cli/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const {cwd} = require('../utils');
* @param {string[]} argv - Array of arguments to parse, or by default the lovely `process.argv.slice(2)`
* @param {object} [mochaArgs] - Object of already parsed Mocha arguments (by bin/mocha)
*/
exports.main = (argv = process.argv.slice(2), mochaArgs) => {
exports.main = async (argv = process.argv.slice(2), mochaArgs) => {
debug('entered main with raw args', argv);
// ensure we can require() from current working directory
if (typeof module.paths !== 'undefined') {
Expand All @@ -46,7 +46,7 @@ exports.main = (argv = process.argv.slice(2), mochaArgs) => {
debug('unable to set Error.stackTraceLimit = Infinity', err);
}

var args = mochaArgs || loadOptions(argv);
var args = mochaArgs || await loadOptions(argv);

yargs()
.scriptName('mocha')
Expand Down
27 changes: 26 additions & 1 deletion lib/cli/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const utils = require('../utils');
exports.CONFIG_FILES = [
'.mocharc.cjs',
'.mocharc.js',
'.mocharc.mjs',
'.mocharc.yaml',
'.mocharc.yml',
'.mocharc.jsonc',
Expand All @@ -49,6 +50,28 @@ const parsers = (exports.parsers = {
return require(filepath);
}
},
esm: filepath => {
let importer;

try {
/**
* @see
* {@link https://tc39.es/ecma262/multipage/ecmascript-language-scripts-and-modules.html#sec-imports
* | ECMAScript Specifications}
*/
importer = new Function('spec', 'return import(spec);');
} catch(_) {
throw new Error(
`unable to load ${filepath}. ` +
'Your current runtime does not support dynamic module imports. ' +
'Please define the configuration with another file type.'
);
}

// fail hard, in case the environment is unable to import the ESM module so
// that the error message is as verbose as possible
return importer(filepath);
},
json: filepath =>
JSON.parse(
require('strip-json-comments')(fs.readFileSync(filepath, 'utf8'))
Expand All @@ -63,7 +86,7 @@ const parsers = (exports.parsers = {
* @param {string} filepath - Config file path to load
* @returns {Object} Parsed config object
*/
exports.loadConfig = filepath => {
exports.loadConfig = async filepath => {
let config = {};
debug('loadConfig: trying to parse config at %s', filepath);

Expand All @@ -73,6 +96,8 @@ exports.loadConfig = filepath => {
config = parsers.yaml(filepath);
} else if (ext === '.js' || ext === '.cjs') {
config = parsers.js(filepath);
} else if (ext === '.mjs') {
config = await parsers.esm(filepath);
} else {
config = parsers.json(filepath);
}
Expand Down
8 changes: 4 additions & 4 deletions lib/cli/options.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,10 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
* @alias module:lib/cli.loadRc
* @returns {external:yargsParser.Arguments|void} Parsed config, or nothing if `args.config` is `false`
*/
const loadRc = (args = {}) => {
const loadRc = async (args = {}) => {
if (args.config !== false) {
const config = args.config || findConfig();
return config ? loadConfig(config) : {};
return config ? await loadConfig(config) : {};
}
};

Expand Down Expand Up @@ -287,7 +287,7 @@ module.exports.loadPkgRc = loadPkgRc;
* @alias module:lib/cli.loadOptions
* @returns {external:yargsParser.Arguments} Parsed args from everything
*/
const loadOptions = (argv = []) => {
const loadOptions = async (argv = []) => {
let args = parse(argv);
// short-circuit: look for a flag that would abort loading of options
if (
Expand All @@ -300,7 +300,7 @@ const loadOptions = (argv = []) => {
}

const envConfig = parse(process.env.MOCHA_OPTIONS || '');
const rcConfig = loadRc(args);
const rcConfig = await loadRc(args);
const pkgConfig = loadPkgRc(args);

if (rcConfig) {
Expand Down
Loading