Skip to content

Commit

Permalink
CLI: Add native support for ESM .mjs files on Node 12+
Browse files Browse the repository at this point in the history
* Rename test/es2017 and update ESLint config to allow for import/export
  syntax to be used. We don't rely on ESLint to know what is supported
  in Node.js, our test matrix in CI covers that already.

  We should probably re-think the way these tests are organized,
  which I've filed #1511 for.

* Update eslint config for src/cli to es2020. The dynamic `import()`
  statement was only spe'ced in es2020. This is first implemented
  (without experimental flag) in Node 12. We need to support Node 10.
  However it looks like Node 10 already tolerated this (maybe it sees
  it as a normal function, or maybe it was already recognised by the
  V8 parser it shipped with), so the try-catch suffices there.
  Again, the CI test matrix verifies for us that stuff works fine
  on older versions.

Ref #1465.
  • Loading branch information
Krinkle committed Nov 18, 2020
1 parent 259f9af commit fdd01bc
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
],
"parserOptions": {
"sourceType": "script",
"ecmaVersion": 2018
"ecmaVersion": 2020
},
"env": {
"node": true
Expand Down
4 changes: 2 additions & 2 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ module.exports = function( grunt ) {
"test/module-only",
"test/module-skip",
"test/module-todo",
"test/es2017/async-functions",
"test/es2017/throws"
"test/es2018/async-functions",
"test/es2018/throws"
]
},
"watch-repeatable": {
Expand Down
38 changes: 36 additions & 2 deletions src/cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const changedPendingPurge = [];

let QUnit;

function run( args, options ) {
async function run( args, options ) {

// Default to non-zero exit code to avoid false positives
process.exitCode = 1;
Expand Down Expand Up @@ -48,8 +48,42 @@ function run( args, options ) {
const filePath = path.resolve( process.cwd(), files[ i ] );
delete require.cache[ filePath ];

// Node.js 12.0.0 has node_module_version=72
// https://nodejs.org/en/download/releases/
const nodeVint = process.config.variables.node_module_version;

try {
require( filePath );

// QUnit supports passing ESM files to the 'qunit' command when used on
// Node.js 12 or later. The dynamic import() keyword supports both CommonJS files
// (.js, .cjs) and ESM files (.mjs), so we could simply use that unconditionally on
// newer Node versions, regardless of the given file path.
//
// But:
// - Node.js 12 emits a confusing "ExperimentalWarning" when using import(),
// even if just to load a non-ESM file. So we should try to avoid it on non-ESM.
// - This Node.js feature is still considered experimental so to avoid unexpected
// breakage we should continue using require(). Consider flipping once stable and/or
// as part of QUnit 3.0.
// - Plugins and CLI bootstrap scripts may be hooking into require.extensions to modify
// or transform code as it gets loaded. For compatibility with that, we should
// support that until at least QUnit 3.0.
// - File extensions are not sufficient to differentiate between CJS and ESM.
// Use of ".mjs" is optional, as a package may configure Node to default to ESM
// and optionally use ".cjs" for CJS files.
//
// https://nodejs.org/docs/v12.7.0/api/modules.html#modules_addenda_the_mjs_extension
// https://nodejs.org/docs/v12.7.0/api/esm.html#esm_code_import_code_expressions
// https://github.com/qunitjs/qunit/issues/1465
try {
require( filePath );
} catch ( e ) {
if ( e.code === "ERR_REQUIRE_ESM" && ( !nodeVint || nodeVint >= 72 ) ) {
await import( filePath );
} else {
throw e;
}
}
} catch ( e ) {

// eslint-disable-next-line no-loop-func
Expand Down
4 changes: 4 additions & 0 deletions src/cli/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ function getFilesFromArgs( args ) {

// Default to files in the test directory
if ( !patterns.length ) {

// TODO: In QUnit 3.0, change the default to {js,mjs}
patterns.push( "test/**/*.js" );
}

Expand All @@ -45,6 +47,8 @@ function getFilesFromArgs( args ) {
files.add( pattern );
} else {
if ( stat && stat.isDirectory() ) {

// TODO: In QUnit 3.0, change the default to {js,mjs}
pattern = `${pattern}/**/*.js`;
}
const results = glob( pattern, {
Expand Down
9 changes: 9 additions & 0 deletions test/cli/fixtures/expected/tap-outputs.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,15 @@ not ok 2 Example > bad
# fail 1
`,

"qunit ../../es2018/esm.mjs":
`TAP version 13
ok 1 ESM test suite > sum\\(\\)
1..1
# pass 1
# skip 0
# todo 0
# fail 0`,

"qunit timeout":
`TAP version 13
not ok 1 timeout > first
Expand Down
24 changes: 23 additions & 1 deletion test/cli/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,14 @@ QUnit.module( "CLI Main", function() {
await execute( command );
} catch ( e ) {
assert.equal( e.code, 1 );
assert.equal( e.stderr, "" );

// Node 12 enabled ESM by default, without experimental flag,
// but left the warning in stderr. The warning was removed in Node 14.
// Don't bother checking stderr
if ( semver.gte( process.versions.node, "14.0.0" ) ) {
assert.equal( e.stderr, "" );
}

const re = new RegExp( expectedOutput[ command ] );
assert.equal( re.test( e.stdout ), true );
}
Expand Down Expand Up @@ -135,6 +142,21 @@ QUnit.module( "CLI Main", function() {
}
} );

if ( semver.gte( process.versions.node, "12.0.0" ) ) {
QUnit.test( "run ESM test suite with import statement", async function( assert ) {
const command = "qunit ../../es2018/esm.mjs";
const execution = await execute( command );

assert.equal( execution.code, 0 );
assert.equal( execution.stderr, "" );
const re = new RegExp( expectedOutput[ command ] );
assert.equal( re.test( execution.stdout ), true );
if ( !re.test( execution.stdout ) ) {
assert.equal( execution.stdout, expectedOutput[ command ] );
}
} );
}

// https://nodejs.org/dist/v12.12.0/docs/api/cli.html#cli_enable_source_maps
if ( semver.gte( process.versions.node, "14.0.0" ) ) {

Expand Down
5 changes: 0 additions & 5 deletions test/es2017/.eslintrc.json

This file was deleted.

6 changes: 6 additions & 0 deletions test/es2018/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"parserOptions": {
"ecmaVersion": 2018,
"sourceType": "module"
}
}
File renamed without changes.
9 changes: 9 additions & 0 deletions test/es2018/esm.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import sum from "./sum.mjs";

QUnit.module( "ESM test suite", () => {

QUnit.test( "sum()", assert => {
assert.equal( 5, sum( 2, 3 ) );
} );

} );
5 changes: 5 additions & 0 deletions test/es2018/sum.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function sum( a, b ) {
return a + b;
}

export default sum;
File renamed without changes.

0 comments on commit fdd01bc

Please sign in to comment.