Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: upgrade dependencies / migrate to ESM #113

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dirkluijk
Copy link

Based on #112, I took it a bit further.

Since I wasn't directly able to get everything to work locally, I did some rework and applied the following changes:

  • Tweaked the tsconfig setup a bit with some of the recommendation from this cheat sheet and this article
  • Migrated the whole source code to ESM - I had to update a lot of imports to make it fully compatible with the ESM format which requires extensions in the transpiled output
  • Upgraded to Node 18 and ES2022
  • Upgraded to TypeScript 5.6
  • Fixed remaining issues with missing dev dependencies and other TypeScript errors
  • Upgraded the Nest example project to Nest 10 and replaced Jest with Vitest

I also tested all the example projects and they seem to work fine. All that now remains is to publish a new version to NPM.

@axe-me
Copy link
Owner

axe-me commented Sep 22, 2024

Hi Dirk, thanks for your contribution! I will take some time to review it soon. Not sure if I should set the default to esm format, since it's a breaking change, that can break everyone's repo. I have to think about this.

@dirkluijk
Copy link
Author

Yeah, I falsely assumed one can import ESM from CommonJS. Although, this does actually work when you use a build tool for your project.

Personally I try to advocate ESM, since it is the standard and supported by both Node.js and other runtimes/browsers. But I agree this is a breaking change. Of course in this case it would only affect your vite.config.js, I can imagine Vite also wouldn't have a problem with it but we would have to test that. 😎

Thanks for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants