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

fix: fetch results cached st not getting latest data #218

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

kittybest
Copy link
Contributor

Description
The attestations are cached for long time, which causes user not getting the latest approved attestations.

Related Issues
close #213

Copy link

vercel bot commented Jul 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
maci-rpgf ❌ Failed (Inspect) Jul 31, 2024 6:11pm

Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kittybest thanks, just one comment

const approve = useApproveApplication({});
const approve = useApproveApplication({
onSuccess: () => {
window.location.reload();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a good practice to reload page on action/success/error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but I don't have better way to call fetch immediately 🤔
How about I made the refetchInterval to 10 seconds like now?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lib has something for immediate refetching values, the docs should have this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what I wanna do is to refetch instantly in onSuccess

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but make sure you don't call it in a loop, so if there are no new data, just skip refetching.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0xmad how do you think I changed to something like this now? (but I think the data is not updated right after isSuccess so should call after something like 3 seconds? the isSuccess is too fast which is earlier than the transaction succeeded QQ)

Copy link
Collaborator

@0xmad 0xmad Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0xmad how do you think I changed to something like this now?

Can you show what you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so now I split some functions out the fetchAttestation.ts file, and instead of setting cacheInterval to 0 for uncached fetch, made another file for normal fetch, since the node-fetch-cache package cannot be imported or called in the browser. Once the application is approved, the useEffect hook would call normal fetch and merge the results with the original fetch result.

But if it refetch once it's isSuccess, the attestation is actually not yet updated, so I made it sleep for 5 seconds and refetch.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good as a temporary solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open an issue for it: #229

@kittybest
Copy link
Contributor Author

kittybest commented Jul 24, 2024

Problem
Currently already tried several ways to refetch the latest data, now already made sure it would call refetch right after succeeds, but no latest data loaded. Right now I think reload the current page is the best way to complete this PR, we could find another way to solve this problem in the future, but I think it's important to make sure user get the instant update on data status and prevent them from approving same applications.

or add the approved applications into the list using useState after approve.isSuccess, no matter what actually is returned from the useQuery, just to prevent coordinator approving same application twice; if some of them are failed, they would find it out after they do anything else.

@ctrlc03 ctrlc03 requested a review from 0xmad July 31, 2024 17:11
@kittybest kittybest force-pushed the fix/wait-for-application-approved branch from c4f1642 to 18f8dac Compare July 31, 2024 17:41
Comment on lines 174 to 176
const timeout = setTimeout(() => {
fetchData();
clearTimeout(timeout);
}, 5000);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const timeout = setTimeout(() => {
fetchData();
clearTimeout(timeout);
}, 5000);
const timeout = setTimeout(() => {
fetchData();
}, 5000);
return () => clearTimeout(timeout);

Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kittybest thanks!

@ctrlc03 ctrlc03 merged commit f0cd48d into feat/design-ui Jul 31, 2024
8 of 9 checks passed
@ctrlc03 ctrlc03 deleted the fix/wait-for-application-approved branch July 31, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants