Feature: add stale flag in request return object #843
Conversation
src/cache/axios.ts
Outdated
| * | ||
| * @see https://axios-cache-interceptor.js.org/config/response-object#stale | ||
| */ | ||
| stale: boolean; |
There was a problem hiding this comment.
this type might be undefined in some areas, probably a stale?: boolean is a better type
| @@ -348,6 +348,7 @@ export function defaultResponseInterceptor(axios: AxiosCacheInstance): ResponseI | |||
|
|
|||
| return { | |||
| cached: true, | |||
| stale: true, | |||
There was a problem hiding this comment.
you can't you use .cached here? since this boolean will always be true when cached is true too?
There was a problem hiding this comment.
I see your point. In other cases stale may be undefined/false when cached is true.
There was a problem hiding this comment.
Are there other cases where cache is true and the state is stale?
There was a problem hiding this comment.
What I meant was:
why response.cached cannot be used in your case? Since both these values will always have the same value?
There was a problem hiding this comment.
They will be the same when staleIfError is set, but is other cases response.stale may be falsy when response.cached is true. For example: when a request is made, and the cache is still valid (i.e. now() < ttl), then the cache is returned with {...repsonse, cached: true, stale: false}. Does that make sense?
There was a problem hiding this comment.
@MerryCello, all tests you added, stale was the same booleanish value as cached. I still don't see how this would make any difference.
There was a problem hiding this comment.
@arthurfiorette, I added more tests in cases where cached: true and stale: false to show where stale won't always have the same value as cached. This might make it a little more clear. Please review and lmk what you think.
There was a problem hiding this comment.
Hi @arthurfiorette, just following up on this since it's been a while. Let me know if you have further questions.
|
please also add tests |
MerryCello
changed the title
|
Ci is failing |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #843 +/- ##
==========================================
+ Coverage 99.26% 99.27% +0.01%
==========================================
Files 19 19
Lines 2442 2479 +37
Branches 212 214 +2
==========================================
+ Hits 2424 2461 +37
Misses 17 17
Partials 1 1 ☔ View full report in Codecov by Sentry. |
|
@arthurfiorette, I added some tests and fixed the failing ones. I also added some comments as replies above that may have been missed. If you could review those, that would be great. |
This PR is an attempt to add the request response "stale" flag that is present in the axios-cache-adapter. This flag says whether or not the response is from staled cache. This would be returned along with the "cached" flag. Please let me know if there's anything that I'm missing.