search

FormidableLabs / react-swipeable

React swipe event handler hook
https://commerce.nearform.com/open-source/react-swipeable
MIT License
2k stars 147 forks source link

Replace microbundle with rollup #273

Closed binoy14 closed 2 years ago

binoy14 commented 2 years ago

I don't believe this is a breaking changes we did not change the file but it's hard to tell :D so just to be save maybe bundle this in v7 release?

github-actions[bot] commented 2 years ago

size-limit report 📦

Path Size
dist/react-swipeable.js 1.08 KB (-12.26% 🔽)
dist/react-swipeable.umd.js 1.09 KB (+100% 🔺)
dist/react-swipeable.module.js 1.11 KB (+100% 🔺)
hartzis commented 2 years ago
binoy14 commented 2 years ago
  • should we remove the "esmodule": "./dist/react-swipeable.modern.js", output in package

I believe we should still keep it since most modern frameworks are moving to ESmodule, example NextJS so the bundlers have an option to pick whatever the app is using. Not an expert here but I think that's how this works :D

hartzis commented 2 years ago

Reference for exports when reviewing and deciding

rschristian commented 2 years ago

Yes, "esmodule" should be removed as a key. It is the precursor to the modern "exports" but is very much legacy at this point, next to nothing actually supports consuming it. Microbundle was pushing it before "exports" was solidified and picked up by Node. We've since removed it from the docs.

I believe we should still keep it since most modern frameworks are moving to ESmodule,

Supporting ESM is very different from reading the "esmodule" key.

binoy14 commented 2 years ago

I believe we should still keep it since most modern frameworks are moving to ESmodule, example NextJS so the bundlers have an option to pick whatever the app is using. Not an expert here but I think that's how this works :D

I misread the comment earlier, I updated it by removing esmodule from package.json thanks!