-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix(package.json): add export map #652
Conversation
@trusktr Anything you want me to change here? |
This is great! I do agree we should change to type:module and remove the old field. We would probably have to release this as a major bump, along with a release note too I think. Would love to get rid of CJS, and just have ESM. |
That all sounds great, but we we can add the exports field without it being a breaking change, right? And then in the future considering to remove the commonjs output if you want. |
@christjt Hello! Sorry for taking so long. I was hesitant because I wanted to try to see if this breaks anything. But I think we can just release this as a major bump, in case it breaks someone (not sure) and note the possibility in the release. For some reason the build is not running. Do the regular tests pass for you? Can you also add the skypack check to the tests? After this, let's just merge it and I'll bump the major. |
Ok, got the darn build fixed. Looks like it was a bug inside GitHub. |
Ok, tests passing, gonna just merge and release a major bump! |
Running ESM in jest tests (node) isn't working properly with this lib yielding this error:
Running @skypack/package-check also validates this.
This PR adds export map and after these changes everything is healthy:
Package.json
"module"
is legacy and can (and probably should) be removed. You can also concider adding"type": "module"
to properly indicate that this lib is ESM, but again, I'll leave this to the maintainer to decide.This may also fix: #649