plausible / analytics

Simple, open source, lightweight (< 1 KB) and privacy-friendly web analytics alternative to Google Analytics.
https://plausible.io
GNU Affero General Public License v3.0
20.44k stars 1.09k forks source link

`exclusions` extensions does not work with `hash` extension #2167

Closed vincent-czi closed 2 years ago

vincent-czi commented 2 years ago

Past Issues Searched

Issue is a Bug Report

Using official Plausible Cloud hosting or self-hosting?

Plausible Cloud from plausible.io

Describe the bug

The exclusions extension appears to not work when capturing hash-based routing via the hash extension.

I'm developing locally with this script src https://plausible.io/js/script.exclusions.hash.local.js. For non-excluded pages, everything is working great. However, I've included a data-exclude with value of "/#/fetch/**". I can see the attribute is correctly set in the dom, but Plausible is still sending pageview events when I visit routes under that, eg /#/fetch/sensitiveData. I thought it might be that the # character was messing things up, but changing the data-exclude to "/#/fetch/**" did not fix it either.

Looking into the code here, I think the issue is that the exclusions portion of the tracker code relies on the function pathMatches, which itself relies on location.pathname, so it's only extracting the non-hash part of the URL, so it's impossible to exclude anything hash-based there.

Expected behavior

The excludes extension to work the same for hash-based routing.

Screenshots

No response

Environment

- OS: MacOS
- Browser: Chrome
- Browser Version: 104.0.5112.101 (Official Build) (x86_64)
RobertJoonas commented 2 years ago

Thanks for reporting that @vincent-czi!

Indeed, a script.exclusions.hash.js extension wrongly ignores the hash part of the URL when excluding pages. I've created a PR with a fix for this.

vincent-czi commented 2 years ago

That is a fast fix! Much appreciated!

vincent-czi commented 2 years ago

@ukutaht Hi, I looked over the PR and I'm sure it will fix the issue, but it doesn't seem to have gone live yet. I see it's been merged, but the bug is still present (even after clearing my cache). When I grab the script (https://plausible.io/js/script.exclusions.hash.local.js) it looks like it hasn't actually changed to represent the merged PR yet.

I don't know your deploy process: if it's been merged, do you have an expectation of when I'll start seeing the change in the script I'm loading? Thanks!

ukutaht commented 2 years ago

Will deploy it tomorrow. Sorry we'll make it clearer what's deployed and what isn't in the future :)

metmarkosaric commented 2 years ago

it's live now!

vincent-czi commented 2 years ago

Great! Can confirm it works as expected!

Sadly, I realize now that I missed the forest for the trees: it appears that using a HashRouter with react-router-dom means that hash changes do not register as hashchange events because they go through the history.pushState API instead. This appears to work as designed and is not the fault of Plausible, but it means that a pretty common React + react-router set up using the Plausible hash extension won't actually work as expected. I'll write up another issue and you guys can decide what you think of it there. (Edit: new issue here -- https://github.com/plausible/analytics/issues/2203)

Regardless, thanks for the fast fix on this!