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

[Explainer] Add alternative approach to extend sendBeacon #68

Merged
merged 4 commits into from
Mar 8, 2023

Conversation

mingyc
Copy link
Collaborator

@mingyc mingyc commented Mar 8, 2023

This change summarizes the discussion from
WebKit/standards-positions#85 (comment) and also potentially unblocks the need to add custom headers in #50

@mingyc mingyc merged commit 0af026c into WICG:main Mar 8, 2023
To support the [requirements](#requirements) and to make the new API backward compatible, we propose the following shape:

```ts
navigator.sendBeacon(url, fetchOptions): PendingBeacon
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this can work like this. data is the second argument, this would have to be

navigator.sendBeacon(url, data, fetchOptions): PendingBeacon

which would mean that body below might not be needed and maybe data would be null for a GET or would be the URL.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in #69


The reason why `signal` and `AbortController` is not desired is that the proposal would like to supports more than just aborting the requests, it also needs to check pending states and accumulate data. Supporting these requirements via the returned `PendingBeacon` object allows more flexibility.

The above API itself is enough for the requirements (2) and (3), but not for (1), which requires delaying the request. As the `sendBeacon` semantic doesn't make sense for the "delaying" behavior, proposing a new function is better:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's OK to mention this argument but it's not a technical limitation, we could add an option to the say that when options are present, it doesn't send immediately. I think that's a confusing API but there's no technical reason blocking it (I think).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I want to make it clear that this is not a good approach. I've added another alternative "Introducing queueBeacon() API" below, which is rather preferred.

mingyc added a commit to mingyc/pending-beacon that referenced this pull request Mar 9, 2023
mingyc added a commit that referenced this pull request Mar 9, 2023
@mingyc mingyc deleted the explainer-alt-sendbeacon branch April 7, 2023 05:42
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