Skip to content
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

Bugfix - Initialize scripts in the correct order #585

Merged
merged 2 commits into from May 22, 2015

Conversation

jimmynotjim
Copy link
Contributor

Fixes an issue where common.js was initializing scripts in the wrong order

Additions

  • None

Removals

  • None

Changes

  • modified page specific scripts with init wrapper to ensure scripts are initialized in the correct order
  • ensured jQuery is required first to avoid undefined errors

Testing

  • checkout the branch and everything should load exactly the same. Issue is only seen when one scrip requires something from an earlier loaded script, like an event listener.

Review

Preview

None

Preview this PR without the whitespace changes

Notes

@sebworks
Copy link
Contributor

Getting a 'jQuery is undefined error when accessing 'http://localhost:7000/blog/were-making-progress-toward-ensuring-fair-access-to-credit/' by clicking the link on landing page.
Never-mind, re-ran grunt and all appears to be well. 👍

It is the Google Tag Manger that is causing the error.

@anselmbradford
Copy link
Member

It is the Google Tag Manger that is causing the error.

I'm pretty sure this is a hidden error here and on the live site TBH. Take a look at the live www site source, it loads jQuery after Google Tag Manager. GTM has the async attribute and is doing some fancy initialization, so the page continues to load and most likely jQuery will have loaded by the time it's needed. However, if jQuery takes longer to load than GTM, it'll run its code looking for jQuery before jQuery has been downloaded. I see nothing stopping that from happening in the live source. I suspect this could strike on weak 3G connections and the like.

@jimmynotjim
Copy link
Contributor Author

So that would mean we're not capturing stats on any browsing instance where the GTM beats out jQuery right?

@sebworks
Copy link
Contributor

@jimmynotjim , Great point. Needs to be investigated if we think it's occurring on production and thus the stats are bonked.

@jimmynotjim
Copy link
Contributor Author

Linking this conversation to #496. As far as the scope of this PR, everything good?

@sebworks
Copy link
Contributor

yep, 👍

@bethannenc
Copy link

Hi - I'm having issues following this thread. Is there someone I could hop on a call with? Are you all seeing the error on one page in particular? If so, I can test to see the impact on analytics. Chuck should also be included in this conversation.

@jimmynotjim
Copy link
Contributor Author

@bethannenc
Unfortunately the issue isn't consistent, showing up at random. Here's a screenshot

screen shot 2015-05-22 at 10 54 59 am

Emailing you the staging url so you can give it a test.

@bethannenc
Copy link

Is it only happening on pages where JQuery is not present? Or is it happening on pages that do have JQuery?

@wernerc - you should be roped into this as well.

@jimmynotjim
Copy link
Contributor Author

The issue is outside the scope of this PR (which will be closed once merged). Let's keep this discussion in #496 so that it's not lost once that happens.

@KimberlyMunoz
Copy link
Contributor

It looks like it's pretty consistently happening on slower connections or bigger pages. If you open Chrome Developer Tool's network emulator and set it to "Good 3G" or lower you can get the gtm.js errors consistently.

@bethannenc
Copy link

Are you all only seeing the error on demo or are you seeing it on www.consumerfinance.gov as well?

- modified page specific scripts with init wrapper to ensure scripts are initialized in the correct order
- ensured jQuery is required first to avoid undefined errors
@jimmynotjim
Copy link
Contributor Author

I can not replicate this on www.consumerfinance.gov. Looking at the page source, jQuery is defined before GTM (L:70 and L:85) respectively. @anselmbradford where did you see jQuery loaded after GTM?

As far as this PR, updated the changelog and merging.

jimmynotjim added a commit that referenced this pull request May 22, 2015
Bugfix - Initialize scripts in the correct order
@jimmynotjim jimmynotjim merged commit 1ebaa59 into flapjack May 22, 2015
@jimmynotjim jimmynotjim deleted the bugfix-init-order branch May 22, 2015 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants