Cache requests client side #40
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Well that's disappointing. |
|
@WilliamMcCumstie what you had here was a good start, but it wasn't quite right when it came to clearing the cache.
|
POST/PUT/PATCH/DELETE requests trigger the cache to be cleared
AKA dividing by 60,000 is too hard for Ben.
Without this we don't pull in the bug fix that has been blocking us adding this. The new version allows caches to expire even if they are being used.
`useFetch` has a couple of different ways of being called. 1. Pass an array of dependencies as the third argument to `useFetch`. This will cause the request to be made immediately and again each time the given array of dependencies changes. This is how we make GET requests. 2. Do not pass a third arguments to `useFetch`. The request will not be executed until `patch`, `put`, `del`, `get` or `post` is called on the request object. This is how we make `POST` requests. The important thing to not above is that we cannot clear the cache when we construct the request in the second form, we need to wait until the request is executed. I've added an response interceptor which clears the cache when the relevant requests are exectued. It only does so if the response is OK. I've also changed the way in which the cache was being cleared. We now clear the cache via the `useFetch` API. We can get a copy of the entire cache by calling `useFetch` without a path/url. I've also made sure that we clear both the memory and persisted caches. Probably overkill.
benarmston
force-pushed
the
dev/cache-control
branch
from
6b44cf7 to
f58db16
Compare
|
Rebased on latest |
|
Unfortunately, when caching is used |
Unfortunately, we need to construct the cache key, as use-http doesn't expose that.
benarmston
force-pushed
the
master
branch
2 times, most recently
from
6069d25 to
4a8efc3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The client now caches its API requests by using the inbuilt mechanism within
use-fetch.I initially implemented the
CacheControlheaders server side, to allow greater control over how it works. Unfortunately,use-fetchignores these headers. The commit was reverted here: 29ab890The cache needs to be persisted so it gets added to the
local_storage. This allows for the cache to be destroyed on non-GET requests.Currently the entire cache is being cleared on such a requests. This is for two reasons:
includesfor related requests will be correct, andlocal_storagekeys are hard to predicted as the contain the URL and query stringe.g.
url:/dev/job-scripts/api/v0/scripts?include=template||method:GET||body::