fix(cacheInterceptor): handle index route cache key#1189
Conversation
🦋 Changeset detectedLatest commit: 8cc70bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
conico974
left a comment
There was a problem hiding this comment.
Could you add some unit test ?
|
|
||
| // Then we decode the path params | ||
| localizedPath = decodePathParams(localizedPath); | ||
| localizedPath = decodePathParams(localizedPath) || "/"; |
There was a problem hiding this comment.
localizedPath is used in the rest of the files in a couple of places (i.e. hasBeenRevalidated, isStale).
Did you check if it has the right value there ?
There was a problem hiding this comment.
Thanks for the review! It seems that I indeed missed the usage in hasBeenRevalidated and isStale, which should use the cacheKey.
I've added some tests to validate the behavior:
/albumsstill uses/albumsfor the cache lookup and 'original-mode' cache checks, so existing non-index behavior is unchanged- The new index cache key test fails without the route normalization, and now passes.
- The same index cache key test failed after my initial fix, because
hasBeenRevalidatedandisStalestill received/- It now verifies that 'original-mode' tag cache checks use
/index
- It now verifies that 'original-mode' tag cache checks use
- The new stale index route path test verifies that background revalidation still uses the public route
/, not the cache key/index
As you can imagine this code is all new to me, so let me know if there is anything I can improve (:
conico974
left a comment
There was a problem hiding this comment.
LGTM, Thanks for the PR
We noticed that our index page was not being served by the cache interceptor even though
enableCacheInterceptionwas enabled.This is similar to #860, but the existing fix still uses
??fallbacks. After trailing slash normalization,/becomes an empty string, solocalizedPath ?? "/"still evaluates to"".This normalizes the route path back to
/for ISR detection, while keeping/indexas the cache lookup key to match the generated cache asset.Verified against a Cloudflare/OpenNext build:
/index.cache/index