imbhargav5 / rooks

Essential React custom hooks ⚓ to super charge your components!
https://rooks.vercel.app
MIT License
3.18k stars 216 forks source link

[V7] Bundled umd does not pass es2015-es2019 checks #1362

Closed belgattitude closed 2 years ago

belgattitude commented 2 years ago

The bundled dist/umd/rooks.umd.js contains an optional chaining operator which is probably too modern for some browsers.

Not sure if it's intended as it does not appear in esm and cjs bundles. If it is would be interesting to mention it in the changelog and align the bundles.

image

Info

To reproduce

clone the renovate/rooks-7.x branch from https://github.com/belgattitude/nextjs-monorepo-example

yarn install
cd apps/nextjs-app
yarn build && yarn check-dist
// or
yarn es-check es2016 "../../node_modules/rooks/dist/umd/**.js"
imbhargav5 commented 2 years ago

@belgattitude Thanks for this! Do you happen to know a solution to fix this easily? If you can submit a PR or guide us to a solution we can fix this quickly. Appreciate it! 🚀

belgattitude commented 2 years ago

Sure I'll have a look this week-end.

belgattitude commented 2 years ago

Hey @imbhargav5 now that I opened the repo, I see multiple things that would make sense to fix.

First I'd like to rework the exports field (would love to remove unpkg field too - that's the reason why nextjs picked up the umd build, rather than cjs or esm)

Second I feel for bundling esm/cjs I'd like to go with tsup if it makes sense to you.

Third I'm wondering if I could remove the bundled test files and duplicate d.ts files

PS: I would even drop umd imo, but I'm not sure

I'll plan to make some PR's whenever I have time so you can follow and give me directions if needed.

Enjoy your day

imbhargav5 commented 2 years ago

@belgattitude Wow. Please go ahead and make a PR. I am not entirely sure of all the changes but it seems like you have a solid idea of the ideal setup. Once you make a PR based on your idea, I will do a thorough review and get it shipped asap.