Skip to content
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

Change the default value for keepalive to false #1359

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

matus-tomlein
Copy link
Contributor

This PR disables fetch keepalive by default.

The reason for this is the 64KB limit that browsers put on the keepalive requests. Although the default maxPostBytes is under this limit so we shouldn't run into for single requests, the limit is shared for parallel requests. We have seen this when testing in internal apps that have multiple initialized trackers that send events at once – this caused the requests to go into a pending state and fail.

A related problem was reported in #1355 – I think we were able to fix this for the case where there is a single tracker instance sending events, but not for the multiple tracker instances case. Since we can't fix it for that case, we can't keep the default setting to true.

Copy link

bundlemon bot commented Oct 23, 2024

BundleMon

Files added (6)
Status Path Size Limits
trackers/javascript-tracker/dist/sp.js
+24.28KB 30KB / +10%
libraries/browser-tracker-core/dist/index.mod
ule.js
+23.45KB 25KB / +10%
libraries/tracker-core/dist/index.module.js
+19.21KB 20KB / +10%
trackers/browser-tracker/dist/index.umd.min.j
s
+17.22KB 20KB / +10%
trackers/javascript-tracker/dist/sp.lite.js
+17.15KB 20KB / +10%
trackers/browser-tracker/dist/index.module.js
+3.49KB 5KB / +10%

Total files change +104.79KB 0%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history

Copy link
Contributor

@jethron jethron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

All "solutions" to this problem continue to be cursed, I guess. 😅

@matus-tomlein matus-tomlein merged commit 8ec9603 into release/4.0.0 Oct 24, 2024
3 checks passed
@matus-tomlein matus-tomlein deleted the issue/keepalive_default_false branch October 24, 2024 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants