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

Migrate to ESM #284

Closed
nikeee opened this issue May 6, 2023 · 5 comments · Fixed by #319
Closed

Migrate to ESM #284

nikeee opened this issue May 6, 2023 · 5 comments · Fixed by #319

Comments

@nikeee
Copy link
Contributor

nikeee commented May 6, 2023

The TS types state that the module is an ES Module, while it is actually a CommonJS module.

Is this open for fixes?

@dnlup
Copy link
Owner

dnlup commented Dec 20, 2023

Hi @nikeee , so sorry to answer just now. I am not sure I understand where the problem is and how to fix it. If you have a solution PRs are welcome, thank you 🙏🏻

@dnlup
Copy link
Owner

dnlup commented Dec 20, 2023

Can this package help us?

@nikeee
Copy link
Contributor Author

nikeee commented Dec 20, 2023

We have two options:

  1. Port the library to ESM
  2. Fix the type definitions in index.d.ts to match the actual module system used (commonjs)

Regardless of the solution taken, migrating to ESM might be worth it from a long-term perspective.

@nikeee
Copy link
Contributor Author

nikeee commented Dec 20, 2023

Did a PR: #319

@dnlup
Copy link
Owner

dnlup commented Dec 21, 2023

Thank you @nikeee , I have to be honest, I feel reluctant to migrate to ESM now because I still see a lot of commonJS projects and I like having support for both module systems. I'll have to make a decision though otherwise this will never happen.

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 a pull request may close this issue.

2 participants