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

Cache GraphQL Queries #508

Merged
merged 5 commits into from
Mar 4, 2024
Merged

Cache GraphQL Queries #508

merged 5 commits into from
Mar 4, 2024

Conversation

n1ckoates
Copy link
Contributor

This pull request wraps each GraphQL query with Next.js' (unstable_cache)[https://nextjs.org/docs/app/api-reference/functions/unstable_cache] API, which caches the responses of those functions. This should significantly speed up queries (and thus the frontend interface).

I also configured graphql-request to use the patched version of fetch which itself includes caching logic.

Copy link

vercel bot commented Feb 23, 2024

@n1ckoates is attempting to deploy a commit to the Lifeworld Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

vercel bot commented Feb 23, 2024

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

Name Status Preview Comments Updated (UTC)
river-site ✅ Ready (Inspect) Visit Preview Mar 4, 2024 6:31pm
river-site-metadata ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 4, 2024 6:31pm

@salieflewis
Copy link
Contributor

Hi @n1ckoates thank you for introducing these changes! Locally and on the preview deploy I'm encountering an issue that stems from the adjustments made to getUserId. I'm unsure if this is has to do with the instability of the unstable_cache API or because the data that is required to call getUserId is not available on the server. If you could please make those changes I would be happy to merge this in!

The other thing I wanted to ask if there was any documentation you could please point me to surrounding the change made to graphql-request. I am unfamiliar with this parameter and the library's documentation is sparse. A larger goal of mine was to find a library that allow us to use native fetch for all of our gql queries as to not have to balance the behavior of multiple caching mechanisms in our head. Does this accomplish something similar?

@n1ckoates
Copy link
Contributor Author

@salieflewis I didn't realize getUserId couldn't be called from the server - unstable_cache shouldn't be used there then, and I'll remove it. Thanks for catching that!

The fetch option in graphql-request lets you change the function it uses to perform requests. In a Next.js project, the global fetch function includes additional caching mechanisms, which is what I changed it to. As far as I can tell, this isn't documented anywhere, but I found a few issues (vercel/next.js#49438 and graffle-js/graffle#579) which showed that functionality. graphql-request's README.md also points to documentation on this, but the docs don't seem to exist anymore.

I wasn't able to setup a local dev environment, so I was mostly just relying on the Vercel preview. I couldn't find instructions on how to do that here (README.md points to a removed CONTRIBUTING.md guide) - how should I go about that?

@salieflewis
Copy link
Contributor

@salieflewis I didn't realize getUserId couldn't be called from the server - unstable_cache shouldn't be used there then, and I'll remove it. Thanks for catching that!

The fetch option in graphql-request lets you change the function it uses to perform requests. In a Next.js project, the global fetch function includes additional caching mechanisms, which is what I changed it to. As far as I can tell, this isn't documented anywhere, but I found a few issues (vercel/next.js#49438 and jasonkuhrt/graphql-request#579) which showed that functionality. graphql-request's README.md also points to documentation on this, but the docs don't seem to exist anymore.

I wasn't able to setup a local dev environment, so I was mostly just relying on the Vercel preview. I couldn't find instructions on how to do that here (README.md points to a removed CONTRIBUTING.md guide) - how should I go about that?

I've got to update our contribution guidelines, but yeah for the time being working off the preview deploy makes sense. Configuring a local environment without sharing a ton of API keys would still be quite difficult at this time.

And that is interesting about gql request, thanks for sharing.

@salieflewis
Copy link
Contributor

salieflewis commented Feb 26, 2024

Nick, I added your email [email protected] to the invite list of the platform, so you should be able to login at our main URL river.ph and make an account. That way you can then login via the preview deploy and test actions like creating a channel, and adding/removing an item.

The reason I'm saying this is because I'm still experiencing complications surrounding unstable_cache. The same error that was afflicting the getUserId call popped up in the console when I attempted to remove an item from a channel on the preview deploy.

I recognize not having a local dev environment is a large barrier for contributions. We're working on mitigating this. Thanks for your patience with this.

cc: @n1ckoates

@n1ckoates
Copy link
Contributor Author

If it doesn't have any sensitive details in it, could you share the error here?

Also, could any of the variables in apps/site/.env.example be shared, or replaced by me if they aren't sensitive? Not sure which ones are required to match the server. This would let me just run the Next.js app.

@salieflewis
Copy link
Contributor

If it doesn't have any sensitive details in it, could you share the error here?

Also, could any of the variables in apps/site/.env.example be shared, or replaced by me if they aren't sensitive? Not sure which ones are required to match the server. This would let me just run the Next.js app.

Error fetching txn hash: 0x7b31ebd94dce321936863110ea2e933c72d9cb14495f2d92030b8e0aacddff9f Error: Invariant: incrementalCache missing in unstable_cache async e=>{let{hash:t}=e;return{txn:(await S.txnHash({hash:t})).txn}}

Here is the error. It is getting bubble up because of changes to getTxnHash which need not have a cache due to the way it is being used, which is essentially as a polling mechanism.

@salieflewis
Copy link
Contributor

Hey @n1ckoates wanted to follow up with you about this. I think the last remaining thing is reverting the changes to getTxnHash. Appreciate your patience with this.

@n1ckoates
Copy link
Contributor Author

Reverted those changes just now. Sorry for the delay!

@salieflewis
Copy link
Contributor

Reverted those changes just now. Sorry for the delay!

No worries! Approving the deployments now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants