Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: Add native support for ESM .mjs files on Node 12+ #1510

Merged
merged 2 commits into from
Nov 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -190,8 +190,8 @@ module.exports = function( grunt ) {
// "test/module-only.js",
// "test/module-skip.js",
// "test/module-todo.js",
"test/es2017/async-functions.js",
"test/es2017/throws.js"
"test/es2018/async-functions.js",
"test/es2018/throws.js"
]
},
"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
22 changes: 22 additions & 0 deletions test/cli/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,28 @@ 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 );

// 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( 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.