Not strict mode safe

[ef4]

As distributed on NPM, qunit is not strict-mode safe. It relies on global this.

The reason this matters is that people are increasingly relying on the browser's native module loader to pull in all dependencies during development (snowpack, vite). These tools can wrap the vast majority of NPM packages up as modules to achieve interoperability, but they can't readily escape the strict mode requirement imposed by modules.

Strict mode has been javascript best practice for twelve years now. All qunit's own source is already authored under strict mode. It's only the published build that relies on non-strict behavior.

This issue could be resolved by publishing an ESM build as discussed in #1551, but it could also be resolved with a smaller change by switching to a different technique for locating globalThis.

globalThis is now a stable (stage4) ECMAScript feature, and there are broadly-compatible polyfills for it that take care of cross-engine issues.

[Krinkle]

Yikes, that should indeed just work as expected. E.g. because it's easier that way for a particular way of running tests, or even if by accident. There's no reason not to support it and it's easy to do so indeed.

@ef4 Could you link to an example of how you run tests in a browser such that qunit itself ends up processed by a bundler? I'm curious to see how and why this is done, learning that would enable me to better test for and support that in the future, and could inspire ecosystem improvements to reduce manual setup for new projects. I ask because we usually recommend just dropping qunit as-is into a test page or test runner, typically right before importing your application code, dependencies/polyfills, and test suites (which may be bundled, if needed or helpful.)

I've tagged this as help-welcome; feel free to submit a PR. Otherwise, I'll probably patch it for you during the next release prep in a week or two. Thanks for reporting!

[ef4]

I kinda want to flip the question around and ask what this really means to you:

just dropping qunit as-is into a test page

because the QUnit docs are silent on the topic of how to consume QUnit in the browser from NPM.

I can certainly write some manual transformation that will do require.resolve('qunit') to get the true path to the published qunit.js, mount that path into a dev webserver, and insert a script tag that points at it. But that's an awful lot of special handling for a dependency that really doesn't need to be any different from the dozens of other dependencies in the project, all of which are consumed via import or require, which is the normal way to consume NPM packages and which requires no manual setup.

It comes down to what other commenters said already in #1551. Getting qunit into the page via an import is preferable to a script tag because it's just better aligned with how NPM works. NPM packages don't naturally expose URLs, they only offer require, require.resolve, and (more recently) import. They're much easier to consume from Javascript than from HTML.

My specific use case is that I'm working on making it possible to build Ember apps with a variety of different build tools, including webpack and snowpack. All Ember apps default to using qunit and their test blueprints import it, like here. How that is implemented has shifted over time, and in current stable releases qunit is running through webpack and that works OK. It was my attempt to run it through snowpack that inspired this issue, because snowpack uses entirely browser-native ES modules, and so tries to wrap QUnit in an ES module.

[Krinkle]

I kinda want to flip the question around and ask what this really means to you:

just dropping qunit as-is into a test page

because the QUnit docs are silent on the topic of how to consume QUnit in the browser from NPM.

I understand that the context you're in needs a different answer, but in general whether the library is downloaded from npm is in my opinion orthogonal to how a test suite is written or run with a browser.

When debugging, or using a runner that takes an HTML file and static directory and serves it up and listens for events from a test framework, then you add qunit to that HTML file the same way as you would your main application and test files (or if they're bundled, then you load it from the same place as you load your bundle). For example:

<!DOCTYPE html>
<link rel="stylesheet" href="node_modules/qunit/qunit/qunit.css">
<div id="qunit"></div>
<script src="node_modules/qunit/qunit.js"></script>
<script src="dist/bundle.js"></script>

This is documented in the Intro (though we should probably default the example to node_modules instead of recommending use of the CDN, which feels pretty weird looking at it now). This is all you need to do load QUnit when using qunit-puppeteer, grunt-contrib-qunit, gulp-qunit, testee, testem, browserstack-runner, or other runners that take a static directory or URL.

For some test runners, it is possible (or required) to let the HTML be managed for you. These tend to have a config file where you list the JS file paths. You already specify the path to your bundle there, and if not done automatically, one would also specify the path to qunit there.

For example, Karma takes an array of files (example, docs) that it will serve from a static server, and materialize in the browser on your behalf as a <script> tag, including support for preprocessors such as Istanbul coverage, and rollup/babel/typescript transformations etc. In the case of Karma you don't need to specify qunit.js in this list, since the karma-qunit plugin that listens to the results for you also injects qunit for you as the first file.

I can certainly write some manual transformation that will do require.resolve('qunit') to get the true path to the published qunit.js, mount that path into a dev webserver, and insert a script tag that points at it. But that's an awful lot of special handling for a dependency that really doesn't need to be any different from the dozens of other dependencies in the project, all of which are consumed via import or require, which is the normal way […]

This is the kind of code a bundler like Rollup, or framework like Karma might have internally. As individual project using QUnit that should never be needed indeed. I would expect that whatever minimal configuration is needed to tell the test runner where your bundle is, or where your code and test suites are, would also be where you can tell it where qunit.js is. (Similar for Mocha or Jasmine, I would expect.) In my experience, such runners don't have a need to expand these into URLs or remap them, but if that's the case then that would presumably work the same way for qunit.js as for bundle.js or tests.js.

[…] special handling for a dependency that really doesn't need to be any different from the dozens of other dependencies in the project, all of which are consumed via import or require,

This is a new perspective for me, so bear with me for a minute. I think it's different from the other dependencies because it isn't a library that your code or your tests depend on to call utility methods from. QUnit is managing the test execution itself, as an application. It also exposes a singleton API as QUnit for test suites to register tests with indeed. The application should be loaded before the code and tests so that it can reliably handle script errors and provide an event stream to CI or your local test runner. Having it imported manually by you I think only adds extra work for you and your bundler, and begs the question where or which file will "do" the importing? The first test suite? What's the first test suite? Or import in all of them just in case? Or have a wrapper script that imports it and then manually imports each test? If the bundle ends up containing a syntax error this would mean QUnit might be unable to start the client-server communication. That might be okay if you have another client-server layer on top of this that does get injected as its own separate script.

In my view, importing it is more likely to cause unwanted side-effects, and generally more effort to set up and maintain - compared to including it side-by-side with your own code as standalone app. Having said that, I'm sure my preferred approach has other limitations that can't think of right now.

My own preferences aside, QUnit is simple and flexible and as a project it should not be opinionated on things like this. Whichever approach you take, it should Just Work ™, out-of-the-box, and do the best it can do within the strengths and limitations native to that approach.

I understand from @rwjblue 's review that this is a regression in 2.14, which makes it doubly bad. I'll cut patch release later this week. Thanks!

[ef4]

Thank you for merging and releasing.

I can give you a concrete list of things that fail to work out-of-the-box when QUnit must be consumed via <script> instead of as a module import:

• linters like eslint need to be manually configured to understand where the globals are coming from
• in typescript you are forced to either split your project into separate configs or accept that the global types will pollute non test code
• when using rollup or webpack, app authors are required to add a new entrypoint to their build configuration in order to generate a URL for qunit, and manually add a script tag for it to their HTML.
• users of rollup also need to install the CJS plugin, despite the fact that QUnit is authored as ES modules -- we're going through an unnecessary translation from ES modules to CJS back to ES modules.
• when using snowpack, there is, as far as i can tell, no public API way to get a standalone URL that points at a dependency like QUnit.

Each of these issues becomes a non-issue when you use ES modules instead. You just get to delete all the config mentioned above.

I think it's different from the other dependencies because it isn't a library that your code or your tests depend on to call utility methods from. QUnit is managing the test execution itself, as an application.

As a core contributor to one of the most frameworky frameworks out there (Ember), this is not an alien concept to me. But we have found that it's better to give users access to framework methods via imports than via globals. There is far better tooling support for the imports.

It really doesn't matter who is ultimately at the top of the callstack. At the point where the user's code is calling some part of QUnit, that is a function call across a package boundary, and that is what ES imports are designed for.

[Krinkle]

I can give you a concrete list of things that fail to work out-of-the-box when QUnit must be consumed via <script> instead of as a module import:

Thanks, these definitely help see the balanced perspective.

Basically, for linting I think listing the QUnit global in the test directory is not very different from enabling Web APIs in a /src/client directory, or Node APIs in a /src/server/ directory etc. In practice, I think one would benefit from going a step further and use eslint-plugin-qunit which would need to be enabled from the eslint config for the test directory, and takes cares of the global automatically. TypeScript definitions are also available by default for QUnit (not yet bundled, but will be in the future).

I won't respond in detail to the other points, other than to say I don't think there's any need to build, transform, or create endpoints of any kind for QUnit. It is already distributed as ready-for-use application. You only need one build for your own code (if your code requires building), and in the HTML file or abstraction thereof where you point to your artefact, you also point to node_modules/qunit (or if you use Karma, or QUnit CLI, then this is done for you). In projects where code does require building, I would generally prefer to build the source only, not using the tests as entry point. The tests can use the native approaches of the web or Node respectively to import the source code and test it. This is also the model Tape and node-tap use, which I'm quite fond of.

All in all, I think our approaches both involve some amount of trade-off and added cost which we may consider significant or negligible depending on our familiarity and available automation. Both offer a range of benefits, and we choose differently. No problem with that! I'm glad this little bug makes that work again, and I do understand this represents a notable part of the ecosystem.

[ef4]

you also point to node_modules/qunit

You keep saying that, but that's not actually correct. There are many scenarios, including ones that use only stock NPM, where that breaks. For example, NPM 7 now supports workspaces, so NPM may unilaterally choose to move qunit from node_modules/qunit to... somewhere else. All well-behaved code is supposed to tolerate that by not assuming they know where NPM will put it, but rather going through NPM's only supported APIs for accessing a dependency: require.resolve, require, or import.

[NullVoxPopuli]

can we re-open this? this is pretty important.

We don't need to be supporting legacy environments, and we can ship ESM native packages now (which are strict mode by default, which is a feature of ESM) that would still be compatible everywhere.

I'd like to split the qunit package as well.

• browser only modules (the `qunit' package)
• cli, (maybe @qunit/cli)