chore: publish ESM only

[remcohaszing]

What kind of change does this PR introduce?

• Bug Fix
• Feature
• Refactoring
• Style
• Build
• Chore
• Documentation
• CI

Did you add tests for your changes?

• Yes, my code is well tested
• Not relevant

If relevant, did you update the documentation?

• Yes, I've updated the documentation
• Not relevant

Summary
This change removes the build process of compiling the package to CJS, making the packages ESM only. Since ESM is only supported in more modern versions of Node.js, the minimal Node.js version requirement was increased to 18. This is reflected in the package.json engines field and GitHub action workflows.

The build process of downleveling the ESM code was also removed, as modern versions of Node.js support all syntax used. As a result, all build related dependencies could be removed.

There were some issues running the tests with the old version of Mocha used. Instead of updating Mocha, I replaced it with the builtin Node.js test runner.

Since new versions have issues consuming the old version 1 lockfile, it was deleted and a new version 3 lockfile was generated.

Does this PR introduce a breaking change?
Yes. The CJS build is no longer available

Other information
This change was discussed in #65 (comment)

A potential follow-up PR could generate the type definitions from the JSdoc annotations and type check everything.

Closes #29
Closes #63
Closes #64

[rolandjitsu]

@remcohaszing what happens to old clients that haven't upgraded yet to node 18+? E.g. this pkg is used in react-dropzone, and so, if we have some clients using older versions of node, will this pkg still be consumable (as far as I can tell, it should be ok)?

Also, this would be a breaking change, so the commit must be a feat and include a BREAKING CHANGE (see https://www.conventionalcommits.org/en/v1.0.0/).

[remcohaszing]

I picked Node.js 18 because that’s the latest Node.js version that’s not EOL. Of course that doesn’t mean you may not support older versions. Node.js 12 was the first version to add ESM support. This package doesn’t use really fancy modern JavaScript features, so that should just work.

There things that determine which Node.js version is supported are:

• The engines field in package.json. Package managers may warn or error if the current Node.js version doesn’t match the engines field. For my own projects I just don’t specify it.
• The readme and changelog (but it’s currently not documented)
• The Node.js versions you run tests with in GitHub actions. It’s up to you to decided if you want to run tests against all supported versions. Note that node:test was introduced in Node.js 16. If you want to test on older versions, we need a test runner that supports both Node.js 12 and ESM. There’s probably a Mocha version that supports it, but I would need to investigate that.

[rolandjitsu]

@remcohaszing I'm all up for modernisation.

The only caveat here is that users on old versions of node will have to upgrade to node 18+ if they want new features (assuming we'll land some in the future).

That means that react-dropzone will stick to the current version as it's engine is set to 10+ to maintain compatibility (actually, node doesn't really matter at runtime where react-dropzone is used).

Which is why I asked the question. Ideally, we should maintain compatibility with older versions, but if that's too difficult, we'll go ahead with a breaking change and bump the major.

[edemaine]

To me a bigger issue with moving to ESM only is that you cannot require ESM modules from CJS code. I thought this would be true in all versions of NodeJS, but it's apparently changing in the future.

I don't have a particular need for a CJS build of this package, but maybe someone else does? Anyway, up to the maintainer what's best here; just wanted to provide some context so you can make an informed decision.

[rolandjitsu]

I think we still need these 2.

[rolandjitsu]

Probably still need these 2 as well.

[rolandjitsu]

@remcohaszing do you think that we could, instead of completely dropping cjs support, just add support for esm in this PR? Or would that require too much work? We're currently trying that for react-dropzone.

I would definitely transition all dev deps to latest and get rid of mocha, but I'd really like to keep supporting older clients for now and wait a bit more before migrating to pure esm.

Otherwise, any fix would need to also go into older versions (a v 2.2.x maintenance branch).

[edemaine]

do you think that we could, instead of completely dropping cjs support, just add support for esm in this PR?

I guess like #64? Honestly, I don't even remember writing #64, and it seems to have an outstanding comment, but #64 looks pretty simple and so would the comment fix. I could go back to revising it if that would be helpful. But maybe there have been various other changes in the meantime. This PR (#66) also does other things, like switch from Mocha to Node's new testing harness.

[rolandjitsu]

@edemaine it's up to you. @remcohaszing what are your thoughts?

As for the other changes, I would pull that out into a separate PR.

[edemaine]

I've gone ahead and revised #64, which had minimal conflicts (only README) so I don't think it's outdated. If it's considered good, perhaps the other features of this PR could be built on top of that one? I just read #65 and see the original intent was to fix the CJS exports. That definitely seems worth doing, and isn't in #64.

Alternatively, if it's easier to redo this PR with dual CJS/ESM, that would seem like an improvement to me.

[rolandjitsu]

I've gone ahead and revised #64, which had minimal conflicts (only README) so I don't think it's outdated. If it's considered good, perhaps the other features of this PR could be built on top of that one? I just read #65 and see the original intent was to fix the CJS exports. That definitely seems worth doing, and isn't in #64.

Alternatively, if it's easier to redo this PR with dual CJS/ESM, that would seem like an improvement to me.

Thanks @edemaine . I'm trying to work on some changes locally as well to see what can be done. Note that in your PR, if you bump versions of pkgs, the lock file needs to be regenerated.

I'm just noticing now that there are a ton of dependencies that are out of date 😬 I've started bumping things, but some haven't been updated in years either ... so unless I go ahead and updated those, we'll have to stick with current versions.

There are also the issues around the package.lock that @remcohaszing mentioned ... I'm on node 20 and it's taking forever to install deps via npm with the current lock file, removing it seems to do the trick. But that probably means we'll need users to be on later versions of node.

We may end up dropping support for older node versions and going ahead with esm. Older clients can probably use some transpilers to be able to consume esm if they cannot yet.

[edemaine]

I've also failed to successfully run npm install (seems to hang forever). Thanks for the tip to delete the lock file!

Like I said, I'm not worried about supporting old Node versions. The issue is that modern Node versions still support CJS (and I doubt that will ever disappear), and packages that haven't migrated to ESM yet won't be able to use this anymore. I guess there's a trend to force people to update to ESM by publishing ESM only, but I'm not a fan personally. Of course, it's entirely up to you what you prefer!

[rolandjitsu]

@remcohaszing and @edemaine I've opened #75 where we do most of what's being done here and also ensures some of the other things that worked before still work (e.g. size limit check).

While I would have preferred to keep supporting CommonJS, in the long run, it makes more sense to migrate to ESM.

[jonkoops]

Let's close out this PR now that #75 was merged