code review

Author: juergba

.eslintrc.yml

@@ -37,6 +37,8 @@ overrides:
       ecmaVersion: 2018
       sourceType: module
     parser: babel-eslint
+    env:
+      browser: false
   - files:
       - test/**/*.{js,mjs}
     env:

docs/index.md

@@ -71,6 +71,7 @@ Mocha is a feature-rich JavaScript test framework running on [Node.js][] and in
 - [Command-Line Usage](#command-line-usage)
 - [Interfaces](#interfaces)
 - [Reporters](#reporters)
+- [Node.JS native ESM support](#nodejs-native-esm-support)
 - [Running Mocha in the Browser](#running-mocha-in-the-browser)
 - [Desktop Notification Support](#desktop-notification-support)
 - [Configuring Mocha (Node.js)](#configuring-mocha-nodejs)
@@ -869,7 +870,8 @@ Configuration
   --package  Path to package.json for config                            [string]
 
 File Handling
-  --extension          File extension(s) to load           [array] [default: js]
+  --extension          File extension(s) to load
+                                           [array] [default: ["js","cjs","mjs"]]
   --file               Specify file(s) to be loaded prior to root suite
                        execution                       [array] [default: (none)]
   --ignore, --exclude  Ignore file(s) or glob pattern(s)
@@ -1541,7 +1543,9 @@ Alias: `HTML`, `html`
 
 ## Node.JS native ESM support
 
-Mocha supports writing your tests as ES modules (without needing to use the `esm` polyfill module), and not just using CommonJS. For example:
+> _New in v7.1.0_
+
+Mocha supports writing your tests as ES modules, and not just using CommonJS. For example:
 
 ```js
 // test.mjs
@@ -1558,18 +1562,20 @@ this means either ending the file with a `.mjs` extension, or, if you want to us
 adding `"type": "module"` to your `package.json`.
 More information can be found in the [Node.js documentation](https://nodejs.org/api/esm.html).
 
-> Mocha supports ES modules only from Node.js v12.11.0 and above. Also note that
-> to enable this in vesions smaller than 13.2.0, you need to add `--experimental-modules` when running
+> Mocha supports ES modules only from Node.js v12.11.0 and above. To enable this in versions smaller than 13.2.0, you need to add `--experimental-modules` when running
 > Mocha. From version 13.2.0 of Node.js, you can use ES modules without any flags.
 
-### Limitations
+### Current Limitations
+
+Node.JS native ESM support still has status: **Stability: 1 - Experimental**
 
-- "Watch mode" (i.e. using `--watch` options) does not currently support ES Module test files,
-  although we intend to support it in the future
+- [Watch mode](#-watch-w) does not support ES Module test files
 - [Custom reporters](#third-party-reporters) and [custom interfaces](#interfaces)
-  can currently only be CommonJS files, although we intend to support it in the future
-- `mocharc.js` can only be a CommonJS file (can also be called `mocharc.cjs`),
-  although we intend to support ESM in the future
+  can only be CommonJS files
+- [Required modules](#-require-module-r-module) can only be CommonJS files
+- [Configuration file](#configuring-mocha-nodejs) can only be a CommonJS file (`mocharc.js` or `mocharc.cjs`)
+- When using module-level mocks via libs like `proxyquire`, `rewiremock` or `rewire`, hold off on using ES modules for your test files
+- Node.JS native ESM support does not work with [esm][npm-esm] module
 
 ## Running Mocha in the Browser
 

lib/cli/run-helpers.js

@@ -15,8 +15,6 @@ const collectFiles = require('./collect-files');
 
 const cwd = (exports.cwd = process.cwd());
 
-exports.watchRun = watchRun;
-
 /**
  * Exits Mocha when tests + code under test has finished execution (default)
  * @param {number} code - Exit code; typically # of failures
@@ -101,7 +99,7 @@ exports.handleRequires = (requires = []) => {
  * @returns {Promise<Runner>}
  * @private
  */
-exports.singleRun = async (mocha, {exit}, fileCollectParams) => {
+const singleRun = async (mocha, {exit}, fileCollectParams) => {
   const files = collectFiles(fileCollectParams);
   debug('running tests with files', files);
   mocha.files = files;
@@ -117,7 +115,7 @@ exports.singleRun = async (mocha, {exit}, fileCollectParams) => {
  * @private
  * @returns {Promise}
  */
-exports.runMocha = (mocha, options) => {
+exports.runMocha = async (mocha, options) => {
   const {
     watch = false,
     extension = [],
@@ -143,7 +141,7 @@ exports.runMocha = (mocha, options) => {
   if (watch) {
     watchRun(mocha, {watchFiles, watchIgnore}, fileCollectParams);
   } else {
-    return exports.singleRun(mocha, {exit}, fileCollectParams);
+    await singleRun(mocha, {exit}, fileCollectParams);
   }
 };
 

lib/cli/run.js

@@ -87,7 +87,6 @@ exports.builder = yargs =>
       },
       extension: {
         default: defaults.extension,
-        defaultDescription: 'js',
         description: 'File extension(s) to load',
         group: GROUPS.FILES,
         requiresArg: true,
@@ -306,7 +305,7 @@ exports.handler = async function(argv) {
   try {
     await runMocha(mocha, argv);
   } catch (err) {
-    console.error(err.stack || `Error: ${err.message || err}`);
+    console.error('\n' + (err.stack || `Error: ${err.message || err}`));
     process.exit(1);
   }
 };

lib/esm-utils.js

@@ -1,17 +1,13 @@
-// This file is allowed to use async/await because it is not exposed to browsers (see the `eslintrc`),
-// and Node supports async/await in all its non-dead version.
-
 const url = require('url');
 const path = require('path');
 
-exports.requireOrImport = async file => {
+const requireOrImport = async file => {
   file = path.resolve(file);
 
   if (path.extname(file) === '.mjs') {
     return import(url.pathToFileURL(file));
   }
-  // This way of figuring out whether a test file is CJS or ESM is currently the only known
-  // way of figuring out whether a file is CJS or ESM.
+  // This is currently the only known way of figuring out whether a file is CJS or ESM.
   // If Node.js or the community establish a better procedure for that, we can fix this code.
   // Another option here would be to always use `import()`, as this also supports CJS, but I would be
   // wary of using it for _all_ existing test files, till ESM is fully stable.
@@ -29,7 +25,7 @@ exports.requireOrImport = async file => {
 exports.loadFilesAsync = async (files, preLoadFunc, postLoadFunc) => {
   for (const file of files) {
     preLoadFunc(file);
-    const result = await exports.requireOrImport(file);
+    const result = await requireOrImport(file);
     postLoadFunc(file, result);
   }
 };

lib/mocha.js

@@ -329,8 +329,13 @@ Mocha.prototype.loadFiles = function(fn) {
  * @see {@link Mocha#addFile}
  * @see {@link Mocha#run}
  * @see {@link Mocha#unloadFiles}
- *
  * @returns {Promise}
+ * @example
+ *
+ * // loads ESM (and CJS) test files asynchronously, then runs root suite
+ * mocha.loadFilesAsync()
+ *   .then(() => mocha.run(failures => process.exitCode = failures ? 1 : 0))
+ *   .catch(() => process.exitCode = 1);
  */
 Mocha.prototype.loadFilesAsync = function() {
   var self = this;
@@ -371,8 +376,9 @@ Mocha.unloadFile = function(file) {
  * Unloads `files` from Node's `require` cache.
  *
  * @description
- * This allows files to be "freshly" reloaded, providing the ability
+ * This allows required files to be "freshly" reloaded, providing the ability
  * to reuse a Mocha instance programmatically.
+ * Note: does not clear ESM module files from the cache
  *
  * <strong>Intended for consumers &mdash; not used internally</strong>
  *
@@ -883,7 +889,11 @@ Object.defineProperty(Mocha.prototype, 'version', {
  * @see {@link Mocha#unloadFiles}
  * @see {@link Runner#run}
  * @param {DoneCB} [fn] - Callback invoked when test execution completed.
- * @return {Runner} runner instance
+ * @returns {Runner} runner instance
+ * @example
+ *
+ * // exit with non-zero status if there were test failures
+ * mocha.run(failures => process.exitCode = failures ? 1 : 0);
  */
 Mocha.prototype.run = function(fn) {
   if (this.files.length && !this.loadAsync) {

lib/utils.js

@@ -838,39 +838,18 @@ exports.defineConstants = function(obj) {
  * @description
  * Versions prior to 10 did not support ES Modules, and version 10 has an old incompatibile version of ESM.
  * This function returns whether Node.JS has ES Module supports that is compatible with Mocha's needs,
- * which is version 12 and older
+ * which is version >=12.11.
  *
- * @param {Boolean} unflagged whether the support is unflagged (`true`) or only using the `--experimental-modules` flag (`false`)
- * @returns {Boolean} whether the current version of Node.JS supports ES Modules in a way that is compatbile with Mocha
+ * @returns {Boolean} whether the current version of Node.JS supports ES Modules in a way that is compatible with Mocha
  */
-exports.supportsEsModules = function(unflagged) {
-  if (typeof document !== 'undefined') {
-    return false;
-  }
-  if (
-    typeof process !== 'object' ||
-    !process.versions ||
-    !process.versions.node
-  ) {
-    return false;
-  }
-  var versionFields = process.versions.node.split('.');
-  var major = +versionFields[0];
-  var minor = +versionFields[1];
-
-  if (major >= 13) {
-    if (unflagged) {
-      return minor >= 2;
+exports.supportsEsModules = function() {
+  if (!process.browser && process.versions && process.versions.node) {
+    var versionFields = process.versions.node.split('.');
+    var major = +versionFields[0];
+    var minor = +versionFields[1];
+
+    if (major >= 13 || (major === 12 && minor >= 11)) {
+      return true;
     }
-    return true;
   }
-  if (unflagged) {
-    return false;
-  }
-  if (major < 12) {
-    return false;
-  }
-  // major === 12
-
-  return minor >= 11;
 };

test/integration/esm.spec.js

@@ -1,56 +1,53 @@
 'use strict';
 var run = require('./helpers').runMochaJSON;
 var utils = require('../../lib/utils');
-
-if (!utils.supportsEsModules()) return;
+var args =
+  +process.versions.node.split('.')[0] >= 13 ? [] : ['--experimental-modules'];
 
 describe('esm', function() {
+  before(function() {
+    if (!utils.supportsEsModules()) this.skip();
+  });
+
   it('should pass a passing esm test that uses esm', function(done) {
-    run(
-      'esm/esm-success.fixture.mjs',
-      utils.supportsEsModules(true)
-        ? ['--no-warnings']
-        : ['--experimental-modules', '--no-warnings'],
-      function(err, result) {
-        if (err) {
-          done(err);
-          return;
-        }
-        expect(result, 'to have passed test count', 1);
-        done();
-      },
-      'pipe'
-    );
+    var fixture = 'esm/esm-success.fixture.mjs';
+    run(fixture, args, function(err, result) {
+      if (err) {
+        done(err);
+        return;
+      }
+
+      expect(result, 'to have passed test count', 1);
+      done();
+    });
   });
 
   it('should fail a failing esm test that uses esm', function(done) {
-    run(
-      'esm/esm-failure.fixture.mjs',
-      ['--experimental-modules', '--no-warnings'],
-      function(err, result) {
-        if (err) {
-          done(err);
-          return;
-        }
-        expect(result, 'to have failed test count', 1);
-        done();
+    var fixture = 'esm/esm-failure.fixture.mjs';
+    run(fixture, args, function(err, result) {
+      if (err) {
+        done(err);
+        return;
       }
-    );
+
+      expect(result, 'to have failed test count', 1).and(
+        'to have failed test',
+        'should use a function from an esm, and fail'
+      );
+      done();
+    });
   });
 
   it('should recognize esm files ending with .js due to package.json type flag', function(done) {
-    run(
-      'esm/js-folder/esm-in-js.fixture.js',
-      ['--experimental-modules', '--no-warnings'],
-      function(err, result) {
-        if (err) {
-          done(err);
-          return;
-        }
-        expect(result, 'to have passed test count', 1);
-        done();
-      },
-      'pipe'
-    );
+    var fixture = 'esm/js-folder/esm-in-js.fixture.js';
+    run(fixture, args, function(err, result) {
+      if (err) {
+        done(err);
+        return;
+      }
+
+      expect(result, 'to have passed test count', 1);
+      done();
+    });
   });
 });