feat: Add ESM support while maintaining CJS support

[andrew-ifrita]

Use tsup to support both ESM and CJS.

This is effectively replacing #455 which has gone stale.

No logic or tests have been touched.

Motivation

Add both ESM and CJS support in an attempt to modernize the library and open up to more users.

Key Changes

• build script is now tsup src/index.ts --format esm,cjs --dts
• All import statements were changes to include a file extension and explicit file
• Replaced clean script rm -rf with rimraf (dev dep) which is more cross platform
• explicitly add reflect-metadata as a dev dep in order for e2e tests to pass
  • it is a peer dependency that was slipping by previously
• added .nvmrc file to use the same node version that CI is using
• moved chopstick testing dependencies into "devDependencies"

Main files to review

• package.json
• tsconfig.json

[andrew-ifrita]

Two questions from my side:

1. Should I also update the CHANGELOG here?
2. If this is successfully merged, does this deserve a release? If so, is it purely a minor release since no API actually changed?

[TarikGul]

LGTM, 1 question - I couldn't find where the new reflect-metadata package was being used?

[TarikGul]

Two questions from my side:

1. Should I also update the CHANGELOG here?
2. If this is successfully merged, does this deserve a release? If so, is it purely a minor release since no API actually changed?

1. For releases we usually update the CHANGELOG at the time of the release, but to make it easier you can definitely add it in this PR if you would like. I think if we started doing that in general it would save us time during the release process.

2. Yea it would just be a minor release since its adding ESM support.

[andrew-ifrita]

@TarikGul

1. For releases we usually update the CHANGELOG at the time of the release, but to make it easier you can definitely add it in this PR if you would like.

I will keep as is for now then.

I couldn't find where the new reflect-metadata package was being used?

It is being used in the test:e2e suite. You can see here in the failing CI. It was reproducible locally as well. It was a peer dependency of typeorm which is a dep of chopsticks which is used in the e2e testing I believe. Unrelated but I'll move chopsticks and the other pure dev/testing dependencies into "devDependencies" in a follow up MR

edit: I fixed the dev dependencies in this PR

[ryanleecode]

why follow up can just make the change now.

also latest node lts is 22 not 18 and generally corepack is preferred over nvmrc

[andrew-ifrita]

@ryanleecode

also latest node lts is 22 not 18

Effectively to keep changes minimal. package.json lists node 18. CI is currently using node 18. If we update the min supported node version, it should happen explicitly in its own PR and not be hidden with other changes, especially when PRs squash commits. I also didn't want to unravel any CI issues that it might bring.

and generally corepack is preferred over nvmrc

Corepack is a little different as it relates to the package managers while nvm (and fnm) manage node version. Aside: corepack will no long be shipped with node. I assume it will be maintained independently but the future looks a little shaky at the moment.

[ryanleecode]

i think better to use tshy because currently tsup isnt doing any code splitting and the package isnt tree shakable. i've added some updates in this PR that is against the current PR.

i've also validated the bundle on https://arethetypeswrong.github.io/

#562

[andrew-ifrita]

i think better to use tshy because currently tsup isnt doing any code splitting and the package isnt tree shakable. i've added some updates in this PR that is against the current PR.

That works for me. Left my full thought on the PR before merging it into this one, but it seems fine and I lack any strong enough opinion between the different tooling here. Thanks again for the help

[TarikGul]

👍