1ifeworld / river

Set information free.
https://river.ph
GNU Affero General Public License v3.0
25 stars 2 forks source link

Cache GraphQL Queries #508

Closed n1ckoates closed 7 months ago

n1ckoates commented 7 months ago

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.

vercel[bot] commented 7 months ago

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

A member of the Team first needs to authorize it.

vercel[bot] commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

@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 (https://github.com/vercel/next.js/issues/49438 and https://github.com/jasonkuhrt/graphql-request/issues/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 commented 7 months ago

@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 commented 7 months ago

Nick, I added your email nick@nickoates.com 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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

Reverted those changes just now. Sorry for the delay!

salieflewis commented 7 months ago

Reverted those changes just now. Sorry for the delay!

No worries! Approving the deployments now.