Skip to content

Commit 5bff13a

Browse files
authored
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 #1771.
1 parent 5812597 commit 5bff13a

File tree

6 files changed

+46
-39
lines changed

6 files changed

+46
-39
lines changed

.eslintrc.json

+3-2
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
"sourceType": "script"
9595
},
9696
"env": {
97+
"es2020": true,
9798
"node": true
9899
},
99100
"rules": {
@@ -109,11 +110,11 @@
109110
{
110111
"files": ["test/cli/**"],
111112
"env": {
112-
"es2017": true,
113+
"es2020": true,
113114
"node": true
114115
},
115116
"parserOptions": {
116-
"ecmaVersion": 2018
117+
"ecmaVersion": 2020
117118
},
118119
"rules": {
119120
"compat/compat": "off"

build/tasks/test-on-node.js

+10-11
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,24 @@ module.exports = function (grunt) {
1616
grunt.log.ok(`Ran ${totals.files} files`);
1717
}
1818

19-
// Refresh the QUnit global to be used in other tests
20-
global.QUnit = requireFresh('../../qunit/qunit.js');
21-
2219
done(!totals.failed);
2320
});
2421

25-
function requireFresh (path) {
26-
let resolvedPath = require.resolve(path);
22+
function requireFreshQUnit () {
23+
// Resolve current QUnit path and remove it from the require cache
24+
// to avoid stacking the QUnit logs.
25+
const path = '../../qunit/qunit.js';
26+
const resolvedPath = require.resolve(path);
2727
delete require.cache[resolvedPath];
28+
29+
// Clear any QUnit global from a previous test
30+
delete globalThis.QUnit;
31+
2832
return require(path);
2933
}
3034

3135
async function runQUnit (file) {
32-
// Resolve current QUnit path and remove it from the require cache
33-
// to avoid stacking the QUnit logs.
34-
let QUnit = requireFresh('../../qunit/qunit.js');
35-
36-
// Expose QUnit to the global scope to be seen on the other tests.
37-
global.QUnit = QUnit;
36+
const QUnit = requireFreshQUnit();
3837

3938
QUnit.config.autostart = false;
4039
await import('../../' + file);

src/cli/run.js

+4-8
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ async function run (args, options) {
4343

4444
const files = utils.getFilesFromArgs(args);
4545

46-
QUnit = requireQUnit();
46+
// Replace any previous instance, e.g. in watch mode
47+
QUnit = globalThis.QUnit = requireQUnit();
4748

4849
if (options.filter) {
4950
QUnit.config.filter = options.filter;
@@ -65,10 +66,6 @@ async function run (args, options) {
6566
console.log(`Running tests with seed: ${QUnit.config.seed}`);
6667
}
6768

68-
// TODO: Enable mode where QUnit is not auto-injected, but other setup is
69-
// still done automatically.
70-
global.QUnit = QUnit;
71-
7269
options.requires.forEach(requireFromCWD);
7370

7471
findReporter(options.reporter, QUnit.reporters).init(QUnit);
@@ -177,7 +174,7 @@ function abort (callback) {
177174
process.off('exit', onExit);
178175
running = false;
179176

180-
delete global.QUnit;
177+
delete globalThis.QUnit;
181178
QUnit = null;
182179
if (callback) {
183180
callback();
@@ -192,8 +189,7 @@ run.watch = function watch (args, options) {
192189
const watch = require('node-watch');
193190
const baseDir = process.cwd();
194191

195-
QUnit = requireQUnit();
196-
global.QUnit = QUnit;
192+
requireQUnit();
197193
options.requires.forEach(requireFromCWD);
198194

199195
// Include TypeScript when in use (automatically via require.extensions),

src/core/export.js

+25-14
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,29 @@
11
/* global module, exports */
22
import { window, document, globalThis } from './globals.js';
33

4+
/**
5+
* Available exports:
6+
*
7+
* globalThis:
8+
* - browser (globalThis === window)
9+
* - Web Worker (globalThis === self)
10+
* - Node.js
11+
* - SpiderMonkey (mozjs)
12+
* - Rhino 7.14+
13+
* - any other embedded JS engine
14+
*
15+
* CommonJS module.exports (commonjs2):
16+
* - Node.js
17+
*
18+
* CommonJS exports (commonjs, https://wiki.commonjs.org/wiki/Modules):
19+
* - Rhino
20+
*/
421
export default function exportQUnit (QUnit) {
5-
let exportedModule = false;
6-
722
if (window && document) {
823
// QUnit may be defined when it is preconfigured but then only QUnit and QUnit.config may be defined.
9-
if (window.QUnit && window.QUnit.version) {
24+
if (globalThis.QUnit && globalThis.QUnit.version) {
1025
throw new Error('QUnit has already been defined.');
1126
}
12-
13-
window.QUnit = QUnit;
14-
15-
exportedModule = true;
1627
}
1728

1829
// For Node.js
@@ -21,20 +32,20 @@ export default function exportQUnit (QUnit) {
2132

2233
// For consistency with CommonJS environments' exports
2334
module.exports.QUnit = QUnit;
24-
25-
exportedModule = true;
2635
}
2736

2837
// For CommonJS with exports, but without module.exports, like Rhino
2938
if (typeof exports !== 'undefined' && exports) {
3039
exports.QUnit = QUnit;
31-
32-
exportedModule = true;
3340
}
3441

35-
// For other environments, including Web Workers (globalThis === self),
36-
// SpiderMonkey (mozjs), and other embedded JavaScript engines
37-
if (!exportedModule) {
42+
// Ensure the global is available in all environments.
43+
//
44+
// For backward compatibility, we only enforce load-once in browsers above.
45+
// In other environments QUnit is accessible via import/require() and may
46+
// load multiple times. Callers may decide whether their secondary instance
47+
// should be global or not.
48+
if (!globalThis.QUnit || !globalThis.QUnit.version) {
3849
globalThis.QUnit = QUnit;
3950
}
4051
}

test/benchmark/micro.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* eslint-env node */
22

3-
global.QUnit = require('qunit');
3+
require('qunit');
44
require('./micro-fixture.js');
55
require('./micro-bench.js');

test/cli/fixtures/inception.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
const outerQUnit = global.QUnit;
2-
delete global.QUnit;
1+
const outerQUnit = globalThis.QUnit;
2+
delete globalThis.QUnit;
33
const myQUnit = require('../../../src/cli/require-qunit')();
4-
global.QUnit = outerQUnit;
4+
globalThis.QUnit = outerQUnit;
55

66
const data = [];
77
myQUnit.on('runStart', function () {

0 commit comments

Comments
 (0)