search

code-hike / bright

React Server Component for syntax highlighting
https://bright.codehike.org
1.45k stars 20 forks source link

Consider caching fetch calls #21

Closed alvarlagerlof closed 1 year ago

alvarlagerlof commented 1 year ago

image

I noticed these fetches happening in my Vercel dashboard and they seem to be mostly uncached. I wouldn't expect those files to change often. Is there a way that you / I can cache them?

pomber commented 1 year ago

just to check, you are using Bright on a dynamic page right?

alvarlagerlof commented 1 year ago

It's not explicitly dynamic. It just has revalidate set.

tom-sherman commented 1 year ago

@alvarlagerlof if you change from a revalidate route config to instead setting revalidate on individual fetches then this problem should go away I think - those grammars and themes will be cached forever.

alvarlagerlof commented 1 year ago

@alvarlagerlof if you change from a revalidate route config to instead setting revalidate on individual fetches then this problem should go away I think - those grammars and themes will be cached forever.

I don't trigger this fetch from my code. It's started by bright I think just by rendering the component on the server.

tom-sherman commented 1 year ago

Yes, but the default for Next.js will be to cache the fetch forever - if you have a revalidate route config then the grammars/themes will be refetched every time instead. You're telling Next.js to refetch in this instance.

This is why the Next.js docs recommend revalidating only the fetches you need instead of using the route segment config.

alvarlagerlof commented 1 year ago

Yes, but the default for Next.js will be to cache the fetch forever - if you have a revalidate route config then the grammars/themes will be refetched every time instead. You're telling Next.js to refetch in this instance.

This is why the Next.js docs recommend revalidating only the fetches you need instead of using the route segment config.

Ah, my bad. I did misunderstand you. But now that I get your point, I remembered why it put it there. The main thing I want to have revalidating is a data fetch to sanity, using their JS package. I don't believe we have a way to control fetches like that yet right?

tom-sherman commented 1 year ago

Nope, not unless sanity allows you to pass the fetch options somehow (or swap out the fetch function entirely). It's the same issue as I just raised in lighter that's linked above.

alvarlagerlof commented 1 year ago

Nope, not unless sanity allows you to pass the fetch options somehow (or swap out the fetch function entirely). It's the same issue as I just raised in lighter that's linked above.

Ah damn. I see. Your issue makes more sense (and on the right package, so I'll close this one).