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

[1.0] Fixes 1317 Google Analytics plugin; updates attachHistory listener logic #1318

Merged
merged 5 commits into from
Jul 1, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# editorconfig.org
Copy link
Contributor

Choose a reason for hiding this comment

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

ah nice!

root = true

[*]
indent_style = space
indent_size = 2
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

[{*.json, *.svg}]
indent_style = space
indent_size = 4

# Matches the exact package.json, or *rc
[{package.json,*.yml,*rc}]
indent_style = space
indent_size = 2
4 changes: 2 additions & 2 deletions packages/gatsby-plugin-google-analytics/src/gatsby-browser.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
exports.onRouteUpdate = function(location) {
exports.onRouteUpdate = function({ location }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added destructuring, I guess I misread the plugin docs regarding the src folder.

// Don't track while developing.
if (process.env.NODE_ENV === `production`) {
ga(`set`, `page`, location.pathname)
ga(`set`, `page`, (location || {}).pathname)
ga(`send`, `pageview`)
}
}
10 changes: 6 additions & 4 deletions packages/gatsby/src/cache-dir/production-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,13 @@ window.___navigateTo = navigateTo
const history = createHistory()

function attachToHistory(history) {
window.___history = history
if(!window.___history) {
window.___history = history

history.listen((location, action) => {
apiRunner(`onRouteUpdate`, { location, action })
})
history.listen((location, action) => {
apiRunner(`onRouteUpdate`, { location, action })
})
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, also noticed that this same code exists within:

attachToHistory(routeProps.history)

But the production-app.js file is the root cause

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, root.js is for the development version of Gatsby. It should be fixed too but since you're not generally tracking yourself in development it's not as urgent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well I went ahead and adjusted it too!

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect :-)

Thanks! Pulling this in now!

}

function shouldUpdateScroll(prevRouterProps, { location: { pathname } }) {
Expand Down