Skip to content

Conversation

@bgergen
Copy link
Collaborator

@bgergen bgergen commented Nov 12, 2019

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
Copy link
Collaborator

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

@bgergen
Copy link
Collaborator Author

bgergen commented Nov 19, 2019

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.

Copy link
Collaborator

@tizmagik tizmagik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@tizmagik tizmagik merged commit b5a6697 into nytimes:master Dec 19, 2019
@tizmagik
Copy link
Collaborator

Released as v7.3.0 -- thank you! 🎉

@bgergen bgergen deleted the remove-contexttype branch December 19, 2019 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants