search

okgrow / analytics

UNMAINTAINED! - Complete Google Analytics, Mixpanel, KISSmetrics (and more) integration for Meteor
https://atmospherejs.com/okgrow/analytics
MIT License
213 stars 61 forks source link

Package breaks back navigation on Meteor 1.6 if FlowRouter is used #203

Closed derwaldgeist closed 6 years ago

derwaldgeist commented 6 years ago

As described here in detail:

https://github.com/meteor/meteor/issues/9419

after upgrading my app from Meteor 1.5 to 1.6 I faced the problem that the browser back button did not work anymore if the target of the back navigation was the initial page the app had been loaded with.

After hours of search, I could boil this down to a "monkey patched" history.pushState() function introduced by okgrow:analytics. Once I take this package out, everything works fine. Obviously, this monkey patch worked until Meteor 1.5, but is not compatible with Meteor 1.6 anymore.

RichardLitt commented 6 years ago

Thanks for logging and researching this issue, @derwaldgeist! Noted.

cfnelson commented 6 years ago

@derwaldgeist Thanks for adding this issue. Could you update the issue title to mention FlowRouter as this bug is specific to FlowRouter. I am also curious to know which versions of this package and flow router you are using. Is their any version changes in these two pkgs when you upgrade to Meteor 1.6.

Also the code related to this is located over in our auto-analytics package. Previously have had other issues relating to flow-router and this integration due to the way it handles history state.

I also suspect that it's not the monkey patch but logPageFirstLoad where we use window.history.replaceSate I am guessing this is why path disappears from the state object.

cfnelson commented 6 years ago

A fix has been created for the above bug and a PR has been opened on our auto-analytics pkg.

We will publish a new meteor analytics package once @okgrow/auto-analytics@1.0.6 is released.

For further details you can read this comment here.

derwaldgeist commented 6 years ago

Thanks again for the very fast fix! Changed the title so it reflects FlowRouter.

derwaldgeist commented 6 years ago

Shall I close this issue or do you want to keep it open until the update has been published?

cfnelson commented 6 years ago

@derwaldgeist No problems. Lets keep this open until we publish the fix. As I am waiting for a code review on the original PR & will then need to make a new release for meteor analytics once the npm pkg has been published.

cfnelson commented 6 years ago

@derwaldgeist v3.0.5 is published 🎉 . I have tested locally with the Flow Router example and all seems good! 😄 Let me know how the latest release goes for you!

Going to mark this issue as resolved, but please open back up if you have any issues. 👍

derwaldgeist commented 6 years ago

Yeah, I can confirm it works after the update. Great work, thanks! 👍