Unable to import mocha API functions from 'mocha'

[apellerano-pw]

Prerequisites

• Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
• Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
• 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
• Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

It is no longer possible to do ES6 style imports from mocha. e.g.

import { beforeEach, afterEach } from 'mocha';

import { describe, it } from 'mocha';

Steps to Reproduce

Write a my-test.js module like

import { describe, it } from 'mocha';

describe('A test', function () {
  it('should thing', function () {
  });
});

Expected behavior:

This test runs and passes

Actual behavior:

Something like

TypeError: (0 , mocha__WEBPACK_IMPORTED_MODULE_2__.describe) is not a function

(In this case webpack is the bundler)

Reproduces how often:

100%

Versions

I believe this was introduced by this PR: #4746 and released as version 9.1.2

Additional info

I agree with the general goal in the PR: to stop polluting the mocha module with the contents of global; but it throws the baby out with the bathwater because this was previously how you could write self-documenting import statements which keeps linters happy. eslint:recommended contains no-undef so if you are writing your tests in modules and bundling them together, you are now in a pickle.

[juergba]

@PaperStrike can you join in, please? I need your support.

@apellerano-pw so you are bundling using Mocha's bundle, or using browser-entry.js only?
Which bundle are you using, mocha.js or mocha-es2018.js? It doesn't make a difference, but anyway ...

If you evtl. have some time left, could you test which solution would work in your case:

• Stop using all global vars in browser entry #4746 (comment)
• Stop iterating over window props #4745 (comment)

[apellerano-pw]

Hello @juergba, thank you for reaching out.

I'm not fully aware of the bundling approach because I'm using karma along with karma-mocha so it is normally abstracted away. But I have breakpointed the issue and it looks to be importing browser-entry.js.

Here is the failing line in the bundle:

