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

use fetch + keepalive #113

Closed
gingerlime opened this issue Apr 9, 2022 · 9 comments
Closed

use fetch + keepalive #113

gingerlime opened this issue Apr 9, 2022 · 9 comments

Comments

@gingerlime
Copy link
Collaborator

bumped into https://css-tricks.com/send-an-http-request-on-page-exit/ which describes a more reliable way to make sure requests are sent, even when the user leaves the page, using fetch + keepalive. Not sure if older browsers support it, or if we can use a shim. Another option is to replace the jQuery get with a fetch if it's available, or fallback on the plain JS get.

@gingerlime
Copy link
Collaborator Author

@joker-777 what do you think?

@joker-777
Copy link
Collaborator

joker-777 commented Apr 11, 2022

@gingerlime Super interesting article. I wasn't aware of this behavior. In kenhub we use whatwg-fetch as a polyfill for fetch but we only load it when it doesn't exist. I would simply use fetch + keepalive and let the "developer" who uses Alephbet decide if he needs a polyfill or not.

@gingerlime
Copy link
Collaborator Author

So users can load a polyfill and the Alephbet code would automatically be able to use it?

@joker-777
Copy link
Collaborator

That's kind of what we do. We load this js code very early inline which checks if fetch or other methods exist and if one of them doesn't then we inject this script.

@joker-777
Copy link
Collaborator

Ah, and yes, once loaded, it is available for all js libraries.

@gingerlime
Copy link
Collaborator Author

Okay, but this feels like quite a lot of work for a user of Alephbet to have to worry about. I don't mind having a minimum-browser support, but then it needs to be documented clearly. Otherwise, some people might try to use it and will just get an error and won't necessarily know why. Or won't even know it can error in older browsers, and then their data is skewed.

The current JS fallback supports very old browsers, so we can also keep it like we do already (we check for jquery, but we can check if fetch is available instead). I think it's nicer to offer older-browser support out of the box if it's not super complicated for us to do that.

@joker-777
Copy link
Collaborator

I disagree. You would make the package bigger than necessary. Especially in the case of then we shouldn't worry about it because it is widely supported except for IE. If someone wants to support IE then they will use anyway polyfills which don't have to be by the way so "complicated" as we do it. They simply could include them and that's it.

@gingerlime
Copy link
Collaborator Author

We agree then :) I said it's fine to have a minimum browser support, but we just need to communicate it very clearly. And the code won't be much bigger even if we keep the fallback. We're talking about maybe 5 lines of code?

@joker-777
Copy link
Collaborator

Ah, ok, yes that's true. It doesn't save a lot of code.

gingerlime added a commit that referenced this issue Jun 25, 2022
#113

* instead of jQuery, we check if the browser supports `fetch`
  and use it with `keepalive: true`
* otherwise, fallback to plain JS
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

No branches or pull requests

2 participants