gitify
gitify copied to clipboard
Response caching with conditional requests
📝 Provide a description of the new feature
Add response caching so that we can enable additional features (ie: show read notifications, fetch >50 (all) notifications) without the concerns of being rate-limited.
Using conditional requests should be sufficient, which requires us to store and use response etag or last-modified headers in subsequent GET requests to the same resource.
Changes required to https://github.com/gitify-app/gitify/blob/main/src/utils/api/request.ts
➕ Additional Information
No response
Hi @setchy 👋🏾 – I can look into this. I'll do some research and do a small write-up for you to review before I start. Let me know if that works for you 😄
Hey @setchy – I have some options on how we can implement this:
-
Using axios-cache-interceptor. Here's a good read by its author. This creates an axios interceptor that adds the etag (or last-modified) headers to the request and return the appropriate response.
-
Implement step 1) above without the axios-cache interceptor library, using localStorage to store the cached response.
Somehow I sense that I could be overcomplicating the implementation? Let me know what you think, thanks!
Great research @dammy95.
I don't have a strong view on the best way to implement this. Happy to try and few things and adapt as we move forward
We're already using axios, so I'd suggest taking advantage of that and its large community! Easy solutions are usually the best ones 😁🚀
The other option would be to go down this route https://github.com/gitify-app/gitify/issues/823
@setchy how does that tackle this issue? Do they have built-in caching or something into their client?
@setchy how does that tackle this issue? Do they have built-in caching or something into their client?
I believe so - https://github.com/octokit/octokit.js/issues/890#issuecomment-1335979030
It also has built in throttling for primary and secondary quota exhaustion
Alright!!! Then that looks like a great solution to me 🚀
@dammy95, as the person assigned to the issue, is that okay with you?
Yupp that sounds great to me @afonsojramos @setchy – I'll look in to the octokit refactor solution and create a PR for review 🚀
I guess there is also the option of using react-query (or similar) - https://www.welcomedeveloper.com/posts/managing-cache-react-query/
Might be the most extensible (and widely-used) option honestly, don't know how I forgot about that one 😬
Another library I saw this week - https://github.com/alexcambose/custom-cache-decorator
Could also completely replace axios with https://www.npmjs.com/package/make-fetch-happen (the fetch client used by npm)
Notifications are fetched by an interval which is not ideal, because if the connection is slow requests can stack and make multiple requests at once.
I'm not familiar with the way electron runs timing functions, but a better way would be to use setTimeout recursively. So when a request is finished a new timeout is started.
As an alternative, I can suggest using react-query which handles caching, retries, interval fetching, and request deduplication. Unless it was considered and didn't fit the project.