react-tracking
react-tracking copied to clipboard
Provide a `mergeOptions` configuration to pass along to `deepmerge`
Currently, it is impossible to track objects of type Error
or objects that inherit from Error
. This is because deepmerge
treats Error
as a mergeable object and creates a plain object with it.
See: https://www.npmjs.com/package/deepmerge#ismergeableobject
and: https://github.com/nytimes/react-tracking/blob/main/src/useTrackingImpl.js#L43
This means Error
objects will be converted to plain objects when they reach the custom dispatch function. I assume this isn't intended behaviour?
Would you accept a PR that turns this behaviour off via a configuration value? This would allow the custom dispatch to handle Error
objects. Alternatively, this could be turned off by default but that would be a breaking change.
Ah yeap, good callout, and this use case makes sense.
I wonder if instead of providing a flag to turn this behavior off if we should instead just accept a top-level merge
property in the options object which can either be an object to pass in as configuration to deepmerge
, or is the merge()
function to use itself instead of deepmerge.
Thoughts on that approach?
@tizmagik That would work too. I kind of like your 1st suggestion of passing these options to deepmerge. This way consumers don't need to write or install their own merge function. This would enable the Error
usecase via the isMergeableObject
deepmerge config option, and likely enable other usecases.
If you're ok with this approach I could put a PR together
Yea a PR for that would be great! Only thing I’m not sure about is if we should call it merge
or mergeOptions
?
I kind of like mergeOptions
. Wouldn't want people to get confused and think merge
should be a function
This was completed:
- #187
- #212