Qunit v2 has incorrectly configured exports

[NullVoxPopuli]

Tell us about your runtime:

• QUnit version: v2
• Which environment are you using? (e.g., browser, Node): browser
• How are you running QUnit? (e.g., QUnit CLI, Grunt, Karma, manually in browser): jsbin

Repro: https://jsbin.com/fipayiy/edit?html,output

What are you trying to do?

import { module, test } from 'qunit'

but this is how you have to use module / test:

import QUnit from 'qunit';

// this is needed because QUnit didn't properly configure their exports.
// they _only_ have a default export.
const { module, test } = QUnit;

Repro: https://jsbin.com/fipayiy/edit?html,output

[Krinkle]

The jsbin example uses https://esm.sh. I'm not familiar with that particular transformer, but based on how rollup and other transformers work, and from a quick glance at the ?dev output at https://esm.sh/v133/[email protected]/X-a24/es2022/qunit.development.mjs, it seems esm.sh wraps the module in a closure with a placeholder module.exports object, and then exports this as native ESM export. It seems like all the ingredients are there for this to work, but it currently does not.

I see that esm.sh documents a workaround in the form of https://esm.sh/#specify-cjs-exports. For example:

import { test, module } from 'https://esm.sh/[email protected]?cjs-exports=test,module';

module(…
  test(…

Assuming there is a way esm.sh can understand CJS exports automatically, I'd love to make this work in an upcoming QUnit 2.x release. We can debug the above and figure out what it's doing in the __toESM() and __reExport() functions and where it's getting confused. I may get around to it for the next release, but patches will definitely be welcomed for this!

I note that QUnit does not currently provide its own ESM release (yet). QUnit does currently contain the following:

    if (window) {
      window.QUnit = QUnit;
      // […]
    if (typeof module !== 'undefined' && module && module.exports) {
      module.exports = QUnit;
      module.exports.QUnit = QUnit;
      // […]

The intent of this is to allow developers to choose their preferred style. You can import individual methods, or you can import the API as a whole. And the latter is possible both as default and as explicitly named QUnit import.

The design of QUnit is generally such that individual test files do not need to be written with knowledge or hardcoded awareness of where or how QUnit was loaded. (Afaik most test runners will have already loaded and configured QUnit before the first test file test file loads, and made the API available as global variable. So it'd only be looking to regain access to the cached import at this point.) Thus, assuming the test runner has loaded QUnit for you already, you can (usually) do the following:

const { module, test } = QUnit;

module(…
  test(…

Or, directly:

QUnit.module(…
  QUnit.test(…

QUnit does support being imported in every test file, and that worked under CJS, so it'd be nice for that to work under ESM as well. I wonder if that is still needed today though? I'd love to understand whether that's by choice or whether there's a specific runtime requirement that forces this habbit.

The most common case where this comes up is when you bootstrap your own standalone QUnit process. For example, using a generic framework-agnostic test runner like airtap/browserify. Or when you use Node.js/SpiderMonkey/Deno plainly (e.g. without QUnit CLI); then you do indeed need to load QUnit at least once on your index or entry point script. That entry point could use import 'qunit'; without assigned variable in a browser, or import QUnit from 'qunit'; on the server. If you bootstrap your own process, there is presumably a need to interact with the QUnit API more generally, at least to set up a reporter. That is, I assume the destructured import is something we generally want for ergonomics in test files, but not test runners.

[NullVoxPopuli]

thanks for the deep investigation!

however, I don't think we want to focus on the specifics of what dev.sh is doing, as I can demonstrate the same problem in ESM node, (which you can't see how it handles translating CJS, except that there is a spec for this)

Here is a stackblitz, similar to my jsbin, which uses type=module in the package.json:

https://stackblitz.com/edit/stackblitz-starters-j2kwo2?

in test/add.js:

// This is what folks using ESM expect
// But, are given:
// SyntaxError: Named export 'module' not found. The requested module 'qunit' is a CommonJS module, which may not support all module.exports as named exports.
// import { module, test } from 'qunit';

// Works, but is not ideal, and not what folks using ESM expect to do.
import Qunit from 'qunit';
const { module, test } = Qunit;

import { add } from '../src/add.js';

module('add', function () {
  test('can add valid inputs', function (assert) {
    assert.strictEqual(add(1, 3), 4);
  });
});

The design of QUnit is generally such that individual test files do not need to be written with knowledge or hardcoded awareness of where or how QUnit was loaded

I think that's fine as a goal, but that's not what the expectations of ESM are. Nearly all tooling in the ecosystem now operates under the assumption that you can easily go-to-definition from an identifier and eventually got to where the thing is defined. If QUnit is a global, or generally just not imported, you're not going to be able to have this ergonomics table stakes.

you can (usually) do the following
Or, directly

but that's not how folks (in my circle of the web anyway) want to use things in ESM.
when you have tens of thousands of tests, you want explicit references (imports), and import exactly what you need (usually) -- at least in browser uses.
In node, what is imported matters way less, so there is no reflexive import-only-what-you-need response there, because you don't care about how the module graph is created as you would in the browser.

I note that QUnit does not currently provide its own ESM release (yet). QUnit does currently contain the following:

I see two paths forward here to meet all goals stated in your reply (again, thanks for the super thorough investigation!)

1. Since import { QUnit } from 'qunit' works, due to module.exports.QUnit = QUnit; (which is how the spec declares ESM should work with CJS), we could add module, test, skip, etc to `module.exports as well. This would probably be the least-effort way to fix the problem.
2. I don't personally hold a lot of value in one file trying to support all formats -- I much prefer utilizing package.json#exports to set up cjs/esm separately, as separate bundles -- so long term, if cjs compatibility is desired to be kept, this would be the path I would pursue.

thank you!

[NullVoxPopuli]

I took a stab at a PR over here: #1728,

but I don't yet have a way to test browser-ESM due to constraints of the test setup
(the PR would not resolve browser ESM)

[Krinkle]

Note to self: understand this commit and validate what we do against it.

emberjs/ember-qunit@099da91