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

[Front-End] Implement persistent caching layer in the project #308

Closed
3 of 13 tasks
chadstewart opened this issue Sep 7, 2022 · 9 comments Β· Fixed by #437
Closed
3 of 13 tasks

[Front-End] Implement persistent caching layer in the project #308

chadstewart opened this issue Sep 7, 2022 · 9 comments Β· Fixed by #437

Comments

@chadstewart
Copy link
Contributor

chadstewart commented Sep 7, 2022

Type of feature

Select the type of feature request, the lowercase should also be the PR prefix.

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Current behavior

Is your feature request related to a problem? Please describe.

Currently the project is leveraging caching through the use of the SWR package. Through this issue we would like to explore a more robust caching solution.

Suggested solution

Describe the solution you'd like.

Here are some ideas based on the research I did:

  • SWR provides caching for data that uses the hook but it is an in memory cache and will clear as soon as the application is closed. I've found an article describing implementing a persistent cache using SWR here
  • @bdougie has implemented a solution that is also a persistent cache in the open-sauced repo, detailed here
  • Building a whole new caching solution ourselves leveraging React Hooks and Local Storage
  • @bdougie did some work on this on stream and found this article was helpful

Code of Conduct

By submitting this issue, you agree to follow our Code of Conduct.

  • I agree to follow this project's Code of Conduct

Contributing Docs

If you plan on contributing code please read - Contribution Guide

  • I agree to follow this project's Contribution Docs
@chadstewart
Copy link
Contributor Author

Hey @0-vortex, just created an issue for the caching discussion we had yesterday. Just wanted to make sure if this was the idea you wanted and I added some preliminary research I did.

Let me know if this was your thoughts on what we need for caching and what you think about some of the solutions presented so far.

@brandonroberts
Copy link
Contributor

Catching up on context here, but the options seem reasonable to me. How long do we expect the cache to persist before expiring? And how much data are we caching from the API?

@0-vortex
Copy link
Contributor

0-vortex commented Sep 8, 2022

Catching up on context here, but the options seem reasonable to me. How long do we expect the cache to persist before expiring? And how much data are we caching from the API?

IMHO that's all to be discussed/tested, the limitation being, if we have a list of 10 repos, we need to hit the contributors endpoints for each of the repositories, so 10 times; using stale revalidation in the background we can improve user experience by using any cache value right now, while fine tuning the details based on whatever we find to work best :D

@bdougie
Copy link
Member

bdougie commented Sep 9, 2022

Spent some time working on this for the hot repo on stream. This article in the docs was helpful in understanding

Currently the project is leveraging caching through the use of the SWR package. Through this issue we would like to explore a more robust caching solution.

Per the OP, we can add a provider that connects to local storage. Either localstorage directly or a mobx, redux, useContext etc. Correct me if I am wrong, the provider to localstorage is what is needed.

@0-vortex
Copy link
Contributor

Spent some time working on this for the hot repo on stream. This article in the docs was helpful in understanding

Currently the project is leveraging caching through the use of the SWR package. Through this issue we would like to explore a more robust caching solution.

Per the OP, we can add a provider that connects to local storage. Either localstorage directly or a mobx, redux, useContext etc. Correct me if I am wrong, the provider to localstorage is what is needed.

Exactly! πŸ‘

@bdougie
Copy link
Member

bdougie commented Sep 26, 2022

I spent some time Friday messing with this and localStorage and it didn't quite work. My thought is to put the ContextProvider in there instead. But I question if if the initial cache is not enough.

Pre-1.0 example using Context

bdougie added a commit that referenced this issue Sep 26, 2022
@bdougie bdougie mentioned this issue Sep 26, 2022
19 tasks
@OgDev-01
Copy link
Contributor

Here are some options i would consider us looking into for solving caching in this project

What Makes Recoil Different?

Recoil has unique advantages compared to other state management libraries used for React applications.

  • Fewer boilerplates.
  • Simple to learn and understand.
  • The data flow is simple to comprehend.
  • More in line with React.
  • Asynchronous data is queried using pure functions.
  • Hooks help us to retrieve and update states.

Link to their documentation Recoil

I've used this on some projects and understand quite to an extend how this works.

FWIW, I think we should check this out and see if it will be a good solution πŸ™πŸ™

@0-vortex
Copy link
Contributor

  • Switching to Recoil for state management

This is worth testing for state management, I've seen the other part of this called refine too.

This doesn't solve the SWR issue that we are having and it would not make sense to overload our current fetchers (that can do mutation outside of our global state) but it is definitely useful for some of the private information that belongs to a user that we would likely not cache with nextjs SWR always.

  • Hooks help us to retrieve and update states.

Not sure if this is a real plus but it's the only one I count πŸ•

Overall I think it's worth considering but during the state management talks and for that purpose only, making the global and application state that doesn't fully rely on github data more easier to manage and develop with! πŸ‘

bdougie added a commit that referenced this issue Sep 30, 2022
* feat: adds localstorage provider to cache

fix #308

* npm run format
github-actions bot pushed a commit that referenced this issue Sep 30, 2022
## [1.8.0-beta.5](v1.8.0-beta.4...v1.8.0-beta.5) (2022-09-30)

