swr
swr copied to clipboard
feat: Keep track `updatedAt` as part of the SWR state
Seeding work to keep track of the updatedAt
time. #1703
At the top of my head I can think of a few things to do:
- [x] Are tests exhaustive enough?
- [ ] Updates to other parts of the SWR API (?)
- [ ] Documentation
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit be0c42e0f27ce0ef4765e02aa54c2dc2ece630cf:
Sandbox | Source |
---|---|
SWR-Basic | Configuration |
SWR-States | Configuration |
SWR-Infinite | Configuration |
SWR-SSR | Configuration |
Thank you @icyJoseph — the implementation looks great! One more thing I think we should include in the test is reflecting the updatedAt
state change in other hooks, that are using the same key.
And also, I'd like to hear your idea about the name "updatedAt" here. Should we call it "fetchedAt"? Simply because I've been thinking about this "updatedAt" name for a while and it feels that the mutate()
calls should be counted as "updates" too. But in some cases their updates are not that easy to track.
Thank you @icyJoseph — the implementation looks great! One more thing I think we should include in the test is reflecting the
updatedAt
state change in other hooks, that are using the same key.
Right, if we have two or more hooks looking at the same key, updatedAt
ought to be the same in those. I'll write a test for it.
And also, I'd like to hear your idea about the name "updatedAt" here. Should we call it "fetchedAt"? Simply because I've been thinking about this "updatedAt" name for a while and it feels that the
mutate()
calls should be counted as "updates" too. But in some cases their updates are not that easy to track.
I see, mutate calls can take many shapes, from a simple call to fetch fresh data, to a controlled call where we are in total control of the new data.
I do agree that fetchedAt
would be more appropriate if the intention is to tell the user when was the last time the fetcher was called, rather than the last time the data was mutated
.
Here, broadcastState
is called twice, src/utils/mutate.ts
wouldn't it be enough to get Date.now()
before calling broadcast state in these cases?
Here, broadcastState is called twice,
src/utils/mutate.ts
wouldn't it be enough to getDate.now()
before calling broadcast state in these cases?
It's a bit tricky, since the updatedAt
state only exists locally in the hook, not in the cache. So when a useSWR
hook mounts, it can't know what's the last time the data gets updated.
After some research, I think we should change the structure of the cache into "key → { data, error, isValidating, ... }". Currently it is "data_key → data", "error_key → error", ... which is not very extendable.
I agree with your last point there.
Just as an update, over the holidays I wrote a test to check that other hooks consuming key
should update their updatedAt
key to the latest.
For better or worse, the test failed, so I've been making some changes to turn it green.
While I am at, I've come to believe that eitherlastFetchedAt
or fetchedAt
make much more sense.
Now I am back from my break. I see I've been making an error on my tests. Mostly the call to mutate, and expecting that calls to it should reflect similar updatedAt
in the rest of hooks.
One more thing I think we should include in the test is reflecting the updatedAt state change in other hooks, that are using the same key.
In this case, we are talking purely about calls to revalidate
, for instance, when a hook mounts with `revalidateOnMount, or when the window is refocused, right?
I'll rewrite the two tests that use mutate
. I think we are nearly there with this feature.
@shuding how do you feel about the PR now? I think we just need to decide a name for the prop:
It would be helpful to introduce a new state updatedAt, which is a timestamp representing the last time we get the data from the fetcher or mutator.
-
updatedAt
: As you said before, this implies the mutator changes, and those, at the moment, are hard to catch. I did give it a go at some point, and the code got out of control. I believe a global map/table would be necessary to achieve this. -
lastFetchedAt
: In the current implementation, whenever the re-validate function actually starts a new request, we callDate.now()
right after theawait
statement, updatethis
hook, and broadcast this property to other hooks.
Other names could be fetchedAt
, revalidatedAt
(though the name doesn't imply fetch that happens on mount).
@shuding Alright, then should we wait on your PR first, and then adapt this branch to that?
What happened to this PR? I was thinking of writing some similar functionality myself.
sad that it hasn't been implemented.