From c78d475f3f635323c3443d3267dd7b4a3912c1b5 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 18 Jun 2024 21:51:08 +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. Except in the QUnit CLI, where we do set the global anyway, because that's how most QUnit tests are written, incliuding for Node.js. So really the only case where the QUnit global is missing is custom test runners that: * run in Node.js, * and require/import qunit.js directly, * and don't export it as a global. I'm aware a growing proportion of developers import 'qunit' directly in each test file for improved type support. That's a great way to avoid needing to rely on globals. But, it's not a requirement, and is not always an option, especially for simple no-build-step and browser-facing projects. == Why == 1. 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. 2. Prepare for native ESM build. We don't natively support ESM exports yet, but once we do this will become a problem. To prevent split-brain problems with mixed use (e.g. in test registry and other state) standardise internally on which ever globalThis.QUnit was defined first, and then reliably export that to any importers. Ref https://github.com/qunitjs/qunit/issues/1551. --- build/tasks/test-on-node.js | 9 +++--- src/cli/run.js | 9 ++---- src/core.js | 3 -- src/core/config.js | 2 +- src/export.js | 40 ----------------------- src/qunit.js | 61 +++++++++++++++++++++++++++++++++--- test/benchmark/index-node.js | 2 +- test/overload.html | 14 ++++----- 8 files changed, 71 insertions(+), 69 deletions(-) delete mode 100644 src/export.js diff --git a/build/tasks/test-on-node.js b/build/tasks/test-on-node.js index 5f87be5aa..05bef4427 100644 --- a/build/tasks/test-on-node.js +++ b/build/tasks/test-on-node.js @@ -16,8 +16,9 @@ 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'); + // Reset for anything after this task + delete globalThis.QUnit; + delete require.cache[require.resolve('../../qunit/qunit.js')]; done(!totals.failed); }); @@ -31,11 +32,9 @@ module.exports = function (grunt) { function runQUnit (file, runEnd) { // Resolve current QUnit path and remove it from the require cache // to avoid stacking the QUnit logs. + delete globalThis.QUnit; let QUnit = requireFresh('../../qunit/qunit.js'); - // Expose QUnit to the global scope to be seen on the other tests. - global.QUnit = QUnit; - QUnit.config.autostart = false; requireFresh('../../' + file); registerEvents(QUnit, file, runEnd); diff --git a/src/cli/run.js b/src/cli/run.js index fe3aad4d2..89f70681c 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -64,10 +64,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); @@ -176,7 +172,7 @@ function abort (callback) { process.off('exit', onExit); running = false; - delete global.QUnit; + delete globalThis.QUnit; QUnit = null; if (callback) { callback(); @@ -196,8 +192,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.js b/src/core.js index fa083fc57..c12230f63 100644 --- a/src/core.js +++ b/src/core.js @@ -5,7 +5,6 @@ import dump from './dump'; import { runSuite, module } from './module'; import Assert from './assert'; import Test, { test, pushFailure } from './test'; -import exportQUnit from './export'; import reporters from './reporters'; import config from './core/config'; @@ -141,6 +140,4 @@ function begin () { }).then(unblockAndAdvanceQueue); } -exportQUnit(QUnit); - export default QUnit; diff --git a/src/core/config.js b/src/core/config.js index 86e07bb91..d63835dbe 100644 --- a/src/core/config.js +++ b/src/core/config.js @@ -212,7 +212,7 @@ readFlatPreconfig(globalThis); // Apply a predefined QUnit.config object // // Ignore QUnit.config if it is a QUnit distribution instead of preconfig. -// That means QUnit was loaded twice! (Use the same approach as export.js) +// This should use the same heuristic as the export in qunit.js. const preConfig = globalThis && globalThis.QUnit && !globalThis.QUnit.version && globalThis.QUnit.config; if (preConfig) { extend(config, preConfig); diff --git a/src/export.js b/src/export.js deleted file mode 100644 index cbf15b3c3..000000000 --- a/src/export.js +++ /dev/null @@ -1,40 +0,0 @@ -/* global module, exports */ -import { window, document, globalThis } from './globals'; - -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) { - throw new Error('QUnit has already been defined.'); - } - - window.QUnit = QUnit; - - exportedModule = true; - } - - // For Node.js - if (typeof module !== 'undefined' && module && module.exports) { - module.exports = 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) { - globalThis.QUnit = QUnit; - } -} diff --git a/src/qunit.js b/src/qunit.js index 1f9cbccd5..c69c62ad0 100644 --- a/src/qunit.js +++ b/src/qunit.js @@ -1,7 +1,60 @@ -import QUnit from './core'; +/* global module, exports */ + +import core from './core'; import { initBrowser } from './browser/browser-runner'; -import { window, document } from './globals'; +import { globalThis, window, document } from './globals'; + +/** + * 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 + */ + +// If a real QUnit global was already defined, then replace our reference +// with that one, and export that instead. Skip initBrowser() to avoid +// doubling the user interface. +// +// Since QUnit 3.0, we no longer throw an error if QUnit is loaded twice. +// This enables mixed use of ESM import and CJS require() in a project, +// without split-brain problems for defining tests, setting config, registering +// reporters, etc. +// +// Note that a placeholder QUnit global may exist for preconfiguration. +// Such placeholder is recognised by not having QUnit.version (it should +// only contain QUnit.config), and will be upgraded to the real QUnit. +let QUnit; +if (globalThis.QUnit && globalThis.QUnit.version) { + QUnit = globalThis.QUnit; +} else { + QUnit = core; + globalThis.QUnit = QUnit; + + if (window && document) { + initBrowser(QUnit, window, document); + } +} + +// For Node.js +if (typeof module !== 'undefined' && module && module.exports) { + module.exports = QUnit; + + // For consistency with CommonJS environments' exports + module.exports.QUnit = QUnit; +} -if (window && document) { - initBrowser(QUnit, window, document); +// For CommonJS with exports, but without module.exports, like Rhino +if (typeof exports !== 'undefined' && exports) { + exports.QUnit = QUnit; } diff --git a/test/benchmark/index-node.js b/test/benchmark/index-node.js index 140edb11f..f8f17e284 100644 --- a/test/benchmark/index-node.js +++ b/test/benchmark/index-node.js @@ -1,6 +1,6 @@ /* eslint-env node */ -global.QUnit = require('qunit'); +require('qunit'); if (process.argv[2] === 'fast-deep-equal') { const fde = require('fast-deep-equal/es6'); diff --git a/test/overload.html b/test/overload.html index 961cef518..1cd8e97e3 100644 --- a/test/overload.html +++ b/test/overload.html @@ -13,17 +13,15 @@ }; - +