Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

don't depend on window.ga till it's actually needed #6066

Merged
merged 2 commits into from Oct 29, 2019

Conversation

peterbe
Copy link
Contributor

@peterbe peterbe commented Oct 29, 2019

Fixes #5986

@peterbe peterbe requested a review from Gregoor October 29, 2019 17:11
Copy link
Contributor

@Gregoor Gregoor left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@codecov-io
Copy link

Codecov Report

Merging #6066 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6066   +/-   ##
=======================================
  Coverage   95.76%   95.76%           
=======================================
  Files         297      297           
  Lines       25466    25466           
  Branches     1889     1889           
=======================================
  Hits        24387    24387           
  Misses        848      848           
  Partials      231      231

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea715ff...d0b268e. Read the comment docs.

@tobinmori tobinmori added the p1 High Priority: Current sprint label Oct 29, 2019
Copy link
Contributor

@escattone escattone left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @peterbe!

@@ -4,9 +4,13 @@ import { useContext, useEffect, useState } from 'react';

export type GAFunction = (...any) => void;

const noop: GAFunction = () => {};
function ga(...args) {
if (typeof window === 'object' && typeof window.ga === 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering, if in the future we should consider making this "Ghostery"-proof like we do here in kuma/static/js/analytics.js? Something to think about, but not for this PR.

@escattone escattone merged commit 30d0ee1 into mdn:master Oct 29, 2019
@peterbe peterbe deleted the 5986-drop-in-ga-traffic branch October 29, 2019 17:46
@tobinmori tobinmori added this to the Rissi Palmer (Q4 Sprint 2) milestone Nov 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p1 High Priority: Current sprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop in GA traffic
5 participants