Skip to content

Magento interferes with the function queue declaration in Google Analytics #13267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
andrewhowdencom opened this issue Jan 18, 2018 · 11 comments
Closed
Labels
Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Progress: needs update

Comments

@andrewhowdencom
Copy link
Contributor

Google Analytics implements the function queue pattern. I explain this pattern in some depth here:

#8837 (comment)

However, the TLDR is that Google Analyics declares a global variable on the window object very early in the page load:

i[r] = i[r] || function() {
        (i[r].q = i[r].q || []).push(arguments)
    }

Paraphrased as:

(function() { 
  window.ga = window.ga || function() { 
    (ga.q = ga.q || []).push(arguments) 
  };
})();

It immediately stores events in this queue, of the form:

ga('send', 'pageview');

However, and critically, it does not execute this queue. It simply stores the view, and appends the JavaScript to the <head> block.

<script async src='//www.google-analytics.com/analytics.js'></script>

The async attribute defers the execution of the analytics block until the browser decides it's appropriate to pull these resources down, but it does not block browser parsing the document fetching other resources (unless the browser is old and crap and you probably should block anyway.

The absence of the function queue means that ga events must all be wrapped in the bosomy cushion of require, which creates a nasty dependency tree that does not need to exist.

Putting back the function queue will resolve any issues with adblockers (#12428). It does not need to be wrapped in require; it's more efficient simply left to its own devices.

Preconditions

  1. 2.2
  2. Do smth like:
                ga('send', {
                    'hitType': 'event',
                    'eventCategory': '__WHOO_CHICKEN_FEET__',
                    'eventAction': 'expand',
                    'eventLabel': shownId
                })

Steps to reproduce

  1. See above

Expected result

  1. ga should work in all cases, regardless of the loading state of the site.

Actual result

  1. (Yes I know there's angular there just ignore it it's fine)
Uncaught ReferenceError: ga is not defined
    at app.js:61
    at Object.e [as invoke] (angular.min.1.4.6.js:39)
    at angular.min.1.4.6.js:41
    at m (angular.min.1.4.6.js:7)
    at fb (angular.min.1.4.6.js:41)
    at d (angular.min.1.4.6.js:19)
    at Object.yc [as bootstrap] (angular.min.1.4.6.js:20)
    at haendlersuche.html:744
    at Object.execCb (require.js:1650)
    at Module.check (require.js:866)
  1. I look stupid.

I apologise for the snark in this comment. I stubbed the ga function queue when GA was disabled, so I did not find this until late in the development lifecycle and I have ... strong feelings.

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Jan 18, 2018
@andrewhowdencom
Copy link
Contributor Author

andrewhowdencom commented Jan 18, 2018

People who are similarly irate: I am going to get around this by simply stubbing the function queue directly after the snippet. This will catch these cases.

Admins, would you accept a PR to do the function queue before the analytics code, and be displayed regardless if it's enabled? It would not resolve the aforementioned add blocker issues (I think we need to take it out of require for that), but it would at least re-establish the function queue.

Wiser people than me: Don't couple your analytics to GA it's more fragile than you'd guess. API usage limits cause critical failures (for example)

@andrewhowdencom
Copy link
Contributor Author

Stubbing the function queue prevents fatal errors, but events lodged before analytics declares it's first create event are lost. Makes sense -- it will process them in order, and doesn't know which property against which to lodge the events until the create event is shown.

So, best solution to this:

  1. Move the entire snippet back to the document, or failing that
  2. Move both the function queue and the create event back to the document.

In the case GA is disabled, the function queue will still be declared, but will harmlessly accrue events.

@ghost ghost self-assigned this Aug 29, 2018
@ghost ghost added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Aug 30, 2018
@ghost
Copy link

ghost commented Aug 30, 2018

HI @andrewhowdencom , thank you for your report.
We were not able to reproduce this issue by following the steps you provided.
Please describe detail information about Steps to reproduce and Expected result, thank you.

@andrewhowdencom
Copy link
Contributor Author

Hi @engcom-backlog-nazar

To reproduce this issue would require extensive effort. The above describes the technical nature of the problem; to reproduce it it should be a case of inserting

                ga('send', {
                    'hitType': 'event',
                    'eventCategory': '__WHOO_CHICKEN_FEET__',
                    'eventAction': 'expand',
                    'eventLabel': shownId
                })

to the page HTML somewhere immediately after the require magic that brings in ga.js.

@ghost
Copy link

ghost commented Sep 3, 2018

@andrewhowdencom I'm no able to reproduce following steps you described. Have no error you are described in actual result.

@andrewhowdencom
Copy link
Contributor Author

Hola @engcom-backlog-nazar

Do you have a test environment (with admin rights)? I can probably just drop it in some sort of custom HTML field, and repro it there.

@ghost
Copy link

ghost commented Sep 3, 2018

@andrewhowdencom Which version you tested ? 2.2-develop ?

@andrewhowdencom
Copy link
Contributor Author

Nah, this would have been against a project directly, and it was before the GA changes that suppressed errors. It should still be reproduce against 2.2-develop though; the principle is the same (race condition where ga is not defined).

@ghost
Copy link

ghost commented Sep 3, 2018

@andrewhowdencom Ok i will try again.

@andrewhowdencom
Copy link
Contributor Author

All good. If you cant repro it close it, as we can just reopen it again in future if someone else finds it. I myself no longer work on Magento 2!

@ghost
Copy link

ghost commented Sep 19, 2018

@andrewhowdencom I'm closing this issue due to inactivity. If you'd like to update it, please reopen the issue.

@ghost ghost closed this as completed Sep 19, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Progress: needs update
Projects
None yet
Development

No branches or pull requests

2 participants