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

Added track options merge property to support custom tracking data me… #128

Closed jbadeau closed 5 years ago

jbadeau commented 5 years ago

Hi,

I have done a simple change to allow for custom tracking data merge strategies. I'm a bit new to the context so if you see something odd please let me know. Test and docs included

tizmagik commented 5 years ago

Thanks @jbadeau this is a great start and makes a lot of sense to want to allow folks to customize the merging strategy. Thanks for the PR and the additional tests!

I think however what you're looking to do might already be supported to some extent? Because react-tracking uses deepmerge under the hood, by default it will append arrays.

Here's a quick Codesandbox example showing the breadcrumbs use case you outlined here: https://codesandbox.io/s/young-mountain-in98p

Does this cover your use case, or do you have additional needs for customizing the merging strategy?

jbadeau commented 5 years ago

To be honest, this was something I wrote last year. I need to look at the use case again. I will get back to you later this week.

tizmagik commented 5 years ago

Hey @jbadeau I'm going to close this since the code has drifted significantly since then, but please feel free to re-open a fresh PR or start with an issue to explore your use case! Thanks again!