feat: add ESM support for named exports

[jerome-benoit]

ESM import with direct access to CommonJS files is still not supported.

Changelog:

• Fix CommonJS Set export collision
• Fix missing *Vector exports
• Fix missing *Heap exports

TODO:

• Document import usage

Closes #169 Closes #218

[jerome-benoit]

I think the CommonJS export for set ops is not correct and will collide with Set. And the site documentation is not reflecting the library usage for ESM user.

[Yomguithereal]

@jerome-benoit I like your PR but I will need some kind of proof it does not break backwards compatibility whatsoever. I attempted such a change on another library of mine lately, it broke a lot of things and people were angry at me (which they should not feel entitled to, but this is how the Internet works it seems). What's more, this library is very heavily depended upon.

An issue that arised for instance was that the obliterator library is not ESM-first either and that some package managers seem to consider ESM-first as "viral" where all the dependency chain must abide or break.

It also broke some older TS for instance.

I think the CommonJS export for set ops is not correct and will collide with Set.

What do you mean?

[jerome-benoit]

I think the CommonJS export for set ops is not correct and will collide with Set.

What do you mean?

module.exports = {
  ...
  Set: require('./set.js'),
  ...
}

export Set symbol to access the named exports in set.js. Are you sure it will just extend the node.js Set without causing any issue in module using both?

[jerome-benoit]

@jerome-benoit I like your PR but I will need some kind of proof it does not break backwards compatibility whatsoever. I attempted such a change on another library of mine lately, it broke a lot of things and people were angry at me (which they should not feel entitled to, but this is how the Internet works it seems). What's more, this library is very heavily depended upon.

An issue that arised for instance was that the obliterator library is not ESM-first either and that some package managers seem to consider ESM-first as "viral" where all the dependency chain must abide or break.

It also broke some older TS for instance.

Did you do it as part of a breaking changes version bump with some pre-releases? That PR is meant for such a release cycle since it's a breaking change.

Testing CommonJS, ESM default exports files will be added in that PR, but I prefer to early show the code to project maintainers. TS tests are overkill.

The changes done are the only way to achieve node.js support for CommonJS and ESM without breaking backward compatibility.

• top-level package.json main and types => node.js < 16 or node.js >= 16 and TS with Node10 module resolution are searching for it without "type": "module" and works.
• top-level package.json exports object => node.js >= 16 and TS with Node16 module resolution are searching for it with "type": "module" and will behave correctly as long as types, require and import are properly set.

Basically mnemonist miss the second point and import pointing to ESM wrapper. That's why I have issue after switching a TS repo using it to ESM and Node16 module resolution by default.

Anyhow, at some points, you will have to drop support for unsupported node.js version in your librairies and make a major version bump.

[Yomguithereal]

Hello again @jerome-benoit

Did you do it as part of a breaking changes version bump with some pre-releases?

Of course. But some people just want to watch the world burn regarding their way to handle version dependencies :).

Regarding this PR, this would be part of v0.40.0 of course.

export Set symbol to access the named exports in set.js. Are you sure it will just extend the node.js Set without causing any issue in module using both?

This seems to be an error on my end. The other endpoint correctly exports as lowercased set, which is of course better. Since we are breaking with this release, I vouch to fix this error in the common js endpoint.

Testing CommonJS, ESM default exports files will be added in that PR, but I prefer to early show the code to project maintainers. TS tests are overkill.

I am willing to go forward with this, so if you know how to add those tests and are willing to do so, let's proceed?

Regarding ESM-first virality, can you confirm that the fact that obliterator does not support ESM will not be an issue here?

Regarding the fact that single files will not be able to be imported using ESM, I guess this is not an issue since tree-shaking should now be sufficiently performant (it was not the case when this library was first developped). Which means the documentation will probably need to be updated wrt imports (which is totally fine with me).

[jerome-benoit]

Testing CommonJS, ESM default exports files will be added in that PR, but I prefer to early show the code to project maintainers. TS tests are overkill.

I am willing to go forward with this, so if you know how to add those tests and are willing to do so, let's proceed?

I'm a bit busy lately, but feel free to code on that branch until I'm able to finish the PR.

Regarding ESM-first virality, can you confirm that the fact that obliterator does not support ESM will not be an issue here?

As long as you do not switch that repo to ESM ("type": "module" in package.json), no.

Regarding the fact that single files will not be able to be imported using ESM, I guess this is not an issue since tree-shaking should now be sufficiently performant (it was not the case when this library was first developped). Which means the documentation will probably need to be updated wrt imports (which is totally fine with me).

ESM module can consume CommonJS most of the time but some configurations will not work, like enforcing Node16 module resolution.

[jerome-benoit]

Testing CommonJS, ESM default exports files will be added in that PR, but I prefer to early show the code to project maintainers. TS tests are overkill.

I am willing to go forward with this, so if you know how to add those tests and are willing to do so, let's proceed?

I'm a bit busy lately, but feel free to code on that branch until I'm able to finish the PR.

@Yomguithereal: The PR is now in mergeable state. The TODO bullet points are targeted to other PRs.

[Yomguithereal]

Thanks @jerome-benoit, I will review your PR and probably merge it as-is in the near future. I am a bit overwhelmed by work currently so don't worry if I disappear here and then.

[jerome-benoit]

@Yomguithereal: is it possible to schedule a 0.40.0-0 prerelease with that PR in? It's not time consuming for a maintainer and harmless for libraries end-users while allowing further testing.

[Yomguithereal]

@jerome-benoit I have committed a merge with current master. Will merge your PR soon and publish a release candidate on npm. Should be in the afternoon, or beginning of next week.

[Yomguithereal]

@jerome-benoit v0.40.0-rc1 is live on npm. I am wondering whether test/exports/package-lock.json should be committed to the repo or not.

[jerome-benoit]

@jerome-benoit v0.40.0-rc1 is live on npm.

Will test it soon to see if now bundlers works as intended in an ESM project with mnemonist as an external dep.

I am wondering whether test/exports/package-lock.json should be committed to the repo or not.

First I've not commited it. Then I thought having a strict versioning for deps in test would provide better reproducibility. But I do not think minor or patch version only updates done under the hood would harm. Fixing deps for non major version and a testing purpose is debatable anyways.

[jerome-benoit]

@jerome-benoit v0.40.0-rc1 is live on npm.

Will test it soon to see if now bundlers works as intended in an ESM project with mnemonist as an external dep.

Tested successfully on various repos using mnemonist.

Maybe you can announce the prerelease on GitHub to give it more visibility and testing before promoting the rc(s) as final.

[Yomguithereal]

Maybe you can announce the prerelease on GitHub to give it more visibility and testing before promoting the rc(s) as final.

Would you do this as a tag+issue? I am not entirely convinced a lot of people use mnemonist directly but rather as part of a dependency to their dependencies. I can try tweeting/tooting etc. also.

[jerome-benoit]

Maybe you can announce the prerelease on GitHub to give it more visibility and testing before promoting the rc(s) as final.

Would you do this as a tag+issue?

An issue? GitHub allows to create a release flagged as a prerelease associated with a git tag. That will work if the repo GitHub actions does not include a releasing workflow based on tag addition since the npm package is already pushed on the registry. I do not know how you add a new release on GitHub, but that's the same except with the prerelease checkbox on.

I am not entirely convinced a lot of people use mnemonist directly but rather as part of a dependency to their dependencies. I can try tweeting/tooting etc. also.

Sure, but I think having a link to the GitHub prerelease announcement with the changelog will help you to communicate a bit on it.