Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Preflight cache middleware opt-out not working for rewrite of dynamic routes #66881

Closed
ambrauer opened this issue Jun 14, 2024 · 1 comment · Fixed by #67734
Closed

Preflight cache middleware opt-out not working for rewrite of dynamic routes #66881

ambrauer opened this issue Jun 14, 2024 · 1 comment · Fixed by #67734
Labels
bug Issue was opened via the bug report template. locked Middleware Related to Next.js Middleware. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Pages Router Related to Pages Router.

Comments

@ambrauer
Copy link

Link to the code that reproduces this issue

https://github.com/ambrauer/next-prefetch-cache-issue

To Reproduce

  1. Run yarn install, yarn build, then yarn start
  2. Open the home page and click on the "Dynamic" nav link (/1)

Current vs. Expected behavior

Current behavior
"Page 1" content is loaded on navigation.

Expected behavior
"Page 2" content is loaded on navigation.

If you reload the page, you'll see "Page 2" loaded properly.

Additional details
PR #32767 introduced a way for middleware to opt-out of next/link prefetch caching by setting the x-middleware-cache header to no-cache. However, this doesn't seem to be working when using NextResponse.rewrite with dynamic routes (in this example, [id].tsx with ids 1 and 2). The middleware example simulates a change in behavior between prefetch and navigation by rewriting to different paths based on the purpose header === prefetch.

Compare this behavior with static routes (in this example, a.tsx and b.tsx) by clicking on the "Static" nav link (/a). You'll see the "Page B" content is loaded on navigation as expected.

Also compare this behavior with use of redirect instead of rewrite. If you change this line in the middleware from

const response = NextResponse.rewrite(new URL(rewriteUrl, request.url));

to

const response = NextResponse.redirect(new URL(rewriteUrl, request.url));

and try the same "Dynamic" nav link, you'll see "Page 2" content is loaded on navigation as expected.

Provide environment information

Operating System:
  Platform: win32
  Arch: x64
  Version: Windows 11 Pro
  Available memory (MB): 32432
  Available CPU cores: 16
Binaries:
  Node: 20.11.1
  npm: N/A
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 15.0.0-canary.29 // Latest available version is detected (15.0.0-canary.29).
  eslint-config-next: N/A
  react: 19.0.0-beta-04b058868c-20240508
  react-dom: 19.0.0-beta-04b058868c-20240508
  typescript: 5.1.3
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Middleware, Navigation, Pages Router

Which stage(s) are affected? (Select all that apply)

next start (local)

Additional context

