From 5bff13af12efea200548f3032a133d7a5f78b70c Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 30 Jul 2024 17:53:46 +0100 Subject: [PATCH] Core: Always define globalThis.QUnit == Background In QUnit 2.x, we always set the QUnit global in browser contexts, but outside browsers, we only set the global if CJS/AMD wasn't detected. Then, in the QUnit CLI, we set the global anyway, and we expect other test runners to do the same, since that's how most QUnit tests are written, including for Node.js targets. In practice, the QUnit global would only be missing in custom test runners that 1) target Node.js, 2) require/import qunit.js directly, and 3) don't export it as a global. I'm aware many communities import 'qunit' directly in each test file for improved type support. That's great, and avoids needing to rely on globals. But, that's not a requirement I want to impose on everyone, especially for simple no-build-step and browser-facing projects. == Why Improve portability between test runners. Remove the last edge case where the QUnit global can be undefined. Make it QUnit's responsiblity to define this reliably, as indeed it almost always already does. Remove this as undocumented requirement for specific test runners to patch up on their end. In light of Karma deprecation, and emergence of more general purpose TAP runners, I'd like to remove this as factor that might make/break QUnit support in one of those. Closes https://github.com/qunitjs/qunit/pull/1771. --- .eslintrc.json | 5 +++-- build/tasks/test-on-node.js | 21 +++++++++--------- src/cli/run.js | 12 ++++------- src/core/export.js | 39 ++++++++++++++++++++++------------ test/benchmark/micro.js | 2 +- test/cli/fixtures/inception.js | 6 +++--- 6 files changed, 46 insertions(+), 39 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index f758a8d45..9f9c9fed1 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -94,6 +94,7 @@ "sourceType": "script" }, "env": { + "es2020": true, "node": true }, "rules": { @@ -109,11 +110,11 @@ { "files": ["test/cli/**"], "env": { - "es2017": true, + "es2020": true, "node": true }, "parserOptions": { - "ecmaVersion": 2018 + "ecmaVersion": 2020 }, "rules": { "compat/compat": "off" diff --git a/build/tasks/test-on-node.js b/build/tasks/test-on-node.js index a1610d0c1..e98132a71 100644 --- a/build/tasks/test-on-node.js +++ b/build/tasks/test-on-node.js @@ -16,25 +16,24 @@ module.exports = function (grunt) { grunt.log.ok(`Ran ${totals.files} files`); } - // Refresh the QUnit global to be used in other tests - global.QUnit = requireFresh('../../qunit/qunit.js'); - done(!totals.failed); }); - function requireFresh (path) { - let resolvedPath = require.resolve(path); + function requireFreshQUnit () { + // Resolve current QUnit path and remove it from the require cache + // to avoid stacking the QUnit logs. + const path = '../../qunit/qunit.js'; + const resolvedPath = require.resolve(path); delete require.cache[resolvedPath]; + + // Clear any QUnit global from a previous test + delete globalThis.QUnit; + return require(path); } async function runQUnit (file) { - // Resolve current QUnit path and remove it from the require cache - // to avoid stacking the QUnit logs. - let QUnit = requireFresh('../../qunit/qunit.js'); - - // Expose QUnit to the global scope to be seen on the other tests. - global.QUnit = QUnit; + const QUnit = requireFreshQUnit(); QUnit.config.autostart = false; await import('../../' + file); diff --git a/src/cli/run.js b/src/cli/run.js index 6b8c6a04c..9e2a2e9f9 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -43,7 +43,8 @@ async function run (args, options) { const files = utils.getFilesFromArgs(args); - QUnit = requireQUnit(); + // Replace any previous instance, e.g. in watch mode + QUnit = globalThis.QUnit = requireQUnit(); if (options.filter) { QUnit.config.filter = options.filter; @@ -65,10 +66,6 @@ async function run (args, options) { console.log(`Running tests with seed: ${QUnit.config.seed}`); } - // TODO: Enable mode where QUnit is not auto-injected, but other setup is - // still done automatically. - global.QUnit = QUnit; - options.requires.forEach(requireFromCWD); findReporter(options.reporter, QUnit.reporters).init(QUnit); @@ -177,7 +174,7 @@ function abort (callback) { process.off('exit', onExit); running = false; - delete global.QUnit; + delete globalThis.QUnit; QUnit = null; if (callback) { callback(); @@ -192,8 +189,7 @@ run.watch = function watch (args, options) { const watch = require('node-watch'); const baseDir = process.cwd(); - QUnit = requireQUnit(); - global.QUnit = QUnit; + requireQUnit(); options.requires.forEach(requireFromCWD); // Include TypeScript when in use (automatically via require.extensions), diff --git a/src/core/export.js b/src/core/export.js index 1802f45cc..4ec44ce79 100644 --- a/src/core/export.js +++ b/src/core/export.js @@ -1,18 +1,29 @@ /* global module, exports */ import { window, document, globalThis } from './globals.js'; +/** + * Available exports: + * + * globalThis: + * - browser (globalThis === window) + * - Web Worker (globalThis === self) + * - Node.js + * - SpiderMonkey (mozjs) + * - Rhino 7.14+ + * - any other embedded JS engine + * + * CommonJS module.exports (commonjs2): + * - Node.js + * + * CommonJS exports (commonjs, https://wiki.commonjs.org/wiki/Modules): + * - Rhino + */ export default function exportQUnit (QUnit) { - let exportedModule = false; - if (window && document) { // QUnit may be defined when it is preconfigured but then only QUnit and QUnit.config may be defined. - if (window.QUnit && window.QUnit.version) { + if (globalThis.QUnit && globalThis.QUnit.version) { throw new Error('QUnit has already been defined.'); } - - window.QUnit = QUnit; - - exportedModule = true; } // For Node.js @@ -21,20 +32,20 @@ export default function exportQUnit (QUnit) { // For consistency with CommonJS environments' exports module.exports.QUnit = QUnit; - - exportedModule = true; } // For CommonJS with exports, but without module.exports, like Rhino if (typeof exports !== 'undefined' && exports) { exports.QUnit = QUnit; - - exportedModule = true; } - // For other environments, including Web Workers (globalThis === self), - // SpiderMonkey (mozjs), and other embedded JavaScript engines - if (!exportedModule) { + // Ensure the global is available in all environments. + // + // For backward compatibility, we only enforce load-once in browsers above. + // In other environments QUnit is accessible via import/require() and may + // load multiple times. Callers may decide whether their secondary instance + // should be global or not. + if (!globalThis.QUnit || !globalThis.QUnit.version) { globalThis.QUnit = QUnit; } } diff --git a/test/benchmark/micro.js b/test/benchmark/micro.js index da0014964..0d7949fea 100644 --- a/test/benchmark/micro.js +++ b/test/benchmark/micro.js @@ -1,5 +1,5 @@ /* eslint-env node */ -global.QUnit = require('qunit'); +require('qunit'); require('./micro-fixture.js'); require('./micro-bench.js'); diff --git a/test/cli/fixtures/inception.js b/test/cli/fixtures/inception.js index 125117fab..8eeae9825 100644 --- a/test/cli/fixtures/inception.js +++ b/test/cli/fixtures/inception.js @@ -1,7 +1,7 @@ -const outerQUnit = global.QUnit; -delete global.QUnit; +const outerQUnit = globalThis.QUnit; +delete globalThis.QUnit; const myQUnit = require('../../../src/cli/require-qunit')(); -global.QUnit = outerQUnit; +globalThis.QUnit = outerQUnit; const data = []; myQUnit.on('runStart', function () {