Adds es modules support and tests

[orta]

Deprecates #121 #119 #87

This PR:

• Adds ES Modules support
• Adds tests for some of the most popular ways this lib is consumed (node cjs, node esm, rollup treeshaked, webpack treeshaked) to verify they work
• Keeps all backwards compatibility, without duplicating code
• Works with CommonJS and ES Modules

[guybedford]

This looks great to me, the ESM wrapper sounds like a sensible approach as well.

[guybedford]

This pattern definitely works if the .js extension is important for both. I take it that is the constraint you're working too, even if .mjs would be slightly simpler.

[Sweetog]

Curious, what was the point of adding this almost empty package.json with no package name, no package version?

Sincerely,
ES Module Bundlers

[Andarist]

https://nodejs.org/api/packages.html#type

The "type" field defines the module format that Node.js uses for all .js files that have that package.json file as their nearest parent.

[Sweetog]

Hmm, is it best to comprehensively interpret this spec like this though? To just drop a nested, nearly empty package.json in a distribution directory, when thinking about all the nuances that go into an NPM package and bundling/transpiling packages?

And how is an NPM package like pkg-dir expected to behave now, in it's claim to:

Find the root directory of a Node.js project or npm package

The above pkg-dir behavior will fail to resolve the root of the NPM package for tslib after an ESM bundler follows to path ./imports/index.js declared in package.json#exports['.'].imports and then executes pkg-dir with a cwd of: node_modules/tslib/imports/index.js

It seems the case that NOT using the .mjs extension was a shortcut and may be more the problem here

[Andarist]

Hmm, is it best to comprehensively interpret this spec like this though?

The spec is quite intentionally written like this as far as I know. It intentionally doesn't use something like "package.json at the root of the package directory" multiple times

To just drop a nested, nearly empty package.json in a distribution directory, when thinking about all the nuances that go into an NPM package and bundling/transpiling packages?

Even before node has supported .mjs, .cjs, package.json#type and more, it was already possible to define nested package.json files. It was possible to put package.json#main there to "redirect" the request to another location. I've been using this for years already.

And how is an NPM package like pkg-dir expected to behave now, in it's claim to:

I'm not sure if this package is implemented correctly but a lot of similar resolvers were implemented correctly in the past. All modern module bundlers etc already work just OK with those nested package.json files. The rule, in fact, is quite simple if you want to just grab the root of a pkg, arguably you wouldn't even have to look for package.json files but rather just find a directory nested directly within node_modules directory

[guybedford]

The tests seem like they will add some maintenance burden though - it does seem like overkill here to me.

[orta]

I agree the tests are definitely overkill for this specific change.

However, this entire lib has no tests and I'd rather make it easier for the next change like this 👍🏻

[Andarist]

Please provide "module" condition as well. It's going to be supported by the upcoming webpack 5 and will allow it to keep only a single copy of tslib in the bundle. Point of reference: https://gist.github.com/sokra/e032a0f17c1721c71cfced6f14516c62#reference-syntax

[orta]

I'm open to covering this in a new PR, that gist isn't really enough docs for me to go with yet

[Andarist]

Sure, can be added afterwards.

There are no real docs for this yet on their page but this gist has been updated multiple times when they have been iterating over semantics. This is already implemented and shipped in webpack 5 RC.

[guybedford]

@Andarist do you have any examples of packages using the "module" condition successfully for reference here?

[Andarist]

@guybedford I did not (mainly because conditional exports themselves are rather rare to find so far), but I've decided to showcase this using one of my smaller modules. Here are exports that I have defined:
https://github.com/Andarist/callbag-last-element/blob/07099fd55b4dfff80a7697065c71ce9b5fc07ade/package.json#L7-L14
and here is the webpack output:
https://github.com/Andarist/webpack-module-condition-test/tree/7bca3a5ae4f07b5429c7b36db3d43b2072b39cf2/src
Notice how last function is only there a single time in this bundle even when mixing ESM and CJS files.

The input for this is as simple as:

// cjs-file.js
module.exports = require("callbag-last-element").default;

// index.js
import lastElement from "callbag-last-element";
import lastElementFromCjs from "./cjs-file";

console.log({ lastElement, lastElementFromCjs });

and can be found here (same repo)

[guybedford]

Thanks again to diving into this.

Please can we make sure to backport this to the 1.x branch as well as that still has significant usage for packages which assume ES module support in bundlers.

[rbuckton]

Not required, but I'd be tempted to say you should move the "dependencies" from https://github.com/microsoft/tslib/pull/126/files#diff-f1e4194bc14f4a4ca169da388d9135de to each individual test so they run in isolation.

[orta]

Aye, normally this is something I'd just throw into a yarn workspace and forget, but I'll make them all conform to a similar scripts structure then just loop through them