enable esm support in Node v10

[giltayar]

Description of the Change

Fixes #4297. It seems that Mocha works with Node v10. So I enabled it to work in Node v10, but added documentation to say that it isn't officially supported and use at your own risk.

[boneskull]

looks good, but we should probably add a test for this to be run on node 10. it could be something that checks the node version in a before hook, then runs invokeMocha with the --experimental-modules flag (this should run bin/mocha)

[boneskull]

would prefer another opinion from @mochajs/core but yeah, I think it's worth testing. @giltayar if you don't have the cycles to add it, we can merge and I can add it after.

[giltayar]

@boneskull I dunno. I don't think adding a test for v10 is worth the time and hassle. I don't see many people using Node.js v10 with modules. I certainly won't recommend it given that Node v10 ESM support is incompatible in certain ways with v>=12 Node support. And v10 is not the active LTS and is going to EOL mid next year.

[giltayar]

But your call, @boneskull. If you think it's worth it, let's merge, and I will add the test.

[boneskull]

my reasoning is that if we support node 10, people will be upset if we break it, and we won’t know if we break it unless there’s a test.

[giltayar]

@boneskull if y'all think it's worth testing, then I'll add a test.

[giltayar]

OK, the whole conversation took more than the change to also test it in Node v10. Sorry about that! ☺️

[coveralls]

Coverage increased (+0.03%) to 93.596% when pulling 0ce4c75 on giltayar:master into 63eb80b on mochajs:master.

[boneskull]

netlify is being ornery; failure is unrelated

[juergba]

You could use this.skip() as well, but it doesn't really matter.

[boneskull]

Resolved conflicts

[juergba]

@giltayar your branch is broken, could you have a look, please?

[giltayar]

Some new ESM tests failed in Node v10. Specifically, those that assume that type:module in the package.json works, which it doesn't in Node 10.

The function supportEsmModules in this PR was changed to enable asking whether the ESM support is full (v>=12) or partial (v10). The default was partial, unfortunately, so any new test that checked if it had to run got true on v10, and failed on tests that assumed "type:module" works.

I switched the supportEsmModules function to default to "full support", so that if you want to specifically check for Node v10 support you can, but by default your tests run only for Node that has full ESM support.

[giltayar]

I believe the Netlify failure is unrelated to this PR.

[juergba]

I don't know, it's the second time that netlify fails. Something is weird about you and netlify.
There shouldn't be a package-lock.json in your last commit, I think.

[giltayar]

The package-lock.json change was strange: my commit changed some URLs from https to http, and I have no idea why. I'm pushing a change that reverts that and we'll see, but frankly, I don't think that's it.

I also tried looking at the problem at Netlify and it said the page was not found or I have insufficient permissions.

[giltayar]

@juergba reverting the package-lock.json didn't work, and I can't access the netlify deploy preview details. Could you have a look at it, or if possible, give me permissions there? (My Netlify user[ is gil@tayar.org)

[juergba]

@giltayar I can't see your netlify log (page not found) neither. I can see the deploy preview of all the others (maintainers, contributors), but not yours. You are special.
Ask @Munter for support, he may know.

[boneskull]

(It's not showing because the branch is on his fork, not on mochajs/mocha)

[boneskull]

It's failing due to a caching problem

[boneskull]

I've rebased onto master; hopefully this will pass CI.

@giltayar: you opened this PR from your fork's master branch, which is problematic 😄

[boneskull]

clearing the cache before force-rebuilding worked.

the problem is that we needed to add an .nvmrc to our repo to force Node 12 for netlify. but since previous deploys were running probably Node 14, the cache contained native modules compiled against a different ABI version, so ... the only way to fix it is to blast the cache.

fwiw, I don't like the .nvmrc; we can do this another way. see #4314

[boneskull]

here's the relevant travis build which, for some reason, I'm not seeing in the list of checks.

[giltayar]

@giltayar: you opened this PR from your fork's master branch, which is problematic 😄

Sorry. Learning about OSS as I go along. Won't happen again! ☺️