Closed A5rocks closed 1 year ago
@ukutaht Hello! Just making sure you've seen this PR. Do you have any opinions for whether navigator.sendBeacon
should be the default (as mentioned in #16 or at the end of my PR explanation)?
Additionally you need to allow (approve?) the workflow to run :P
Hi!
We have considered it but have decided against using sendBeacon at this time. Sorry :/
Alright, thanks.
We have considered it but have decided against using sendBeacon at this time. Sorry :/
@ukutaht Are there any reasons for this? Just curious :)
The main reason is that we don't currently have team capacity to evaluate this change properly. We'd need to evaluate this approach before choosing to adopt it.
Adding sendBeacon
with progressive enhancement will certainly do the following:
In order to accept long-term maintenance of this complexity, strong arguments must be made. The main pieces of data I'm looking for are:
navigator.sendBeacon = function() {}
These two numbers should give us good enough understanding to make a decision.
As someone who also desires this feature it's a little disappointing to see this closed and I'm sure other people are too.
- Increases JS payload size
- Make the script more complicated 2.1 Harder to use and understand 2.2. More opportunities for errors 2.3 Harder to maintain over time
Whilst I understand these points being a concern, the PR itself is about 5 extra lines of code, of which the majority is just adding the new useSendBeacon
parameter. I wouldn't consider this extra complexity, personally, just extra flexibility to those who desire it, especially since the parameter is optional. Also, the error opportunities would most likely be the fault of the browser's implementation of sendBeacon, less so any change this PR creates. Maintenance-wise, with the way it's implemented, it shouldn't really need any.
- How much bigger does the JS payload get
Using yarn build:module
, the built request.js
file increases from 3,319 bytes to 3,710 bytes (~11.7%). Considering this also includes the new code comments, when minified I imagine this will barely be a dent in the overall bundle size. I'm aware Plausible is trying to be as lightweight as possible, but I believe that missing out on this feature for a (relatively) small increase in size is a bit extreme.
- What's the % increase/decrease in reliability when using sendBeacon? 2.1 Need to test with different audiences as adblockers set navigator.sendBeacon = function() {}
The reliability would actually be increased. I don't have percents for you, but considering sendBeacon's purpose it would pretty much guarantee an event would be sent even after they navigate away from the page. It wouldn't be any less reliable than the current method. Regarding adblocking, I've experienced Plausible being adblocked on a few that I've tried so it's not much of a concern here if they no-op sendBeacon.
FWIW, I'm evaluating Plausible for my company and using sendBeacon is one of our requirements. I'm just going to rewrite the JS myself to make it work. The flipside of Plausible's JS being 3KB is you can rewrite it yourself in an afternoon.
If you do end up rewriting it and decide it's good enough to share, mind doing so? We were going to use a rewritten version but decided that would be too much work! (I still have our untested version on disk... heh. It's for some time in the future, I suppose.)
Description
Use a better method of analytics posting.
Related Issue
Fixes #16.
Types of changes
Checklist:
I think a followup to this would be using
navigator.sendBeacon
unlesscallback
is passed. I would be OK with adding that into this.