Skip to content
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

Building/bundling react-tracking #127

Open
tizmagik opened this issue May 21, 2019 · 6 comments
Open

Building/bundling react-tracking #127

tizmagik opened this issue May 21, 2019 · 6 comments

Comments

@tizmagik
Copy link
Collaborator

As per discussion in #119 we need a better way of bundling/building react-tracking. In v7.0.1 we had to move core-js@3 to a direct dependency, even though we don't need the entire library so that user-land doesn't need to adjust their babel configuration. This means we're reporting +40KB on bundlephobia which is misleading.

Maybe there will be some churn in the ecosystem as folks upgrade to core-js@3 and babel 7.4+ and better solutions/patterns will be established, but in the mean time, are there any ways to:

  1. Avoid having to include the entirety of core-js as a direct dependency
  2. Avoid having user-land have to modify babel configuration to use react-tracking (apart form decorators support, which is optional anyway.
  3. Keep it as simple as possible

Any thoughts/ideas/other libs you know about that have solved this in a better way please feel free to chime in!

Some ideas:

  • Use microbundle (WIP branch)
  • Use Rollup
  • Parcel/Webpack?
  • Other thoughts...?
@dortzur
Copy link
Contributor

dortzur commented Jul 17, 2020

Hey, why not leave core-js implementation to userland? For example, we detect old browsers and load core-js for them only. We'd rather libraries not import their own core-js.

If you look at the modules ecosystem this practice is almost non-existent for this reason.

Would change library size from ~13kb to around ~3kb. I think userland is accustomed to having take care of polyfills

@tizmagik
Copy link
Collaborator Author

Yea, ideally we would do that, but the tension was trying to keep the upgrade path as simple as possible since having core-js defined as a peerDependency means folks had to specify core-js@3 in their babel config which wasn't even an option before babel@7. More context in that other issue, #119

It's been some time since then so maybe worth revisiting, especially if, as you said, this is the common practice in other module ecosystems. In fact, if you can point me to a few examples, maybe we can bundle in the same way so there isn't an exception here.

I'd also be happy to review a PR to make this change if you're up for it, @dortzur . Thanks for commenting!

@adi518
Copy link
Contributor

adi518 commented Oct 12, 2020

Let's do it, polyfills are global and should be within consumer responsibility.

@tizmagik
Copy link
Collaborator Author

Happy to accept a PR

@adi518
Copy link
Contributor

adi518 commented Oct 12, 2020

I'll create a PR soon, thanks.

@adi518
Copy link
Contributor

adi518 commented Oct 20, 2020

Done: #167

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants