gitify-app / gitify

GitHub notifications on your menu bar. Available on macOS, Windows & Linux.
https://www.gitify.io
MIT License
4.38k stars 256 forks source link

Response caching with conditional requests #1124

Open setchy opened 1 month ago

setchy commented 1 month ago

📝 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

dammy95 commented 2 weeks ago

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 😄

dammy95 commented 2 weeks ago

Hey @setchy – I have some options on how we can implement this:

1) 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.

2) 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!

setchy commented 2 weeks ago

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

afonsojramos commented 2 weeks ago

We're already using axios, so I'd suggest taking advantage of that and its large community! Easy solutions are usually the best ones 😁🚀

setchy commented 2 weeks ago

The other option would be to go down this route https://github.com/gitify-app/gitify/issues/823

afonsojramos commented 2 weeks ago

@setchy how does that tackle this issue? Do they have built-in caching or something into their client?

setchy commented 2 weeks ago

@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

afonsojramos commented 2 weeks ago

Alright!!! Then that looks like a great solution to me 🚀

@dammy95, as the person assigned to the issue, is that okay with you?

dammy95 commented 2 weeks ago

Yupp that sounds great to me @afonsojramos @setchy – I'll look in to the octokit refactor solution and create a PR for review 🚀

setchy commented 1 week ago

I guess there is also the option of using react-query (or similar) - https://www.welcomedeveloper.com/posts/managing-cache-react-query/

afonsojramos commented 3 days ago

Might be the most extensible (and widely-used) option honestly, don't know how I forgot about that one 😬

setchy commented 3 days ago

Another library I saw this week - https://github.com/alexcambose/custom-cache-decorator