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

Profile server client refresh #420

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

bitbeckers
Copy link
Contributor

@bitbeckers bitbeckers commented Feb 7, 2025

Refactors the data fetching of the hypercert profile tabs to user server side APIs. Also implements a path invalidation flow that clears cached calls when a fraction claim was succesfully executed

@bitbeckers bitbeckers added the bug Something isn't working label Feb 7, 2025
@bitbeckers bitbeckers requested a review from Jipperism February 7, 2025 15:43
@bitbeckers bitbeckers self-assigned this Feb 7, 2025
Copy link

vercel bot commented Feb 7, 2025

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

Name Status Preview Comments Updated (UTC)
hypercerts-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 8:30pm

This was referenced Feb 7, 2025
Base automatically changed from feat/batch-claim-fraction to dev February 7, 2025 18:10
@bitbeckers bitbeckers changed the title WIP: profile server client refresh Profile server client refresh Feb 10, 2025
@bitbeckers bitbeckers marked this pull request as ready for review February 10, 2025 09:28
Copy link
Contributor

@Jipperism Jipperism left a comment

Choose a reason for hiding this comment

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

I'm struggling a bit to come to grips with how all of this works. I understand we're polling from the client side now, but I'm not sure how the api routes get invalidated as they will be caching results as well. Calling revalidateServerPath("profile/[address]") will invalidate the cached data for all server components. I take it that it will invalidate all api's under that "profile/address" path as well? Because that data is now not server-component bound anymore but being fetched using react-query.

Another question: how does this data get invalidated if the user closes their window before the transaction completes? Are we then stuck with stale data until the next invalidation due to interaction (or vercel deploy)?

`/profile/${account.address}?tab=hypercerts-claimable`,
`/profile/${account.address}?tab=hypercerts-owned`,
]);
await refreshData(getAddress(account.address!));
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like we are not invalidating /hypercerts/${selectedHypercert?.hypercert_id} anymore now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I read the correctly, we should precisely specify all API call paths: https://nextjs.org/docs/14/app/api-reference/functions/revalidatePath

correct?

Copy link
Contributor

@Jipperism Jipperism Feb 10, 2025

Choose a reason for hiding this comment

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

Depends on what you mean by API call path. As I understand it, the paths you specify here are supposed to be frontend paths, and not api paths. The server components that use fetch to get data before being rendered that are in those frontend paths will then have their cache purged, which results in them refetching data when refreshed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the invalidation methods to match the profile, api and hypcert view pages according to the doc. Again, a bit pending on this one to be completed to validate: hypercerts-org/hypercerts-api#254

Copy link
Contributor

@Jipperism Jipperism Feb 19, 2025

Choose a reason for hiding this comment

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

@bitbeckers I would like to test this on a preview branch before resolving but there are currently issues with it being deployed

Copy link
Contributor

Choose a reason for hiding this comment

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

@bitbeckers still some deployment errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's live @Jipperism

Copy link
Contributor

Choose a reason for hiding this comment

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

It's very frustrating but it doesn't seem like the cache is being invalidated, after successfully claiming a fraction it still shows up on the claimable fractions page.

Refactors and optimises the data fetching of the tabs pages to speed up
the loading of the data
typo in filename, console logs, redundant business logic
Replace useQuery implementation in hypercerts-tab-content and -counts
component with hooks for reusability
Removes unused hooks and services from the hypercerts folder
Updates the refreshData methods in the unclaimed hypercerts claim and
batch claim button to:

- invalidate the user's profile page
- invalidate the relevant API paths
- all hypercert view pages for the claimed fractions hypercert IDs
- Refactors the refactor into server side data fetching, aiming for as
much SSR as possible
- Moves actions for hypercerts and allowlist into `actions` folder to
make their purpose explicit
- Removes the suspense boundary on the hypercerts-tab-content component
and puts it on the highest level possible - the UnclaimedFractions table
- Adds allowlist action for fetching unclaimed fractions with metadata
attached
- removes the delete action from the metadata fetcher. The requestmap is
ephemeral so we let is run with the lifecycle of the method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

2 participants