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

dispatchOnMount: true has no effect when `process()` ends up not dispatching #102

Closed tizmagik closed 6 years ago

tizmagik commented 6 years ago

Description

When there is a top-level process function defined and dispatchOnMount: true is passed in and the result of calling process() is to not dispatch, then the boolean for dispatchOnMount essentially has no effect.

Expected Behavior

When dispatchOnMount: true is passed in, it should always dispatch when the component mounts, unless the call to process() has dispatched, then it shouldn't (because we wouldn't want to dispatch the same data twice).

Actual Behavior

When dispatchOnMount: true is passed in, and there is a process() function defined (that happens to not dispatch), then that flag is essentially ignored.

The root cause of the issue is the conditional logic here:

https://github.com/NYTimes/react-tracking/blob/585da2754dd63475dfb61d69dcebf3da18ebbef3/src/withTrackingComponentDecorator.js#L68-L88

h/t @callihiggins

tizmagik commented 6 years ago

A workaround is to define { dispatchOnMount: () => true } so that the first conditional L70 triggers

tizmagik commented 6 years ago

Released in v5.5.4