feat: support 'use cache' #2862
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📊 Package size report 2%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
3 times, most recently
from
de692a4 to
3b2a0f2
Compare
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
from
7fa0925 to
81fd1b4
Compare
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
2 times, most recently
from
4cd7931 to
d51d811
Compare
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
3 times, most recently
from
7888b9a to
04d4a9b
Compare
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
2 times, most recently
from
6b28b3b to
a929442
Compare
pieh
added
the
test all versions
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
2 times, most recently
from
0e477cf to
ecab0b9
Compare
pieh
commented
Apr 28, 2025
package.json
Outdated
| @@ -77,6 +77,7 @@ | |||
| "msw": "^2.0.7", | |||
| "netlify-cli": "^20.1.1", | |||
| "next": "^15.0.0-canary.28", | |||
| "next-next": "npm:next@^15.3.1-canary.7", | |||
There was a problem hiding this comment.
This is to get types for new use cache handler. I didn't want to touch next version we already use to not increase scope of this PR, as there is a lot of next types issues to solve in general.
Maybe this alias should be named better to indicate it's for use cache specifically (at least as of now)
There was a problem hiding this comment.
This alias is now renamed
opennextjs-netlify/package.json
Line 80 in 8a23cea
pieh
commented
Apr 28, 2025
src/build/functions/server.ts
Outdated
Comment on lines
136
to
153
| const writeRunConfig = async (ctx: PluginContext, standaloneNextConfig: NextConfigComplete) => { | ||
| // write our run-config.json to the root dir so that we can easily get the runtime config of the required-server-files.json | ||
| // without the need to know about the monorepo or distDir configuration upfront. | ||
| await writeFile( | ||
| join(ctx.serverHandlerDir, RUN_CONFIG_FILE), | ||
| JSON.stringify({ | ||
| nextConfig: standaloneNextConfig, | ||
| // only enable setting up 'use cache' handler when Next.js supports CacheHandlerV2 as we don't have V1 compatible implementation | ||
| // see https://github.com/vercel/next.js/pull/76687 first released in v15.3.0-canary.13 | ||
| enableUseCacheHandler: ctx.nextVersion | ||
| ? satisfies(ctx.nextVersion, '>=15.3.0-canary.13', { | ||
| includePrerelease: true, | ||
| }) | ||
| : false, | ||
| } satisfies RunConfig), | ||
| 'utf-8', | ||
| ) | ||
| } |
There was a problem hiding this comment.
this is to make run-config.json file to also contain next-runtime toggles and not just Next.js config to allow us to conditionally enable some setups - in this case conditionally setup our use cache handler depending on version because using implementation in this PR that does not implement V1 variant is resulting in crashes due to some methods from V1 interface not being implemented
There was a problem hiding this comment.
This was moved since initially commented on, but leaving comment up for visibility as same change is still in this PR, just in slightly different place (to reduce amount of changes initial commits adding it introduced)
pieh
commented
Apr 28, 2025
Comment on lines
+28
to
+48
| /** | ||
| * Get the most recent revalidation timestamp for a list of tags | ||
| */ | ||
| export async function getMostRecentTagRevalidationTimestamp(tags: string[]) { | ||
| if (tags.length === 0) { | ||
| return 0 | ||
| } | ||
|
|
||
| const cacheStore = getMemoizedKeyValueStoreBackedByRegionalBlobStore({ consistency: 'strong' }) | ||
|
|
||
| const timestampsOrNulls = await Promise.all( | ||
| tags.map((tag) => getTagRevalidatedAt(tag, cacheStore)), | ||
| ) | ||
|
|
||
| const timestamps = timestampsOrNulls.filter((timestamp) => timestamp !== null) | ||
| if (timestamps.length === 0) { | ||
| return 0 | ||
| } | ||
| return Math.max(...timestamps) | ||
| } | ||
|
|
There was a problem hiding this comment.
To support interface of use cache handler we need method that will return most recent tag expiration timestamp .. or 0 if none of tags were ever revalidated (as opposed to just checking if tag is stale that is used in response cache handler)
pieh
commented
Apr 28, 2025
Comment on lines
34
to
35
| isErrored: boolean // pieh: this doesn't seem to be actually used in the default implementation | ||
| errorRetryCount: number // pieh: this doesn't seem to be actually used in the default implementation |
There was a problem hiding this comment.
Those are only ever set, but never actually read, so possibly those should be removed. This interface is copied as-is from Next.js default implementation, so I did want to include this intentionally here for awareness
There was a problem hiding this comment.
This was removed in 8a23cea
pieh
commented
Apr 28, 2025
| // eslint-disable-next-line @typescript-eslint/no-empty-function | ||
| const tmpResolvePendingBeforeCreatingAPromise = () => {} | ||
|
|
||
| export const NetlifyDefaultUseCacheHandler = { |
There was a problem hiding this comment.
This is mostly copy & paste from Next.js default cache handler except for tags handling and added OTEL
There was a problem hiding this comment.
Could we please add a reference comment to what this was modelled after?
There was a problem hiding this comment.
I should clear this up -
opennextjs-netlify/src/run/handlers/use-cache-handler.ts
Lines 19 to 20 in 11c7164
There was a problem hiding this comment.
Hopefully added more clear description in 8a23cea
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
2 times, most recently
from
1679b5e to
e8eb0c1
Compare
mrstork
reviewed
Apr 28, 2025
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-empty-function | ||
| const tmpResolvePendingBeforeCreatingAPromise = () => {} |
There was a problem hiding this comment.
This is our temp variable, or was this also copy pasted?
There was a problem hiding this comment.
it was originally same as in https://github.com/vercel/next.js/blob/0eb78fd629858858a5a31591b3b3de294762f870/packages/next/src/server/lib/cache-handlers/default.ts#L101, just our lintings rule didn't like defining this temporary no-op function in the same place:
and to me, the rule mede sense so I did move it to outer scope
the purpose of tmp function there that we re-assign in the new Promise callback in the first place is to make typescript happy, because otherwise resolvePending is either nullable or we have to force a type, so none of the options are really great and I just thought the way it currently is was the least worst option
mrstork
reviewed
Apr 28, 2025
Comment on lines
+120
to
+119
| if (ttl < 0) { | ||
| // In-memory caches should expire after revalidate time because it is | ||
| // unlikely that a new entry will be able to be used before it is dropped | ||
| // from the cache. |
There was a problem hiding this comment.
Is there any handling around the "stale while revalidate" case?
There was a problem hiding this comment.
with the default cache handler there is not
mrstork
reviewed
Apr 28, 2025
| return undefined | ||
| } | ||
|
|
||
| if (await isAnyTagStale(entry.tags, entry.timestamp)) { |
There was a problem hiding this comment.
Learning - If any tag is stale, then the entry is stale? So if for example _n_t_/layout became stale then every page using that tag would also be stale? (since it's often the top level tag for an entry)
There was a problem hiding this comment.
Yes, that's correct - so if you mark layout as stale - then all pages that use that layout are stale and should be regenerated on next opportunity
mrstork
reviewed
Apr 29, 2025
tests/integration/use-cache.test.ts
Outdated
| useCacheTagPrefix: 'data', | ||
| expectedCachingBehaviorWhenUseCacheRegenerates: { | ||
| // getData function has 'use cache' so it should report same generation time, everything else is dynamically regenerated on each request | ||
| getDataTimeShouldShouldBeEqual: true, |
There was a problem hiding this comment.
Don't believe the double Should is intentional
There was a problem hiding this comment.
fixed typo in e18b9b8 🤞
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
2 times, most recently
from
2bac965 to
359cc10
Compare
…ation in module preamble comment
pieh
force-pushed
the
michalpiechowiak/frb-1405-next15-use-cache-not-persisted
branch
from
359cc10 to
8a23cea
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is work in progress to support upcoming
'use cache'directiveNotes:
The primary need for custom Netlify cache handler implementation (the new one, that is used by
'use cache'directive) is that default implementation is in-memory only. While it seems like at leastdefaulthandler foruse cachedoesn't need entries to be actually persisted - it still need to support tags (seecacheTag) properly and in serverless environment it means that we can't use in-memory only tag manifest like the default implementation does and will need to check with some persisted source of truth.Review notes:
This PR contains also changes from following refactor PRs that it depends on.
To check just "use cache" specific changes without polluting diff with refactors use this compare view
Documentation
Tests
Primarily integration tests are being added for various
use cachescenarios:use cacheis used on (data fetching function, non-page react component, page react component, whole page module)And test check behavior when executing against same serverless function instance, different serverless function instances (default cache is in-memory only, so to test behaviors it forces use of integration tests as you can't ensure hitting same or different serverless function in e2e), checking behaviors after tag invalidation (also depending on wether same serverless instance is used or not and that it should work the same in both cases because tags are shared, while cache entries themselves are not) and finally checking that expiry behavior works.
Tests failing on their own in https://github.com/opennextjs/opennextjs-netlify/actions/runs/14674525410/job/41267496397 (comment about most of tests passing - lot of test cases there do NOT test primary difference between Next.js's default cache handler and Netlify's implementation - only test related to revalidating tags on different lambda that is checked later - this is somewhat intentional because the whole cache handler is implemented here and doesn't reuse Next.js code - this is intentional to make sure any potentially future changes to default cache handler don't break any assumptions - and because we don't actually share code I added test cases just for overall cache handler behavior to make sure it works as expected)
Relevant links (GitHub issues, etc.) or a picture of cute animal
https://linear.app/netlify/issue/FRB-1405/[next15]-use-cache-not-persisted