-
Notifications
You must be signed in to change notification settings - Fork 32
feat: add GTM handler and options #43
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
@hecktarzuli do you have any opinions? Have I missed anything?
@eljass I would appreciate it if you could also add tests for these changes. |
@farzadso I would but I don't really know how to test other libraries such as $gtm or global variables such as |
lib/plugin.js
Outdated
|
||
const exp = experiment.experimentID + '.' + experiment.$variantIndexes.join('-') | ||
|
||
window.ga('set', 'exp', exp) | ||
// Choose method to track experiment and variant | ||
<% if (options.eventHandler === 'gtm') { %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like the wrong direction to me. What we are essentially doing is corrupting this module based on some select libraries. What we should be striving for instead is exposing exp
so anyone can do whatever they like.
For example, at work we use Segment now, how would this integrate with that? etc.. I understand ga and gtm are common, but if I had my way I wouldn't have auto-included GA to begin with (call me a purist :P)
I do have more plans related to this, but I've been busy with MasteringNuxt for the past months :(
Letter C here (#26)
It's not difficult, I just don't have time at the moment. I think that's a better solution so people can use a 1-2 liner to wire up pretty much any other integration.
cc @farzadso
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hecktarzuli I absolutely agree with you. A completely global solution would be much better and we'd be giving the flexibility users require.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my newest update, what do you think of dynamic handling of tracking. This would allow users to define their own function to send tracking events. I tested this with $gtm module and it worked well.
This is pretty much the same solution as here #13.
This same issue also seems to be addressed in three PR right now. If there is something I can do to speed up the development of the wanted way to implement this functionality just tell me! We need this to work in our product and I would like to use this module and not my own fork in it to achieve this.
It occurs to me that this is not probably necessary as the user could write a plugin which handles sending experiment with preferred way? So should we just introduce an option to disable Docs could then explain how to send a preferred event from the plugin. For example GTM module would be // plugins/google-optimize.js
export default function ({ $exp, $gtm }) {
const exp = experiment.experimentID + '.' + experiment.$variantIndexes.join('-')
$gtm.push({ exp })
} // nuxt.conifg.js
export default {
...
googleOptimize: {
experimentsDir: '~/experiments',
maxAge: 60 * 60 * 24 * 7, // 1 Week
pushPlugin: true,
plugins: ['~/plugins/google-optimize.js'],
excludeBots: true,
botExpression: /(bot|spider|crawler)/i
}
} Would it be enough then to add option to disable ga event? |
Hello, can we merge this please? I'm also interested in using GTM instead of separate GA implementation @farzadso |
Since this repo seems to be unmaintained I've created a fork of @eljass his code and created a new package on npm |
This PR is inspired from #13. That PR seems inactive so this will address the need for GTM integration.
Adds the possibility to use @nuxtjs/gtm-module or custom tag manager implementation with the possibility to rename used dataLayer variable.
This PR adds three options to implement sending experiment info:
options.dataLayer: DATA_LAYER_NAME