feat(ensadmin): introduce useIndexingStatusWithSwr hook#1282
Conversation
This proxy hook allows to create a React Query which will always serve cached result, and allow refetching the current result in the background.
🦋 Changeset detectedLatest commit: b5077fc The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
did we try something just like disclosure, i just looked at the docs and asked claude, so let me know if this was short-sighted and doesn't actually accomplish the goal |
tk-o
force-pushed
the
feat/1200-use-swr-query-hook
branch
from
c937655 to
81cb04d
Compare
|
or perhaps something like this? |
|
@shrugs thanks for offering simpler choices.
Also, we can't just set these options: Please consider there's no cached result, and the query returns an error. We want this error to be gone (so we need to allow the query to turn stale). What do you think about those considerations? |
|
your assessment is likely correct, you have way more experience with this problem and code than i do, so go with your gut |
Introduces the `useIndexingStatusWithSwr` hook which replaces `useIndexingStatus` hook. The new hook is capable of storing the last OK Response from Indexing Status API and have it served when either the new one cannot be fetched at the moment, or could be be fetched, but its `responseCode` is `error`.
… cached Indexing Status with response code OK.
tk-o
force-pushed
the
feat/1200-use-swr-query-hook
branch
from
b81510b to
e47b690
Compare
tk-o
changed the title
useSwrQuery hookuseIndexingStatusWithSwr hook
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Hey great to see this enhancement! Shared a few comments with suggestions. Please feel welcome to merge this when ready 👍
| IndexingStatusResponseOk, | ||
| } from "@ensnode/ensnode-sdk"; | ||
|
|
||
| const DEFAULT_REFETCH_INTERVAL = 3 * 1000; |
There was a problem hiding this comment.
Every 3 seconds? Seems quite frequent! Maybe increase to 10 seconds?
|
|
||
| /** | ||
| * A proxy hook for {@link useIndexingStatus} which applies | ||
| * stale-while-revalidate cache for successfully resolved responses. |
There was a problem hiding this comment.
| * stale-while-revalidate cache for successfully resolved responses. | |
| * stale-while-revalidate cache for successful responses. |
| ); | ||
| } | ||
|
|
||
| // resolve response to be cached |
There was a problem hiding this comment.
| // resolve response to be cached | |
| // successful response to be cached |
| staleTime: cachedResult ? Infinity : undefined, | ||
| // cached result can never be removed by garbage collector | ||
| gcTime: cachedResult ? Infinity : undefined, | ||
| placeholderData: keepPreviousData, // Keep showing previous data during refetch (does not work for errors) |
There was a problem hiding this comment.
Can you please expand upon and clarify in more details what it means when you say: "does not work for errors"?
| const memoizedQueryResult = useMemo(() => { | ||
| // If the query result is error | ||
| // and the cachedResult is available | ||
| // override the query result to be success, including cachedResult data |
There was a problem hiding this comment.
| // override the query result to be success, including cachedResult data | |
| // override the query result to be success, replacing the unsuccessful result with the most recent successful cachedResult |
Is that fair?
Edit: it might also be helpful to rename cachedResult to cachedSuccessfulResult to be more explicit and clear what we're doing here.
Simplify and document the useSwrQuery hook and its consumers.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Great updates. Thank you! Just 2 small questions. Please take the lead to merge when ready 🚀 😄
|
|
||
| if (indexingStatusQuery.status === "error") { | ||
| if (indexingStatusQuery.isError) { | ||
| return <p>Failed to fetch Indexing Status.</p>; |
There was a problem hiding this comment.
It seems we should move the the following logic here that this PR is removing a few lines down in this file?
if (indexingStatus.responseCode === IndexingStatusResponseCodes.Error) {
return (
<IndexingStatsShell>
<IndexingStatsForUnavailableSnapshot />
</IndexingStatsShell>
);
}
Of course, I'm not referring to the logic for IndexingStatusResponseCodes.Error. I'm talking about the HTML that's generated for the case of an error.
| case OmnichainIndexingStatusIds.Completed: | ||
| return indexingStatusResponseOkOmnichain[selectedVariant]; | ||
| return indexingStatusResponseOkOmnichain[selectedVariant] as IndexingStatusResponseOk; | ||
| case "Response Error": |
There was a problem hiding this comment.
Suggest we clarify what is different about these 2 mock cases.
- Response Error: Does this case represent receiving a response successfully but it has a non-ok responseCode?
- Loading Error: Does this case represent having an error receiving a response at all?
Assuming my interpretations above are correct, suggest renaming "Response Error" to "Error ResponseCode"
There was a problem hiding this comment.
Nice suggestion, thank you. It's exactly as you described 👍
update indexing status mocks and the UI used when there is no indexing status available to present
3 participants
Live preview: https://adminensnode-p5chjdg6o-namehash.vercel.app/registrar-actions?connection=https%3A%2F%2Fapi.alpha-sepolia.green.ensnode.io%2F
This PR replaces
useIndexingStatushook in ENSAdmin with a proxy hook calleduseIndexingStatusWithSwr. The API of both hooks is exactly the same, but behaviour differs.useIndexingStatususeIndexingStatusWithSwrIndexingStatusResponseOkorIndexingStatusResponseError.IndexingStatusResponseOk.The
useIndexingStatusWithSwrhook is based on the newuseQuerySwrproxy hook which allows to create a React Query that will always serve previously cached result, and allow refetching the current result in the background.This hook will suport goals of #1200.