nytimes / react-tracking

🎯 Declarative tracking for React apps.
https://open.nytimes.com/introducing-react-tracking-declarative-tracking-for-react-apps-2c76706bb79a
Other
1.88k stars 123 forks source link

remove core-js #167

Closed adi518 closed 3 years ago

adi518 commented 3 years ago

This PR removes core-js (ES polyfills) and leaves it outside the bundle of this package. Consumers should now add polyfills to their app if they need to support old browsers. I wonder whether it should be released as a new major version, WDYT?

Other stuff this PR does:

Next steps: Use Webpack 5 or Rollup to build an ESM output, which will make this library treeshakeable, saving some more KBs for consumers. I suggest trying TSDX for this.

PS, Babel CLI docs aren't super updated, so better run yarn babel -h to see the latest options.

tizmagik commented 3 years ago

Thanks, this is great! Do you mind also adding a note in the docs about the needed polyfills for older browser support moving forward after this change?

And yes, to answer your question, I think to be safe we will consider this a major breaking change.

tizmagik commented 3 years ago

Use Webpack 5 or Rollup to build an ESM output, which will make this library treeshakeable, saving some more KBs for consumers. I suggest trying TSDX for this.

Great idea. Happy to accept PRs for these as well (or at least an issue).

adi518 commented 3 years ago

Sure, I'll update the readme.

patrickskim commented 3 years ago

core-js was an issue we've been having as well! Please merge & release ASAP!

tizmagik commented 3 years ago

Yes, looking to do a release this week hopefully. I want to land #168 as semver minor in v8 first

tizmagik commented 3 years ago

@patrickskim @adi518 looks like #168 might not land this week so I've published this PR against @alpha premajor release:

npm install react-tracking@9.0.0-0

Try it out and please post back with any issues you come across. Thanks!

tizmagik commented 3 years ago

@patrickskim @adi518 have you encountered any issues with this premajor?

adi518 commented 3 years ago

I haven't pushed to production with the new version yet, but it's ongoing.

antciccone commented 3 years ago

@tizmagik We were able to upgrade to react-tracking@9.0.0-0 in our gatsby project. It has been pretty seamless so far. Will update you if @patrickskim or I run into any issues! Thanks for this work πŸ”₯

tizmagik commented 3 years ago

v8.1.0 is released. I'll look to release a v9.0.0 next week or so with this change. Thanks again!

adi518 commented 3 years ago

We are about two weeks in production with 9.0.0-0 and thus far, no issues.

tizmagik commented 3 years ago

RC v9.0.0-4 has been tagged, mind giving that a quick check and if all looks good I'll merge, tag and release this to mainline @adi518 @antciccone @patrickskim

npm install react-tracking@9.0.0-4
gedeagas commented 3 years ago

Removing core js make the bundle size much smaller. 15.3kb and 2.42 when gzip. Screen Shot 2021-07-15 at 12 41 08 AM

So far no issues, i will run the build on staging for a few days and wait the result.

Thank you for pushing this @tizmagik πŸŽ‰

gedeagas commented 3 years ago

@tizmagik after few days of dogfooding and thus far, no issues. I think this release is good to go.

🚒It πŸŽ‰

tizmagik commented 3 years ago

Great, thanks @gedeagas !

tizmagik commented 3 years ago

Hey @bgergen do you think we can get CI going again? I'd love to have it test this PR before we merge/publish, if possible

bgergen commented 3 years ago

@tizmagik Yes! I'll try to get that setup tomorrow. Sorry, I've been swamped today getting ready for Olympics coverage.

bgergen commented 3 years ago

@tizmagik Working on figuring out why this isn't triggering a drone build. Maybe I missed something in the config to get it to run on a PR from a fork.

tizmagik commented 3 years ago

Thanks @bgergen -- feel free to push directly to this branch

tizmagik commented 3 years ago

Yay thank you everyone for all the hard work! πŸŽ‰

tizmagik commented 3 years ago

This is now released as v9.0.0 thanks again all!