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

Bug 1218466: Queue errors before GA loads. #3650

Merged
merged 1 commit into from Nov 19, 2015
Merged

Bug 1218466: Queue errors before GA loads. #3650

merged 1 commit into from Nov 19, 2015

Conversation

stephaniehobson
Copy link
Contributor

Try to avoid missing anything that happens before GA initializes.

Try to avoid missing anything that happens before GA initializes.
@groovecoder
Copy link
Contributor

A 9-line variation for the same 22 lines of logic?

            if (ga) {
                // GA blocked or not yet initialized - strip callback from data
                data.hitCallback = !ga.create ? null : data.hitCallback;
                ga('send', data);
            }
            // Always execute the callback
            if (callback) {
                callback();
            }

Eliminates the dupe code calls for ga('send', data) and callback().

@stephaniehobson
Copy link
Contributor Author

The ga send function will log and execute the callback if we leave it in the data.

@darkwing
Copy link
Contributor

data.hitCallback = (ga.create ? data.hitCallback: null);

I'd roll with positive first.

@groovecoder
Copy link
Contributor

@stephaniehobson I think my ternary logic there (or @darkwing's positive logic) should remove hitCallback from data the same as the existing logic.

@stephaniehobson
Copy link
Contributor Author

But we're always executing the fallback in your proposal, correct? In cases where we pass the callback to ga sucessfully the callback will be executed twice.

@groovecoder
Copy link
Contributor

Aha, yeah I overlooked that.

r+ 😄

groovecoder added a commit that referenced this pull request Nov 19, 2015
…r-ga

Bug 1218466: Queue errors before GA loads.
@groovecoder groovecoder merged commit 29b68ae into mdn:master Nov 19, 2015
@stephaniehobson stephaniehobson deleted the 1218466-queue-errors-for-ga branch November 25, 2015 00:40
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.

None yet

3 participants