Skip to content

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Oct 9, 2025

@vicb vicb requested review from conico974 and james-elicx October 9, 2025 12:54
@vicb vicb requested a review from a team as a code owner October 9, 2025 12:54
@imconnorngl
Copy link

imconnorngl commented Oct 9, 2025

Thanks for this! This is really useful especially the options to pass to Next.js to ensure it is not minified.

One thing that I think might be valuable to the average developer is clarifying when incremental cache vs the tag cache is used (for example when caching with unstable_cache where would my cache actually be saved - in the incremental cache or the tag cache). I think it would probably help developers to understand whether they need a tag cache vs just utilising an incremental cache.

We're currently using KV since we are not too bothered about consistency across replicas for our needs. Are there any pitfalls other than this when using KV?

@vicb
Copy link
Contributor Author

vicb commented Oct 9, 2025

where would my cache actually be saved - in the incremental cache or the tag cache

Good point, we should clarify this.

The incremental cache is the cache store, it contains pages, fetched data, and unstable_cached data.
The tag cache is not really a "cache", it only stores the timestamps when tags have been revalidated via revalidateTag and revalidatePath.

vicb and others added 3 commits October 10, 2025 08:39
@vicb
Copy link
Contributor Author

vicb commented Oct 10, 2025

Thanks @anonrig @khuezy @james-elicx @imconnorngl for the review and fixes!

Copy link
Contributor

@conico974 conico974 left a comment

Choose a reason for hiding this comment

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

LGTM, just a few nits/question and probably things to address in a different PR


The Incremental cache is the store containing all the cached data (i.e. pages, `fetch`, `unstable_cache`).

You should use the Workers Static Assets based cache if your site is SSG. It is the fastest available option. Note that as Workers Static Assets are read-only, this option can not be used with revalidation.
Copy link
Contributor

Choose a reason for hiding this comment

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

We could add a note about the data cache here, you could use static assets if you only have SSG route (that you don't revalidate) and SSR route with absolutely no data cache (or one that don't change after build).
If they use that they should also not use the tag cache


It should be configured for App-Router based app using `revalidateTag` or `revalidatePath`.

The D1 based tag cache should only be used if your site receives low traffic. The Durable Object based tag cache is the recommend option for most sites. See [the reference guide](/cloudflare/caching#tag-cache-for-on-demand-revalidation) for the available configuration options.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we say low traffic ? Do we want to add some numbers here ? I feel like a lot of people just testing OpenNext should use D1
And I think that a lot of people actually have low enough traffic for D1, D1 is still pretty capable

It should be configured for App-Router based app using `revalidateTag` or `revalidatePath`.

The D1 based tag cache should only be used if your site receives low traffic. The Durable Object based tag cache is the recommend option for most sites. See [the reference guide](/cloudflare/caching#tag-cache-for-on-demand-revalidation) for the available configuration options.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should definitely add some notes here about the DO tag cache around the sizing of it, a lot of people issues with that is probably about bad sizing actually. Maybe in another PR, because it is a big subject

@vicb
Copy link
Contributor Author

vicb commented Oct 10, 2025

Thanks for the review @conico974
I have created #186 to follow up on your comments

@vicb vicb merged commit f728d23 into main Oct 10, 2025
1 check passed
@vicb vicb deleted the vicb/perf branch October 10, 2025 07:34
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.

6 participants