From fdd01bcec8b1d59e5d32c507ba1e135c520021c3 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Mon, 16 Nov 2020 06:49:03 +0000 Subject: [PATCH] CLI: Add native support for ESM .mjs files on Node 12+ * 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 https://github.com/qunitjs/qunit/issues/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 https://github.com/qunitjs/qunit/issues/1465. --- .eslintrc.json | 2 +- Gruntfile.js | 4 +-- src/cli/run.js | 38 ++++++++++++++++++++-- src/cli/utils.js | 4 +++ test/cli/fixtures/expected/tap-outputs.js | 9 +++++ test/cli/main.js | 24 +++++++++++++- test/es2017/.eslintrc.json | 5 --- test/es2018/.eslintrc.json | 6 ++++ test/{es2017 => es2018}/async-functions.js | 0 test/es2018/esm.mjs | 9 +++++ test/es2018/sum.mjs | 5 +++ test/{es2017 => es2018}/throws.js | 0 12 files changed, 95 insertions(+), 11 deletions(-) delete mode 100644 test/es2017/.eslintrc.json create mode 100644 test/es2018/.eslintrc.json rename test/{es2017 => es2018}/async-functions.js (100%) create mode 100644 test/es2018/esm.mjs create mode 100644 test/es2018/sum.mjs rename test/{es2017 => es2018}/throws.js (100%) diff --git a/.eslintrc.json b/.eslintrc.json index 1e72bffa3..c4dcb1ced 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -52,7 +52,7 @@ ], "parserOptions": { "sourceType": "script", - "ecmaVersion": 2018 + "ecmaVersion": 2020 }, "env": { "node": true diff --git a/Gruntfile.js b/Gruntfile.js index 68634753c..db8e5b95f 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -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": { diff --git a/src/cli/run.js b/src/cli/run.js index d871368b0..38335938f 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -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; @@ -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 diff --git a/src/cli/utils.js b/src/cli/utils.js index 9a0bf4fb8..fdd309f95 100644 --- a/src/cli/utils.js +++ b/src/cli/utils.js @@ -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" ); } @@ -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, { diff --git a/test/cli/fixtures/expected/tap-outputs.js b/test/cli/fixtures/expected/tap-outputs.js index 5636ff0eb..0e83481e5 100644 --- a/test/cli/fixtures/expected/tap-outputs.js +++ b/test/cli/fixtures/expected/tap-outputs.js @@ -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 diff --git a/test/cli/main.js b/test/cli/main.js index 6a01ccd22..1cddb7889 100644 --- a/test/cli/main.js +++ b/test/cli/main.js @@ -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 ); } @@ -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" ) ) { diff --git a/test/es2017/.eslintrc.json b/test/es2017/.eslintrc.json deleted file mode 100644 index 848f2a128..000000000 --- a/test/es2017/.eslintrc.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "parserOptions": { - "ecmaVersion": 2017 - } -} diff --git a/test/es2018/.eslintrc.json b/test/es2018/.eslintrc.json new file mode 100644 index 000000000..f420f338f --- /dev/null +++ b/test/es2018/.eslintrc.json @@ -0,0 +1,6 @@ +{ + "parserOptions": { + "ecmaVersion": 2018, + "sourceType": "module" + } +} diff --git a/test/es2017/async-functions.js b/test/es2018/async-functions.js similarity index 100% rename from test/es2017/async-functions.js rename to test/es2018/async-functions.js diff --git a/test/es2018/esm.mjs b/test/es2018/esm.mjs new file mode 100644 index 000000000..ec0bbfb58 --- /dev/null +++ b/test/es2018/esm.mjs @@ -0,0 +1,9 @@ +import sum from "./sum.mjs"; + +QUnit.module( "ESM test suite", () => { + + QUnit.test( "sum()", assert => { + assert.equal( 5, sum( 2, 3 ) ); + } ); + +} ); diff --git a/test/es2018/sum.mjs b/test/es2018/sum.mjs new file mode 100644 index 000000000..1e2fbadce --- /dev/null +++ b/test/es2018/sum.mjs @@ -0,0 +1,5 @@ +function sum( a, b ) { + return a + b; +} + +export default sum; diff --git a/test/es2017/throws.js b/test/es2018/throws.js similarity index 100% rename from test/es2017/throws.js rename to test/es2018/throws.js