Skip to content

Conversation

@thescientist13
Copy link

@thescientist13 thescientist13 commented Jul 1, 2025

PR Checklist

Overview

Introduces the ability to use ESM configuration files for Mocha, leveraging the require(ESM) capability of Node >= 20.19.0, and updated current and next docs.

See a demo repo with these patches here - https://github.com/thescientist13/mocha-esm-config

TODO

  1. Test Case
  2. Documentation

Per my understanding of support require(ESM) is so far only in >= 20.19.0, so not sure how this will fare with test cases running on Node 18, and or if this is something to just call out in the documentation? Not sure if will get backported to 18.x at this point as it is EOL now...

Either way, let me know your thoughts on the current implementation and the implications of Node support. Happy to add a test case / docs once I make sure this changeset is heading in the right direction. 🫡

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 1, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

* @returns {Object} Parsed config object
*/
exports.loadConfig = filepath => {
const packageJson = require(path.resolve(utils.cwd(), 'package.json'));
Copy link
Author

Choose a reason for hiding this comment

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

Not sure if there's a better / more idiomatic way in Mocha to get the user's package.json file?

Copy link
Member

Choose a reason for hiding this comment

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

I would personally use something like package-up

But, determining whether the caller is CJS or ESM can be more tricky than that. My instinct is it'd be best to not add in specific logic for either. Using require(esm), we can do something simpler like packageJson.default ?? packageJson and call it a day.

@thescientist13 thescientist13 changed the title feature/ESM configuration file feat: ESM configuration file Sep 24, 2025
@JoshuaKGoldberg JoshuaKGoldberg added the status: waiting for author waiting on response from OP or other posters - more information needed label Sep 30, 2025
@JoshuaKGoldberg
Copy link
Member

👋 @thescientist13 just checking in, are you waiting on us for anything?

Now that #5358 is merged, this shouldn't have to worry about require(esm) not being supported. Though we are still seeing errors in other PRs (#5482, #5484, #5487) around that which we still need to debug. If you do see errors around CJS/ESM support you can ignore them for now.

@thescientist13
Copy link
Author

hey @JoshuaKGoldberg , thanks for checking in!

Not waiting on anything per se, was just waiting to see what would happen with that other PR. I just rebased against main and pushed that up here with your feedback suggestion from earlier, so I can work on tests / docs if that's good with you.

@thescientist13 thescientist13 marked this pull request as ready for review October 14, 2025 14:08
@thescientist13
Copy link
Author

Had some intermittent test failures locally, but none of them seemed related to the CLI changes here, so throwing this up to see what happens. 😅

@mark-wiemer
Copy link
Member

mark-wiemer commented Nov 1, 2025

Hi @thescientist13 , thanks for the PR :) I was sick for a while but have recovered, I've resolved merge conflicts and triggered the workflow to run. Please review the results and let me know if you need help addressing any potential new test failures (you can review #5361 for our known issues). I'll check back on this in a bit to do my first review, hopefully all tests pass today!

@mark-wiemer mark-wiemer self-requested a review November 1, 2025 16:11
@codecov
Copy link

codecov bot commented Nov 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.68%. Comparing base (f75d150) to head (90f9dae).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5397      +/-   ##
==========================================
- Coverage   93.69%   93.68%   -0.02%     
==========================================
  Files          57       57              
  Lines        4396     4399       +3     
  Branches      849      851       +2     
==========================================
+ Hits         4119     4121       +2     
- Misses        277      278       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thescientist13
Copy link
Author

thescientist13 commented Nov 1, 2025

Thanks for checking in @mark-wiemer 👋

It looks like everything is passing, the only failures seem to be coming from running some of the codecov tasks?

==> Finishing downloading windows:latest
      Version: v11.2.4
 
gpg: directory '/c/Users/runneradmin/.gnupg' created
gpg: no valid OpenPGP data found.
gpg: Total number processed: 0
==> Verifying GPG signature integrity
 -> Downloading https://cli.codecov.io/latest/windows/codecov.exe.SHA256SUM
 -> Downloading https://cli.codecov.io/latest/windows/codecov.exe.SHA256SUM.sig
 
gpg: Signature made Thu Oct 23 15:06:32 2025 CUT
gpg:                using RSA key 27034E7FDB850E0BBC2C62FF806BB28AED779869
gpg: Can't check signature: No public key
==> Could not verify signature. Please contact Codecov if problem continues
    Exiting...
Error: Process completed with exit code 1.

There is still an open TODO item around documentation, so if you're initial review looks good, I can get that added too. Thinking something like this?
https://mochajs.org/next/running/configuring/

  • JavaScript: Create a .mocharc.js (or .mocharc.cjs when using "type"="module" in your package.json) in your project's root directory, and export an object (module.exports = {/* ... */}) containing your configuration. For native ESM and using type="module" or using .mjs, use a default export (default export {/* ... */}).

(and should I do it for both old and new docs?)

@mark-wiemer
Copy link
Member

We very recently switched to Codecov, I guess we'll have to work out this kink. But you're right, the tests did pass, so I'll review this one in a bit :)

@mark-wiemer mark-wiemer removed the status: waiting for author waiting on response from OP or other posters - more information needed label Nov 1, 2025
@mark-wiemer mark-wiemer self-assigned this Nov 22, 2025
@mark-wiemer mark-wiemer added this to the v12.0.0 milestone Nov 22, 2025
@mark-wiemer
Copy link
Member

I'm not reviewing any PRs in detail right this moment, but I will be tomorrow. Turns out recovery from bronchitis has taken much longer than expected and I'm still not quite working 40 hours a week yet, so apologies for the delay. This is currently top of my list (but I have ~20 more PRs to check out tonight ;) )

Copy link
Member

@mark-wiemer mark-wiemer left a comment

Choose a reason for hiding this comment

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

Organized the tests a bit because they were bothering me, but the core changes work great, thanks for this! Also, you've taught me about the patch-package trick, so cool!

@mark-wiemer mark-wiemer added the semver-major implementation requires increase of "major" version number; "breaking changes" label Nov 22, 2025
@mark-wiemer
Copy link
Member

mark-wiemer commented Nov 22, 2025

@thescientist13 an update to the docs would be much appreciated, and yes, please update both the old and new docs. Your current draft in your latest readme looks good :) You're welcome to add the changes to this PR

@mark-wiemer mark-wiemer added the status: waiting for author waiting on response from OP or other posters - more information needed label Nov 22, 2025
@mark-wiemer mark-wiemer mentioned this pull request Nov 22, 2025
3 tasks
@thescientist13
Copy link
Author

Great, thanks @mark-wiemer . Will get that docs change going now. 👍

@thescientist13
Copy link
Author

Docs changes pushed!

@mark-wiemer mark-wiemer removed the status: waiting for author waiting on response from OP or other posters - more information needed label Nov 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-major implementation requires increase of "major" version number; "breaking changes"

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

🚀 Feature: ESM configuration file

3 participants