(0,mocha__WEBPACK_IMPORTED_MODULE_0__.beforeEach)(function () {

And here is the earlier definition of that var:

/* harmony import */ var mocha__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! mocha */ "./node_modules/mocha/browser-entry.js");

I also looked at the bundled output for browser-entry.js and its Mocha import is mocha.js.

var Mocha = __webpack_require__(/*! ./lib/mocha */ "./node_modules/mocha/lib/mocha.js");

I didn't find mocha-es2018.js in the bundle.

I locally modified my browser-entry.js to include the snippet from your first linked comment (iterate over Mocha keys and add them to mocha) and while that did fix the import statement issue, it failed down the line because currentContext within the function is undefined. So that approach may need a little more setup. It looks like currentContext is normally set in a suite.on('pre-require') hook and that hook is not firing.

I next tried the second linked comment (push a hardcoded array of props from global to mocha) and with this everything works fine again!

The arrays are quite different between them so that could explain the difference in behavior. As could the source of the function. The non-working solution copies them from Mocha and the working solution from global.

Object.keys(Mocha).filter(k => !(k in mocha)).sort()
> (23) ['Context', 'Hook', 'Runnable', 'Runner', 'Suite', 'Test', 'after', 'afterEach', 'before', 'beforeEach', 'describe', 'interfaces', 'it', 'process', 'reporters', 'suiteSetup', 'suiteTeardown', 'teardown', 'test', 'unloadFile', 'utils', 'xdescribe', 'xit']

[ ... the manual array ].sort()
> (21) ['Mocha', 'after', 'afterEach', 'before', 'beforeEach', 'context', 'describe', 'it', 'mocha', 'run', 'setup', 'specify', 'suite', 'suiteSetup', 'suiteTeardown', 'teardown', 'test', 'xcontext', 'xdescribe', 'xit', 'xspecify']

[PaperStrike]

I'm willing to join in but I'm quite confused how it works too. Sorry for the inconvenience. I never have the global describe stuffs defined when browser-entry.js exports (i.e., I can't write like your my-test.js in the issue description even before 9.1.2), and I assumed that's because they won't be in global before a mocha.setup. So I'm quite surprised the PR do affects some cases. And another thing surprised me is that you have unusable currentContext and usable describe stuffs in global at the same time. 🤔

[juergba]

@apellerano-pw @PaperStrike thank you, guys.

In Node this currentContext is set to global (which corresponds to globalThis / window / ?? after bundling). We use a proxy for setting currentContext since in parallel mode each process has its own global. In browser there is no parallel mode, so evtl. by just adding var currentContext = global; to browser-entry.js should do the trick.

I tend to the second solution. It partially does what we had before, assigning from global to mocha, this time just more selective.

Some months ago we bundled to IIFE format, then had to switched to UMD due to problems with other bundlers using our bundle. Mocha, mocha , and global are not identical in these two formats. So - when we drop IE11 with next major release - I'm dreaming of bundling to the ESM format. Which hopefully should be easier to understand and also capable for other bundlers.

[juergba]

Do you both agree with the second approach?
@PaperStrike are you going to open a PR?

[apellerano-pw]

Second approach makes the most sense to me.

[PaperStrike]

Emm, I'm still confused how you can have unusable currentContext and usable global describe stuffs at the same time.
Sorry but before I know what's going on I won't open a PR. You are free to open one as you wish.

And if any of you are willing to help me through it, here's my question:

It looks like currentContext is normally set in a suite.on('pre-require') hook and that hook is not firing.

That hook is fired here,

mocha/browser-entry.js

Lines 137 to 141 in 111467f

	mocha.ui = function(ui) {
	Mocha.prototype.ui.call(this, ui);
	this.suite.emit('pre-require', global, null, this);
	return this;
	};

and mocha.ui should be called when you run mocha.setup with a ui option in

mocha/browser-entry.js

Lines 147 to 165 in 111467f

	mocha.setup = function(opts) {
	if (typeof opts === 'string') {
	opts = {ui: opts};
	}
	if (opts.delay === true) {
	this.delay();
	}
	var self = this;
	Object.keys(opts)
	.filter(function(opt) {
	return opt !== 'delay';
	})
	.forEach(function(opt) {
	if (Object.prototype.hasOwnProperty.call(opts, opt)) {
	self[opt](opts[opt]);
	}
	});
	return this;
	};

Considering the second approach works, it seems like one can have valid describe stuffs before a valid mocha.setup? That's astonishing to me.

[juergba]

In the Mocha constructor there is a call to this.ui() as well.
So the order in browser-entry.js is:

• var mocha = new Mocha({reporter: 'html'});: calls this.ui() with default bdd
• mocha.suite.removeAllListeners('pre-require');: purpose?
• mocha.setup(): second call to this.ui()

We don't know either how karma/karma-mocha is bundling there bundle. It looks like they don't use Mocha's bundle, but create their own, see I locally modified my browser-entry.js to include the snippet [...].

[juergba]

My latest comment doesn't really help.
I did some debugging and I see your puzzle. At the time of pushing our functions from global to mocha, none of these functions exists on global.

What about moving the function copying into the ui function?

Or could an additional, second emit within ui() work? this.suite.emit('pre-require', this, null, this);
On this(== mocha) instead of global?

[juergba]

Considering the second approach works, it seems like one can have valid describe stuffs before a valid mocha.setup? That's astonishing to me.

There must be more than one bundling process, with karma-runner and also with our own CI tests. The first one bundles mocha and the second bundles all the test files. So evtl. after the first bundling process, all our global functions are set?

[juergba]

@apellerano-pw could you test with this short patch, please?

// prettier-ignore
[ 
  'describe', 'it', 'before', 'beforeEach', 'afterEach', 'after',
  'xdescribe', 'xit'
].forEach(function(key) {
  mocha[key] = global[key];
});

If this patch works, I will open a PR and merge, but for sure it's not a nice solution, and it has never been:

• karma-mocha is not well maintained at all and it doesn't feel good to use such an old version code. Without understanding what's going on, at least this issue seems to hit only when using bundlers with import/require.
• global.run and mocha.run are two distinct functions, therefore no copy of run.
• global.setup (tdd version of beforeEach hook) and mocha.setup are two distinct functions, therefore no copy of all tdd functions. Only bdd interface is available via import/require.
• no copy of Mocha and mocha, I don't see a reason for now.
• I wonder why there are no more reports of this issue by users.

[apellerano-pw]

Thank you for suggesting a patch, @juergba. I tested the following patch and it works.

[
  'after',
  'afterEach',
  'before',
  'beforeEach',
  'context',
  'describe',
  'it',
  'specify',
  'xcontext',
  'xdescribe',
  'xit',
  'xspecify'
].forEach(function (key) {
  mocha[key] = global[key];
});

I added the context and specify (and xcontext, xspecify) entries to the array since they are synonyms for describe and it respectively.