Note this is Pages router; I have not verified if the issue exists for App router.
See also related discussion here: #43675
@ambrauer ambrauer added the bug Issue was opened via the bug report template. label Jun 14, 2024
@github-actions github-actions bot added Middleware Related to Next.js Middleware. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Pages Router Related to Pages Router. labels Jun 14, 2024
@ztanner ztanner mentioned this issue Jul 13, 2024
Merged
@github-actions GitHub Actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2024
lubieowoce pushed a commit that referenced this issue Aug 22, 2024
@ztanner @lubieowoce
pages router: ensure x-middleware-cache is respected (#67734)
8922d23 
`x-middleware-cache: "no-store"` in Pages router is a way to signal to
the client that it should not store the response in the cache. However
in certain circumstances, namely when `unstable_skipClientCache` is
true, the data request would be awaited and then stored in the
`inflightCache` regardless of the header.

The original implementation of this in the router has logic to delete
the response from the inflight cache after the request has fulfilled
because `inflightCache` stores the unresolved promise. But in this
optimistic prefetch case, when we're only storing it in the cache once
the request is fulfilled, we can prevent a race condition where the
ignored prefetch is erroneously re-added to the cache by ensuring it's
never added to the cache to begin with if the response says not to.

Fixes #66881
Closes NEXT-3550
lubieowoce pushed a commit that referenced this issue Aug 22, 2024
@ztanner @lubieowoce
pages router: ensure x-middleware-cache is respected (#67734)
bd84edb 
`x-middleware-cache: "no-store"` in Pages router is a way to signal to
the client that it should not store the response in the cache. However
in certain circumstances, namely when `unstable_skipClientCache` is
true, the data request would be awaited and then stored in the
`inflightCache` regardless of the header.

The original implementation of this in the router has logic to delete
the response from the inflight cache after the request has fulfilled
because `inflightCache` stores the unresolved promise. But in this
optimistic prefetch case, when we're only storing it in the cache once
the request is fulfilled, we can prevent a race condition where the
ignored prefetch is erroneously re-added to the cache by ensuring it's
never added to the cache to begin with if the response says not to.

Fixes #66881
Closes NEXT-3550
lubieowoce pushed a commit that referenced this issue Aug 22, 2024
@ztanner @lubieowoce
pages router: ensure x-middleware-cache is respected (#67734)
a320cf4 
`x-middleware-cache: "no-store"` in Pages router is a way to signal to
the client that it should not store the response in the cache. However
in certain circumstances, namely when `unstable_skipClientCache` is
true, the data request would be awaited and then stored in the
`inflightCache` regardless of the header.

The original implementation of this in the router has logic to delete
the response from the inflight cache after the request has fulfilled
because `inflightCache` stores the unresolved promise. But in this
optimistic prefetch case, when we're only storing it in the cache once
the request is fulfilled, we can prevent a race condition where the
ignored prefetch is erroneously re-added to the cache by ensuring it's
never added to the cache to begin with if the response says not to.

Fixes #66881
Closes NEXT-3550
lubieowoce pushed a commit that referenced this issue Aug 22, 2024
@ztanner @lubieowoce
pages router: ensure x-middleware-cache is respected (#67734)
f7c2452 
`x-middleware-cache: "no-store"` in Pages router is a way to signal to
the client that it should not store the response in the cache. However
in certain circumstances, namely when `unstable_skipClientCache` is
true, the data request would be awaited and then stored in the
`inflightCache` regardless of the header.

The original implementation of this in the router has logic to delete
the response from the inflight cache after the request has fulfilled
because `inflightCache` stores the unresolved promise. But in this
optimistic prefetch case, when we're only storing it in the cache once
the request is fulfilled, we can prevent a race condition where the
ignored prefetch is erroneously re-added to the cache by ensuring it's
never added to the cache to begin with if the response says not to.

Fixes #66881
Closes NEXT-3550
lubieowoce pushed a commit that referenced this issue Aug 23, 2024
@ztanner @lubieowoce
pages router: ensure x-middleware-cache is respected (#67734)
beb7f7c 
`x-middleware-cache: "no-store"` in Pages router is a way to signal to
the client that it should not store the response in the cache. However
in certain circumstances, namely when `unstable_skipClientCache` is
true, the data request would be awaited and then stored in the
`inflightCache` regardless of the header.

The original implementation of this in the router has logic to delete
the response from the inflight cache after the request has fulfilled
because `inflightCache` stores the unresolved promise. But in this
optimistic prefetch case, when we're only storing it in the cache once
the request is fulfilled, we can prevent a race condition where the
ignored prefetch is erroneously re-added to the cache by ensuring it's
never added to the cache to begin with if the response says not to.

Fixes #66881
Closes NEXT-3550
lubieowoce pushed a commit that referenced this issue Aug 23, 2024
@ztanner @lubieowoce
pages router: ensure x-middleware-cache is respected (#67734)
a73a501 
`x-middleware-cache: "no-store"` in Pages router is a way to signal to
the client that it should not store the response in the cache. However
in certain circumstances, namely when `unstable_skipClientCache` is
true, the data request would be awaited and then stored in the
`inflightCache` regardless of the header.

The original implementation of this in the router has logic to delete
the response from the inflight cache after the request has fulfilled
because `inflightCache` stores the unresolved promise. But in this
optimistic prefetch case, when we're only storing it in the cache once
the request is fulfilled, we can prevent a race condition where the
ignored prefetch is erroneously re-added to the cache by ensuring it's
never added to the cache to begin with if the response says not to.

Fixes #66881
Closes NEXT-3550
ztanner added a commit that referenced this issue Aug 26, 2024
@ztanner
pages router: ensure x-middleware-cache is respected (#67734)
325dc4b 
`x-middleware-cache: "no-store"` in Pages router is a way to signal to
the client that it should not store the response in the cache. However
in certain circumstances, namely when `unstable_skipClientCache` is
true, the data request would be awaited and then stored in the
`inflightCache` regardless of the header.

The original implementation of this in the router has logic to delete
the response from the inflight cache after the request has fulfilled
because `inflightCache` stores the unresolved promise. But in this
optimistic prefetch case, when we're only storing it in the cache once
the request is fulfilled, we can prevent a race condition where the
ignored prefetch is erroneously re-added to the cache by ensuring it's
never added to the cache to begin with if the response says not to.

Fixes #66881
Closes NEXT-3550
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Issue was opened via the bug report template. locked Middleware Related to Next.js Middleware. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Pages Router Related to Pages Router.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

pages router: ensure x-middleware-cache is respected vercel/next.js
1 participant
@ambrauer