Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build: Add native ESM distribution #1798

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Build: Add native ESM distribution #1798

merged 1 commit into from
Sep 4, 2024

Conversation

Krinkle
Copy link
Member

@Krinkle Krinkle commented Aug 27, 2024

Source

  • Convert /src/core/qunit.js to ESM export.

    This file can be used directly by ESM in theory when debugging, but in practice we'll let rollup bundle it into qunit.module.js, which is otherwise functionally the same as the source file.

  • Add /src/core/qunit-commonjs.js as second entrypoint for rollup.

    Rollup would complain if attempting to export the ESM qunit.js source as "iife", as Rollup will detect the input file as having exports and insist on taking control over exporting it to a global variable. We don't want that as we control that ourselves instead.

    qunit-commonjs.js is a thin entrypoint that generates the qunit.js distribution file, which is the same as qunit.module.js except in "iife" format, with our (conditional) module.exports assignment at the end.

Build

  • /qunit/qunit.js is the same ES5/CJS compatible distribution it has always been, now sourced from /src/core/qunit-commonjs.js.

  • /qunit/qunit.module.js is a complete standalone distribution in ESM format. Given that IE11 doesn't support type=module, I've removed IE11 from the Babel config for this file, which makes the file a bit smaller as it uses most ES6 features natively (275K vs 250K; uncompressed, unminified).

Package

If we simply advertised the new /qunit/qunit.module.js file in package.json#exports.module, then we would break numerous Node.js and bundler use cases.

To test for these, previous commits already added /demos/bunders/, which is tested via node bin/qunit.js demos/bundlers.js, as part of npm run test-demos (and the "CI: Demos" GitHub job).

Specifically:

  • A Node.js project that uses a mixture of CJS require('qunit') and ESM import 'qunit' previously shared a single logical QUnit instance since the module was just one file, and thus both used the same internal "require"-cache. This means the user's test files, or any test runners, plugins, etc, all import/require the same qunit module to define assertion plugins, add a hook listeners, or register tests. And thus everything executes correctly on the main QUnit instance.

    As-is this would break by having some of these attach stuff to a secondary instance that the main test runner (e.g. QUnit CLI) isn't running.

  • A frontend project that uses a bundler or adapter (e.g. karma-qunit), and similarly involves mixed use among its test files and/or between it and any plugins, that sends the result to a browser. As-is this would end up embedding two separate copies of QUnit, unaware of each other.

To mitigate this, package.json contains two overrides:

  • for node, tell ESM mode to import a Node.js-specific wrapper that re-uses and re-exports the CJS file.
  • for bundler, tell CJS mode to import a bundler-specific wrapper that re-uses and re-exports the ESM file.

This is based on https://github.com/jquery/jquery/wiki/jQuery-4-exports-explainer as implemented at jquery/jquery#5429, which does the same for jQuery.

Fixes #1551.
Fixes #1724.

@Krinkle Krinkle force-pushed the esm-dist branch 2 times, most recently from f3f6c02 to 17ce8ca Compare August 28, 2024 02:11
Krinkle added a commit that referenced this pull request Aug 28, 2024
Extracted from #1798 to minimize
the diff.
@Krinkle Krinkle force-pushed the esm-dist branch 3 times, most recently from 57adbb8 to e18fedb Compare August 28, 2024 02:41
@Krinkle Krinkle added this to the 3.0 release milestone Aug 28, 2024
@mgol
Copy link
Member

mgol commented Sep 2, 2024

A frontend project that uses a bundler or adapter (e.g. karma-qunit), and similarly involves mixed use among its test files and/or between it and any plugins, that sends the result to a browser. As-is this would end up embedding two separate copies of QUnit, unaware of each other.

This is a bit more convoluted. Bundlers, when the module condition is used without splitting it to import vs. require conditions, both require & import in source files will use the same file - bundlers can do it as they are preprocessors, they are not concerned with some operations being async vs. sync, so they are generally fine with require('./esm-file.js') & import ... from 'commonjs-file.js'.

However, the interop of module.exports treated as a module object vs. it exposing a single API and how it corresponds to a default export in ESM is problematic and having better control over it requires us to provide separate versions for import & require. As soon as we decide that, we need to use the wrapper approach to avoid code/data duplication.

Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments but nothing blocking

package.json Outdated Show resolved Hide resolved
src/cli/require-qunit.js Show resolved Hide resolved
src/core/qunit-wrapper-nodejs-module.js Outdated Show resolved Hide resolved
Krinkle added a commit that referenced this pull request Sep 4, 2024
Krinkle added a commit that referenced this pull request Sep 4, 2024
@Krinkle Krinkle force-pushed the esm-dist branch 2 times, most recently from 9a020ee to 5ad054c Compare September 4, 2024 03:17
@Krinkle Krinkle merged commit ec7d6e7 into main Sep 4, 2024
21 checks passed
@Krinkle Krinkle deleted the esm-dist branch September 4, 2024 03:47
@matthewp
Copy link

This is amazing 🎇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Qunit v2 has incorrectly configured exports ESM build
6 participants