-
Notifications
You must be signed in to change notification settings - Fork 4
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
Support SendBeacon? #3
Comments
It's something I was thinking about. Do you want to send a pull request? |
Haha I should have looked here earlier! I added server side support. Right now chrome doesn't accept I tested this on the client and it worked with the above PR (I will leave the PR here for others). try {
var json = JSON.stringify(opts)
} catch (e) {
console.log(opts)
return Promise.reject(new Error('could not strigify data'))
}
var blob = new window.Blob([ json ], { type: 'text/plain; charset=UTF-8' })
return window.navigator.sendBeacon(url, blob) |
FYI - In general, the json support for sendBeacon seems a bit broken and strangely implemented across browsers. See this issue for more discussion. |
@vesparny @joehand Based on that info, I'm not sure I should create a PR. The complications of falling back to Feel free to close this. |
I have something working for this using application/x-www-form-urlencoded instead of application/json (I also had to fork the fair-analytics server to support form encoded POST bodies, but it was a one-liner using bodyParser). I haven't tested it across many browsers but it works for Chrome. I fall back to @joehand's suggestion of just using text encoding seems more obvious though and maybe better than form encoding? Not sure. At the moment I'm running the code above for my client currently since I need to be able to send events when the page unloads. P.S. Now realizing maybe some folks are sending hierarchical analytics data in which case form encoding wouldn't cut it, so JSON-as-plain-text is probably better for that. P.P.S. Note that unlike what @AutoSponge originally proposed, this doesn't use |
Just now reviewing my code, I realized I forgot to add a promise return value for the fallback XHR case, but I can add that if we want a PR. |
I updated my fork to use text-as-json and return a promise for the xhr response. master...ea909c9 |
Could this support sendBeacon by default instead of fetch?
The text was updated successfully, but these errors were encountered: