Feature: ESM configuration file

[ByteB4rb1e]

PR Checklist

• Addresses an existing open issue: fixes 🚀 Feature: ESM configuration file #5049
• That issue was marked as status: accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

Added support for ESM module (.mocharc.mjs Michael Jackson Script) RC configuration file. As the nature of ESM module resolution is asynchronous, this requires the entire call tree to be asynchronous. Though I considered some refactoring along the way, this PR solely focuses on adding support for RC ESM module support.

The PR is purposefully in Draft state, as it may make sense to do some refactoring along the way? I've added my thoughts in the affected commits. Will add a test case prior to removing the Draft state.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ByteB4rb1e / name: Tiara Rodney (14ac77d, 639904b)

[ByteB4rb1e]

As far as I can tell, at least some test case failures are also applicable to the main branch at the moment? I will try to configure test-reporting as to get a JSON report for each test suite on my branch as well as main and diff them.

[JoshuaKGoldberg]

Yeah, if it's also on the main branch then you don't have to worry about it.

[JoshuaKGoldberg]

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.

[ByteB4rb1e]

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...

[ByteB4rb1e]

I've written an idempotent script for comparing the failing test suites of my branch and main and additional failures became apparent, which makes me question the feasibility of implementing this feature request with native ESM compatibility, in a manner that does not introduce breaking changes to mocha.
I agree with @JoshuaKGoldberg that the usage of the require() compatibility layer for loading ESM modules synchronously might be less disruptive... Sadly... Really would have loved to see native ESM adoption.

How should I proceed from here?

#!/usr/bin/env sh
get_test_report() {
    workdir="$1"
    remote_url="$2"
    branch="$3"
    spec_basedir="$4"
    output_path="$5"

    cd "$workdir"

    sh -cx "git clone --depth 1 --single-branch --branch '$branch' '$remote_url' ."

    sh -cx "npm install"

    sh -cx "node bin/mocha.js '$spec_basedir'/**/*.spec.js --reporter json-stream" \
    | tee "$output_path"
}

compare_test_reports() {
    spec_basedir="$1"
    diff_out_file="$2"
    workdir="$(mktemp -d)"

    cd "$workdir"

    # define the working directories for execution beforehand so that we can
    # normalize spec paths and make them canonical for comparison.
    test_report_1_workdir="$(mktemp -d)"
    test_report_2_workdir="$(mktemp -d)"

    # paths to output the test reports to
    test_report_1_path="$workdir/main-test-report.json"
    test_report_2_path="$workdir/byteb4rb1e-feature-5049-test-report.json"

    # execute integration test suites on main branch
    get_test_report "$test_report_1_workdir" \
                    'https://github.com/mochajs/mocha.git' \
                    'main' \
                    "$spec_basedir" \
                    "$test_report_1_path" &
    jid="$!"

    # execute integration test suites on PR branch
    get_test_report "$test_report_2_workdir" \
                    'https://github.com/byteb4rb1e/mocha.git' \
                    'feature/5049' \
                    "$spec_basedir" \
                    "$test_report_2_path" &
    jid="$jid $!"

    wait $jid

    # make the path pretty for NT environments
    # only applicable for Cygwin/MSYS2 - in my case
    #diff_out_file="$(
    #    cygpath -w "$diff_out_file" | sed 's|\\|\\\\|g'
    #)"
    #test_report_1_workdir="$(
    #    cygpath -w "$test_report_1_workdir" | sed 's|\\|\\\\|g'
    #)"
    #test_report_2_workdir="$(
    #    cygpath -w "$test_report_2_workdir" | sed 's|\\|\\\\|g'
    #)"
    #
    #test_report_1_path="$(
    #    cygpath -w "$test_report_1_path" | sed 's|\\|\\\\|g'
    #)"
    #test_report_2_path="$(
    #    cygpath -w "$test_report_2_path" | sed 's|\\|\\\\|g'
    #)"

    node << EOF
import { open } from 'node:fs/promises';
import fs from 'node:fs';
import path from 'node:path';
import { createHash } from 'node:crypto';

const testReport1Workdir = '$test_report_1_workdir';
const testReport2Workdir = '$test_report_2_workdir';

const testReport1Path = '$test_report_1_path';
const testReport2Path = '$test_report_2_path';

const diffOutputPath = '$diff_out_file';

const mainResultHashes = {
    pass: [],
    fail: []
};

(async () => {
    const f1 = await open(testReport1Path);
    const f2 = await open(testReport2Path);

    // iterate over every streamed JSON object, extract the case state and
    // metadata, calculate a canonicalized hash and store it in an array for
    // lookup when iterating over the other test results
    for await (const line of f1.readLines()) {
        const [caseState, caseMeta] = JSON.parse(line);

        // skip anything that has nothing to do with a concrete test case
        if (!['fail', 'pass'].includes(caseState)) continue;

        //canonicalize the test suite file path
        const canonFilePath = path.relative(testReport1Workdir, caseMeta.file);

        // calculate a hash of the test case
        const hash = createHash('sha256');
        hash.update(canonFilePath);
        hash.update(caseMeta.fullTitle);
        const hashDigest = hash.copy().digest('hex');

        mainResultHashes[caseState].push(hashDigest);
    }

    for await (const line of f2.readLines()) {
        const [caseState, caseMeta] = JSON.parse(line);

        // skip anything that has nothing to do with a concrete test case
        if (!['fail', 'pass'].includes(caseState)) continue;

        //canonicalize the test suite file path
        const canonFilePath = path.relative(testReport2Workdir, caseMeta.file);

        // calculate a hash of the test case
        const hash = createHash('sha256');
        hash.update(canonFilePath);
        hash.update(caseMeta.fullTitle);
        const hashDigest = hash.copy().digest('hex');

        if (!mainResultHashes[caseState].includes(hashDigest)) {
            var msg  = \`$spec_basedir: mismatched, (is) '\${caseState}': \`;
                msg += JSON.stringify(caseMeta, null, 4);
                msg += '\\n\\n';

            console.log(msg);

            fs.appendFileSync(diffOutputPath, msg);
        }
    }
})()
EOF
}

out_file="$(pwd)/mocha-5094-test-reports-diff";

compare_test_reports 'test/integration' "$out_file"
compare_test_reports 'test/unit' "$out_file"
compare_test_reports 'test/node-unit' "$out_file"

mocha-5094-test-reports-diff.txt