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

Default dispatch function does not appear to work as expected #79

Open riley-steele-parsons opened 6 years ago

riley-steele-parsons commented 6 years ago

I've attempted to setup a track decorator on a high level component which contains an option object as a second argument, like so:

import track from 'react-tracking'
...
@track({}, { process: (ownTrackingData) => {ownTrackingData.page ? {event: 'Pageview'} : null }} )
class AppWithAuth extends React.Component {
...

Based on the docs, I gathered that when a lower level track decorator dispatches an event, the default dispatch function pushes the event object onto the window.dataLayer array which is subsequently sent to Google Analytics. I am not seeing this behavior. window.dataLayer does not update when the event is fired.

However, if I replace the track decorator on the high level component with

@track({}, { process: (ownTrackingData) => {console.log(ownTrackingData);return ownTrackingData.page ? {event: 'Pageview'} : null }, dispatch: (d) => {console.log(d); (window.dataLayer = window.dataLayer || []).push(d)}} )

I am able to see the event objects pushed onto the window.dataLayer array through Chrome's javascript console. I also am able to see the Pageview event on my Google Analytics dashboard.

tizmagik commented 6 years ago

Thanks @riley -- it's possible you've found a bug. Are you able to create a failing test case and/or create a reproduction in https://codesandbox.io or similar?

tizmagik commented 6 years ago

Note that your process function in your snippet never returns anything, it would need to be updated to this:

import track from 'react-tracking'
...
@track(
  {},
  {
    process: ownTrackingData => (ownTrackingData.page ? { event: 'Pageview' } : null),
  }
)
class AppWithAuth extends React.Component {}
...

Note the additional parens ( ) to return the result of the ternary.

That might be what's going on in your case, can you try that out and confirm @riley-steele-parsons ?

riley-steele-parsons commented 6 years ago

@tizmagik , could you take a look at this CodeSandbox? I am now seeing data within the dataLayer property (without manually duplicating the dispatch function). However, instead of having the process function inject properties into each new track object, a single object containing the process data is added to the beginning of the dataLayer.

https://codesandbox.io/s/n94mkxjrzj

tizmagik commented 6 years ago

Hey @riley-steele-parsons thanks for the CodeSandbox repro -- I think I see what you're saying now. However, this is actually expected behavior. The process function is really meant to conditionally dispatch additional tracking objects when a component mounts. As from the docs (emphasis added):

When there's a need to implicitly dispatch an event with some data for every component, you can define an options.process function. This function should be declared once, at some top-level component. It will get called with each component's tracking data as the only argument. The returned object from this function will be merged with all the tracking context data and dispatched in componentDidMount()

The main use case for this is to dispatch an additional "pageview event" for Page components. Perhaps process isn't the best name for this (haha sorry, naming is hard!). The docs could also stand to be clearer about this, for sure. If you have thoughts on how better to phrase that, I'd love a PR 😁

To do what I think you want to do, which is to append additional tracking data for every tracking dispatch, you would actually want to define this in a custom dispatch() function.

For example:

@track(
  {},
  {
    dispatch: d => (window.dataLayer = window.dataLayer || []).push({
      ...d,
      time: new Date()
    })
  }
)
class App extends React.Component { ... }

Updated codesandbox example: https://codesandbox.io/s/qqxqzqp5l6