-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
Add "type: module" (nodenext) support alongside commonjs #481
Comments
Thanks @chronoDave. Do you happen to know how to solve this? Best, |
I've not done much research yet but I can pick it up if you'd like. I suspect it would be simply following the advice from https://arethetypeswrong.github.io/?p=embla-carousel%408.0.0-rc05
Though I'm not sure how it would impact cjs imports. |
@chronoDave seems like this could do the trick but the main part of investigating this would be to make it compatible with all and how to generate this automatically. Because I use Feel free to investigate this further if you want. Best, |
Thank you for the After doing some more research into how I've tried solving the issue using I feel it's probably best to wait for that issue to be resolved and then see if that solves the problem. |
Thanks for taking the time to investigate this @chronoDave 👍. At the time of writing my understanding of this problem and possible solutions is very limited. So sorry if this is a stupid question: What's your understanding of this problem? Because I'm a bit confused. The solution seems to be changing: // from:
export default EmblaCarousel
// to:
export = EmblaCarousel But is this backwards compatible for Best, |
The issue lies with TypeScript not being able to tell the difference between a I've attached a video that should hopefully better describe the issue. I've created a repo as well so you can easily verify it for yourself: https://github.com/chronoDave/ts-esm-cjs Code_PhvzKXuo8y.mp4When trying to resolve the Types
Code
By adding As for a solution, creating seperate type declarations seems to do the trick, but this requires "exports": {
".": {
"import": {
"default": "./embla-carousel.esm.js",
"types": "./esm.d.ts"
},
"require": {
"default": "./embla-carousel.cjs.js",
"types": "./cjs.d.ts"
}
}
}, I hope this helps clarify the issue more. My apologies if my answers seem vague, I'm figuring it out as I go, I don't have too much experience yet with using |
Hi @chronoDave, I just found this article that claims you can support both Best, |
Hey @davidjerleke, I've verified the solution mentioned in the article you linked and it surprisingly works! I've moved the CJS and ESM exports into their respective folders and duplicated the The main The I'm pretty confident it's possible to generate the folder structure in this way using |
after reading microsoft/TypeScript#49189 I got a workaround for now:
|
@chronoDave never mind. I found a solution! |
Add "type: module" (nodenext) support alongside commonjs
@chronoDave and @chiptus this will be released with the next version of Embla. |
Bug is related to
Embla Carousel version
Describe the bug
When using node16 esm import in TypeScript, TypeScript with incorrectly flag up the embla-carousel default import as missing. This is due to incorrect typing: https://arethetypeswrong.github.io/?p=embla-carousel%408.0.0-rc05
This error can be ignored using
@ts-expect-error
or something similar as I can confirm that the default import does work, it's just the typing that's incorrect.CodeSandbox
Unfortutately this bug does not occur when usign CodeSandbox. If needed, I can create a simple repository demonstrating this behaviour.
package.json
tsconfig.json
index.ts
Steps to reproduce
Simply hover over the EmblaCarousel error in your IDE of choice (mine being VSC)
Expected behavior
I expect the default export to be typed correctly.
Additional context
It seems to be a common issue:
default
CJS or ESM exports with modulenodenext
. microsoft/TypeScript#49189The text was updated successfully, but these errors were encountered: