Skip to content
This repository has been archived by the owner on May 20, 2024. It is now read-only.

feat: ability to disable ga emit on load + expose optimize token #34

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

hecktarzuli
Copy link
Collaborator

No description provided.

@hecktarzuli hecktarzuli changed the title feat: ability to disable ga emit on load feat: ability to disable ga emit on load + expose optimize token Aug 13, 2020
@@ -222,14 +211,87 @@ import './styles.scss'
}
```

## Development
Copy link
Member

Choose a reason for hiding this comment

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

Why development section removed?

Copy link
Collaborator Author

@hecktarzuli hecktarzuli Aug 13, 2020

Choose a reason for hiding this comment

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

Because it no longer applies, that test page was removed in favor of automated E2E.
If you like I can re-create the playground.

Copy link
Member

Choose a reason for hiding this comment

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

That would be nice or mentioning how to locally test (using e2e) if it is prefered


## emitOnLoad Caution

By default emitOnLoad is true (for backward compatability). This means that when a user hits your site this plugin will tell Google Analytics (Optimize) that the user is in the experiment.
Copy link
Member

Choose a reason for hiding this comment

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

I think it depends. Not all experiments are specific to some pages so backward compatibility is misleading here even if in next major we are going to disable by default. There could be also smarter ways like defining if an experiment is global or manual in experiment export file and skip auto emit if is not.


By default emitOnLoad is true (for backward compatability). This means that when a user hits your site this plugin will tell Google Analytics (Optimize) that the user is in the experiment.

If your experiments aren't on every page, you'll want to turn this off and report the experiment yourself. If you don't, then your Google Optimize reports will contain a bunch of users who likely never saw your experiment. In other words **GARBAGE DATA**.
Copy link
Member

Choose a reason for hiding this comment

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

Is it really necessary to use negative word like GARBAGE DATA instead of a WARNING: ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, too much passion & personality showing :). I'll tweak.

```js
mounted(){
// call Google Analytics directly and set the experiment for the user
window.ga('set', 'exp', this.$exp.token)
Copy link
Member

Choose a reason for hiding this comment

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

ga and gtm (via dataLayer` are commonly used and best practices for google optimize integration also by our docs.

I strongly believe we should support built-in utility for manual triggering (like this.$exp.send()) for both DX and additional checks like prevent adding extra events.

  • With module options handler (ga|gtm) and handlerName (dataLayer|ga) we can configure how send() utility behaves
  • For advanced use cases without either, we may document manual trigger using exposed token (which currently no 3rd method is even mentioned in docs)

Copy link
Collaborator Author

@hecktarzuli hecktarzuli Aug 17, 2020

Choose a reason for hiding this comment

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

Does it need to be this complex? Even if you use GTM you'd be loading GA via GTM, so window.ga should still exist (so .send() could just use window.ga). For advanced usage you'd still be able to use $exp.token (i can't think of any use-cases)

Copy link
Member

Choose a reason for hiding this comment

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

Well at least the docs is advicing to use dataLayer.push({ exp: window.$nuxt.$exp.token }) which can be much easier IMO by using send. And if changing the way of sending events, instead of refactoring all usages, we can simply change behavior with a config.

Copy link
Collaborator Author

@hecktarzuli hecktarzuli Aug 17, 2020

Choose a reason for hiding this comment

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

We could have handler: (ga | gtm | auto - default) as config options
For auto we'd first check for window.dataLayer and use gtm/push else-if we detect window.ga we'd use window.ga

Copy link
Member

Choose a reason for hiding this comment

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

That's even sexier 💯

const expCookie = experiment.experimentID + '.' + variantIndexes.join('-')

// expose raw optimize token directly
experiment.token = expCookie
Copy link
Member

Choose a reason for hiding this comment

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

Maybe can be merged with L66 like experiment.token = experiment.experimentID + '.' + variantIndexes.join('-') ? to avoid extra expCookie variable?

## Integrating with Google Tag Manager
There are many ways to integrate Google Optimize with GTM. Usually you have Google Analytics setup in GTM and you push the experiment token into GTM.

### Method 1 - Push Token To the Data Layer
Copy link
Member

Choose a reason for hiding this comment

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

What do you think to also link each method to google docs?

googleOptimize: {
// experimentsDir: '~/experiments',
// maxAge: 60 * 60 * 24 * 7 // 1 Week
// maxAge: 604800 // 1 week (in seconds)
Copy link
Member

Choose a reason for hiding this comment

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

60 * 60 * 24 * 7 convention is to allow developers quickly modify time without need to calculations and increasing source readability

@hecktarzuli
Copy link
Collaborator Author

hecktarzuli commented Aug 19, 2020

I'm also toying with the idea of revamping the MVT support. Currently it's just 'sections:#' and weights but in practice I would find it very hard to code an experiment against a variant index and an intVal vs a named obj with values. For example.

As a user, I'd prefer something like this

if($exp.$activeVariants.buttonColor == 'red') ..
if($exp.$activeVariants.headerText == 'excited') ..

vs

An array of variant objects ($exp.$activeVariants)
[ {weight: 25 }, { weight: 25}, {weight: 25 }, { weight: 25}]..

This idea is pretty new and not totally flushed out, but if we do decide to go in this direction, then we'd want a new major version for sure.

@pi0

@pi0 pi0 mentioned this pull request Aug 25, 2020
@pi0
Copy link
Member

pi0 commented Aug 25, 2020

Nice idea @hecktarzuli. Just created #38 based on your suggestion. Maybe we can do it via a non-breaking change too as part of $exp interface improvements. (it was 3 years ago so not much perfect)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants