Change CJS export to ESM
Hi there, great project! While I was trying to use the package, I couldn't manage to get it to work with a custom element (not plain text), e.g.
<Konami>
<MyElement/>
</Konami>
I'm using Vite, and while it worked in development mode, it didn't work when I compiled to production, so it might be a problem with rollup.
Thanks!
Hey @liorp, could you give a little bit more information? Are you using TS or JS? I think I've found an issue, it looks like we missed the children type in the types definition.
I’m using typescript indeed.
Sent from my iPhone
On 9 May 2022, at 12:35, Vinicius Marchesin Araujo @.***> wrote:
Hey @liorp, could you give a little bit more information? Are you using TS or JS? I think I've found an issue, it looks like we missed the children type in the types definition.
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.
I've published a new version 2.3.0 that should fix this. You can test this by cloning this repository and trying it inside the example folder with a production build of next . If it doesn't work then it might be a problem caused by your configuration. I'll wait for your reply to close this issue.
Nope, it still happens, even after updating
"react-konami-code": "^2.3.0"
Sorry but I'm unable to reproduce the error. I just tried it in a TypeScript project of my own running the following dependencies and my production bundle works.
"dependencies": {
"next": "12.1.4",
"react": "18.0.0",
"react-dom": "18.0.0",
"react-konami-code": "^2.3.0"
},
"devDependencies": {
"@types/node": "17.0.23",
"@types/react": "18.0.1",
"@types/react-dom": "18.0.0",
"typescript": "4.6.3"
}
I have a next/image as the child of Konami. I tried the production bundle using next build and it works just fine.
<Konami>
<Image
alt="Image"
src={imgSrc}
width={256}
height={256}
priority
/>
</Konami>
All I'm saying is that I can't reproduce the issue on my side. Could you provide a snippet of the part of your code that doesn't work, or the full project if possible?
Yes, I have an example project for you to check:
https://github.com/liorp/liorpdev/commit/2cbbce554f025c6813c7b1276e6d04c814e8b1a0
Please note that I used the useKonami hook in the last commit to main, so you should look at the commit before (2cbbce554f025c6813c7b1276e6d04c814e8b1a0).
It has a dependency of "react-konami-code": "^2.2.2", but it also fails in 2.3.0 (I just checked it, but didn't commit and push to avoid down time to my site).
Thank you very much for your help on this.
I believe this repo is private and I do not have access to it.
Changed to public. Sorry for that.
Sent from my iPhone
On 9 May 2022, at 21:19, Vinicius Marchesin Araujo @.***> wrote:
I believe this repo is private and I do not have access to it.
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.
Alright, I just checked and this is a Vite issue indeed :) To be specific it's rollup that doesn't like cjs modules. For more information you can check this issue https://github.com/vitejs/vite/issues/2139
react-konami-code is exported as a cjs module so this issue makes sense in this scenario. This is fixed by https://github.com/rollup/plugins/pull/1165 but I don't know when it will be merged.
You can fix this by doing either of these:
- Stop using Vite 😬 Although if you really want to use it there are other options. It looks like the fix in rollup is coming so this should be fixed soon anyway.
- Change your import to:
import K from 'react-konami-code'
// @ts-ignore
const Konami = K.default ? K.default : K
- Use the
useKonamihook as you did.
Changing the export to esm is not a bad idea but I don't have any plans to do that now as the library is working just fine. If you want to submit a PR you're welcome to make the changes yourself and submit it for review (after ensuring it's working properly). If you have other questions feel free to ask, otherwise I'll close this issue soon :)
Cool, thanks for the thorough reply. I’ll try to get to exporting as esm. Do you mind if I opened a ticket tagged as improvement for this?
On 9 May 2022, at 22:04, Vinicius Marchesin Araujo @.***> wrote:
Alright, I just checked and this is a Vite issue indeed :) To be specific it's rollup that doesn't like cjs modules. For more information you can check this issue vitejs/vite#2139
react-konami-code is exported as a cjs module so this issue makes sense in this scenario. This is fixed by rollup/plugins#1165 but I don't know when it will be merged.
You can fix this by doing either of these:
Stop using Vite 😬 Although if you really want to use it there are other options. It looks like the fix in rollup is coming so this should be fixed soon anyway. Change your import to: import K from 'react-konami-code' // @ts-ignore const Konami = K.default ? K.default : K Use the useKonami hook as you did. Changing the export to esm is not a bad idea but I don't have any plans to do that now as the library is working just fine. If you want to submit a PR you're welcome to make the changes yourself and submit it for review (after ensuring it's working properly). If you have other questions feel free to ask, otherwise I'll close this issue soon :)
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.
I'll edit the title of this issue so we keep the discussion here. I started some work in this branch here https://github.com/vmarchesin/react-konami-code/tree/esm-export but I'm not sure if I need to change the library.type to module as per Webpack docs.
I'd have to look into that, and this seems like a a great opportunity for you so I'd hate to take that from you :D Feel free to open a PR if you manage to make it work.
I hope to have some free time soon to actually work on the code itself. In the meantime I strongly recommend you to look into tsup. I've used it to build a library of mine, and it is really fast and convenient. Again, thanks for your work!