Export chai as ES module

[sebamarynissen]

Since Node 13.2 --experimental-modules was unflagged. Since then mocha has enabled support for tests written as an ES module in v7.0.0-esm1.

This means that from now on it is possible to write your tests as

import chai from 'chai';

describe('foo', function() {
  it('bar', function() {
    chai.expect(true).to.be.true;
  });
});

Unfortunately it is not possible to write

import { expect } from 'chai';

because Node doesn't support named exports from CommonJS modules.

This PR changes this by exporting chai as an ES module and by making use of conditional exports, which was unflagged - but is still experimental - in Node 13.7.

I am aware that conditional exports is still experimental, and hence you'll still get a warning, both for

import { expect } from 'chai';
const { expect } = require('chai');

I therefore propose to release the conditional exports as an experimental version - similar to mocha v7.0.0-esm1 - but to include the chai.mjs file in a semver minor so that people can do

import { expect } from 'chai/lib/chai.mjs';

if they want.

[codecov]

Codecov Report

Merging #1317 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1317   +/-   ##
=======================================
  Coverage   94.61%   94.61%           
=======================================
  Files          33       33           
  Lines        1708     1708           
  Branches      416      416           
=======================================
  Hits         1616     1616           
  Misses         92       92

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e543bf...6287fe1. Read the comment docs.

[keithamus]

This looks somewhat sensible, as an interim measure. Someone else from @chaijs/core should take a second look at this, with a view to merge.

[sebamarynissen]

I think it should indeed be considered as an interim measure until chai has been fully migrated to ES modules - which I suppose will eventually be done once ES modules are no longer experimental in Node.

[MylesBorins]

This is exactly the pattern we recommend.while transitioning from CJS to ESM

https://nodejs.org/dist/latest-v13.x/docs/api/esm.html#esm_approach_1_use_an_es_module_wrapper

[chrisveness]

Thanks for this work – it would be good to see it merged & released.

[brettz9]

I would expect this could also potentially allow browsers to use chai via ESM without transpiling (using a direct node_modules path or an import map)--if necessary using "browser" or "browser-esm" within the conditional exports (the exact naming hasn't been standardized).

[keithamus]

This LGTM. @lucasfcosta or other @chaijs/core members want to approve+merge?

[giltayar]

DO NOT MERGE THIS PR, as you will disable deep-linking into chai. See #1340 (comment) for an explanation.

[sebamarynissen]

I'm closing this following #1340. I verified that indeed it does not work if you want to deep import chai:

const chai = require('chai/lib/chai');

@MylesBorins: Is this something that the modules team is aware of? I fear that a lot of modules will suffer from this problem when trying to migrate to ESM with conditional exports.

[GeoffreyBooth]

Is this something that the modules team is aware of? I fear that a lot of modules will suffer from this problem when trying to migrate to ESM with conditional exports.

Yes, this is by design. With "exports" you need to explicitly define what deep imports you want to be exported: https://nodejs.org/api/esm.html#esm_package_entry_points:

Warning: Introducing the "exports" field prevents consumers of a package from using any entry points that are not defined, including the package.json (e.g. require('your-package/package.json'). This will likely be a breaking change.

The docs go into greater detail, but basically you're encouraged to define what the package's actual supported entry points are; or if you truly want to allow access to any file, you can add "./": "./".

[sebamarynissen]

Allright, so if I understand correctly the best way to go forward with this is to only publish it in a semver major, perhaps with some explicit additional entry points. Correct?

[MylesBorins]

@sebamarynissen if you included "./": "./" mapping the root to the root it will be a semver-minor change

[MylesBorins]

Suggested change

	"exports": {
	"import": "./lib/chai.mjs",
	"default": "./index.js"
	},
	"exports": {
	".": {
	"import": "./lib/chai.mjs",
	"default": "./index.js"
	},
	"./": "./"
	},

[sebamarynissen]

Ok, but perhaps that we should go with #1340 then as it adds the same functionality, handles the deep imports problem and contains tests for it.