search

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 TrackingContextType, test useTracking export #146

Closed bgergen closed 4 years ago

bgergen commented 4 years ago

This PR removes TrackingContextType, which I believe is leftover from an era when the library used the legacy context API. As far as I can tell it no longer provides any value. I also removed the test that checked for the context type export and replaced it with the same check for the useTracking hook.

tizmagik commented 4 years ago

Thanks! Makes sense. Do you think this would warrant a major version bump? That would be a bummer...

bgergen commented 4 years ago

Oof, good question. It shouldn't, but maybe it could cause issues if anyone is importing that type for some reason? I can't imagine what they would be doing with it though...

If we do need a major version bump, maybe it makes sense to wait to release until we've got some other stuff in there? I'm still hoping to figure out some way to push contextual data into the useTracking hook without needing to wrap the returned markup in a new provider component.

tizmagik commented 4 years ago

Released as v7.3.0 -- thank you! 🎉