### πŸ• Features

* Add swr cache ([#437](#437)) ([6b2b1cb](6b2b1cb)), closes [#308](#308)
@github-actions
Copy link

πŸŽ‰ This issue has been resolved in version 1.8.0-beta.5 πŸŽ‰

The release is available on GitHub release

Your semantic-release bot πŸ“¦πŸš€

github-actions bot pushed a commit that referenced this issue Oct 3, 2022
## [1.9.0](v1.8.0...v1.9.0) (2022-10-03)

### πŸ› Bug Fixes

* add truncate to contributors repositories list tab ([#472](#472)) ([b7ec96e](b7ec96e)), closes [#460](#460)
* fix merged PRs that looks like closed  ([#475](#475)) ([1dd820a](1dd820a)), closes [#462](#462)
* issue template πŸ‘‹πŸΎ ([31b7a7e](31b7a7e))
* moved page head and footer to repo page to avoid reloading the entire screen ([#473](#473)) ([bf790e8](bf790e8)), closes [#449](#449)
* replace churn with overview ([#470](#470)) ([fc0fe3f](fc0fe3f)), closes [#469](#469)
* SEO descriptions ([980e01d](980e01d))

### πŸ• Features

* add filters to hooks for repos and contributions ([#458](#458)) ([05e6bbe](05e6bbe)), closes [#457](#457)
* add page title and info to repo and contributors page ([#459](#459)) ([c0000e4](c0000e4)), closes [#446](#446)
* add pagination to contributors page ([#454](#454)) ([37f4da8](37f4da8)), closes [#403](#403)
* Add swr cache ([#437](#437)) ([6b2b1cb](6b2b1cb)), closes [#308](#308)
* connect scatter chart to contributors data ([#466](#466)) ([be78024](be78024)), closes [#465](#465)
* hide show/hide buttons temporarily ([#474](#474)) ([2ce5b3c](2ce5b3c)), closes [#439](#439)
* Leverage cloudinary for round images ([#467](#467)) ([7dc64d8](7dc64d8)), closes [#373](#373)
* update filters for repos and contributors ([#464](#464)) ([e15bbf8](e15bbf8)), closes [#461](#461)
ElpisHelle added a commit to ElpisHelle/next.js-tailwindcss that referenced this issue Aug 17, 2023
## [1.8.0-beta.5](open-sauced/app@v1.8.0-beta.4...v1.8.0-beta.5) (2022-09-30)

### πŸ• Features

* Add swr cache ([#437](open-sauced/app#437)) ([6b2b1cb](open-sauced/app@6b2b1cb)), closes [#308](open-sauced/app#308)
ElpisHelle added a commit to ElpisHelle/next.js-tailwindcss that referenced this issue Aug 17, 2023
## [1.9.0](open-sauced/app@v1.8.0...v1.9.0) (2022-10-03)

### πŸ› Bug Fixes

* add truncate to contributors repositories list tab ([#472](open-sauced/app#472)) ([b7ec96e](open-sauced/app@b7ec96e)), closes [#460](open-sauced/app#460)
* fix merged PRs that looks like closed  ([#475](open-sauced/app#475)) ([1dd820a](open-sauced/app@1dd820a)), closes [#462](open-sauced/app#462)
* issue template πŸ‘‹πŸΎ ([31b7a7e](open-sauced/app@31b7a7e))
* moved page head and footer to repo page to avoid reloading the entire screen ([#473](open-sauced/app#473)) ([bf790e8](open-sauced/app@bf790e8)), closes [#449](open-sauced/app#449)
* replace churn with overview ([#470](open-sauced/app#470)) ([fc0fe3f](open-sauced/app@fc0fe3f)), closes [#469](open-sauced/app#469)
* SEO descriptions ([980e01d](open-sauced/app@980e01d))

### πŸ• Features

* add filters to hooks for repos and contributions ([#458](open-sauced/app#458)) ([05e6bbe](open-sauced/app@05e6bbe)), closes [#457](open-sauced/app#457)
* add page title and info to repo and contributors page ([#459](open-sauced/app#459)) ([c0000e4](open-sauced/app@c0000e4)), closes [#446](open-sauced/app#446)
* add pagination to contributors page ([#454](open-sauced/app#454)) ([37f4da8](open-sauced/app@37f4da8)), closes [#403](open-sauced/app#403)
* Add swr cache ([#437](open-sauced/app#437)) ([6b2b1cb](open-sauced/app@6b2b1cb)), closes [#308](open-sauced/app#308)
* connect scatter chart to contributors data ([#466](open-sauced/app#466)) ([be78024](open-sauced/app@be78024)), closes [#465](open-sauced/app#465)
* hide show/hide buttons temporarily ([#474](open-sauced/app#474)) ([2ce5b3c](open-sauced/app@2ce5b3c)), closes [#439](open-sauced/app#439)
* Leverage cloudinary for round images ([#467](open-sauced/app#467)) ([7dc64d8](open-sauced/app@7dc64d8)), closes [#373](open-sauced/app#373)
* update filters for repos and contributors ([#464](open-sauced/app#464)) ([e15bbf8](open-sauced/app@e15bbf8)), closes [#461](open-sauced/app#461)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants