CVM / Magento_GoogleTagManager

Google Tag Manager extension for Magento
67 stars 55 forks source link

Suggestion to use window.dataLayer = window.dataLayer || []; #16

Closed phildpearce closed 10 years ago

phildpearce commented 10 years ago

Change

To...

This reduces the chance of conflicts if dataLayer is called by onload video events later in the page.

Also using the "window." means that if the dataLayer loads in a frame it wont trigger or be overwritten.

CVM commented 10 years ago

GitHub seems to have eaten the code in your post, but I'd guess what you're saying is motivated by discussion in this G+ post.

Brian's code illustrates how to do a dataLayer.push() when you're not confident that the dataLayer is within current scope or think it might have been set to be something else other than an array. It can also help in situations where GTM has been declared in the wrong place, causing dataLayer.push() calls to occur prior to gtm.js loading.

This extension will always put gtm.js in the correct place, so dataLayer.push() calls anywhere in the body below this point should reliably queue up - you won't be able to push before the initial declaration.

In terms of scope, the dataLayer is declared globally, as normal. You can reference it from within other JavaScript functions without any problems, provided that it has not been overwritten, so prefixing with window. shouldn't be necessary. Example below:

<script>
  dataLayer = [];
</script>

...

<script>
  function fireEvent(eventName) {
    dataLayer.push({'event':eventName});
  }
  fireEvent('my_special_event');
</script>

Rather than add further code to every dataLayer.push() in order to guard against possible misuse, I feel that the edge case would be better dealt with by writing good code and not destroying it in the first place. No code within the extension will be doing this and it's very unlikely other extensions will be using the variable name for anything else.

phildpearce commented 10 years ago

This only effects video and social plugins that set a dataLayer later in the page, you are right, it is an edge case.

Here is the unmunchced code...

window.dataLayer = window.dataLayer || []; window.dataLayer.push({

Related post: https://plus.google.com/111244485379930211802/posts/hYCyRyQ19vf

Problem... // Set dataLayer dataLayer = [{'page_virtual': '/virtual/OVERWRITTEN'}];

Githubissues.
  • Githubissues is a development platform for aggregating issues.