civiccc / react-waypoint

A React component to execute a function whenever you scroll to an element.
MIT License
4.08k stars 208 forks source link

Use external @babel/runtime helpers #341

Closed realityking closed 3 years ago

realityking commented 3 years ago

Fixes #302

It does complicate the rollup config quite a bit but this is needed to correctly generate both the ESM and CJS versions.

trotzig commented 3 years ago

Do you know if this increases the size of the built javascript file?

realityking commented 3 years ago

Do you mean the bundled file including the Babel helpers? It will probably depend on how things are build. If other libraries or the application itself uses @babel/runtime it'll be smaller, if the build pipeline is set-up for ESM and concatenating modules (web pack 4 or 5) it should be at worst the same. If it's set-up for CommonJS and there's no deduplication it will be a bit bigger because every module is wrapped.

I'll try to get some data on this :)

realityking commented 3 years ago

To get an idea of the size impact, I created a couple of different builds of the performance tests. This is using Webpack 4, I'd expect better results with Webpack 5 or Rollup.

Conclusion: CJS sucks but no impact on externalising these helpers :)

realityking commented 3 years ago

@trotzig I think a release needs to be done to address #347. It'd be great if you could take a look if you're comfortable including this in that release, if not, please go ahead without this one.

lencioni commented 3 years ago

I did this slightly differently here https://github.com/civiccc/react-waypoint/pull/353

Thanks for opening this PR!

realityking commented 3 years ago

That approach is entirely fair as well. :) The different is really one of personal preference as I don't like complicated .babelrc files 😆

Thanks for getting this merged!