feat: add sendBeacon request parameters hook #555
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change introduces a new hook called `resolveSendBeaconRequestParameters`. This enables consumers to modify a subset of the RequestInit parameters being used by the fetch request that polyfills the navigator.sendBeacon API in the worker context, e.g. setting keepalive: false
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Thank you @hoebbelsB |
|
I just went on vacation after opening the pr. Will update with tests when I'm back :) |
|
Thanks for your help, have a nice vacation 🏝️ |
|
hey @gioboa just added the tests, please check if this is sufficient :) |
mhevery
approved these changes
Mar 1, 2024
gioboa
approved these changes
Mar 2, 2024
There was a problem hiding this comment.
@hoebbelsB the core team verified it, that's a great piece of code.
Thanks for your help.
|
Amazing work, thanks so much for this PR! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is it?
Description
This change introduces a new hook called
resolveSendBeaconRequestParameterson thePartytownConfig. This enables consumers to modify a subset of theRequestInitparameters being used by the fetch request that polyfills the navigator.sendBeacon API in the worker context, e.g. setting keepalive: false.Alternative solution
An alternative solution would be to just set
keepalive: falseas default. But as I am not aware of the reasoning behind your defaults, I thought it's a good idea to make it user configurable.Use cases and why
The current implementation of the
sendBeaconhas static values for theRequestInitparameters it uses to initialize the post request. The goal is to make those parameters being user configurable.I am not 100% sure why you have chosen to go with those default values, but I've notice that espcially
keepalive: trueis causing troubles in combination with google analytics (analytics.js).I've already pointed out the behavior it causes in this issue #492, but at that point I didn't know the root cause is
keepalive: true.Short description of what happens with some visuals.
The application starts behaving normal, the first couple of requests sent via
sendBeaconare actually fine and not producing any errors. Until a certain amount of events was sent, most of the following requests are simply inpendingstate forever.The "failing" requests are also producing errors like crazy
Lastly, I've figured out that also the "good" requests are having issues, as they seem to actually never finish until they run into some timeout.
After manually setting
keepalive: falseand override the script locally, the application suddenly recovers and all consequent requests are perfectly fine. It looks like the never finishing requests are filling some queue which causes consequent requests to be stalled.Checklist: