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.

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

revalidatePath() does not revalidate layout when followed by redirect() #70483

Closed
tom-sherman opened this issue Sep 25, 2024 · 3 comments · Fixed by #70715
Closed

revalidatePath() does not revalidate layout when followed by redirect() #70483

tom-sherman opened this issue Sep 25, 2024 · 3 comments · Fixed by #70715
Assignees
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@tom-sherman
Copy link

Link to the code that reproduces this issue

https://github.com/tom-sherman/next-redirect-revalidate-bug

To Reproduce

  1. Start the application
  2. Increase the counter to set a value in the cookie
  3. Press reset

Current vs. Expected behavior

Current behaviour:

Layout is not revalidated/refreshed

Expected behaviour:

Layout to be revalidated

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.6.0: Wed Jul 31 20:49:39 PDT 2024; root:xnu-10063.141.1.700.5~1/RELEASE_ARM64_T6000
  Available memory (MB): 16384
  Available CPU cores: 8
Binaries:
  Node: 20.9.0
  npm: 10.5.0
  Yarn: 1.22.19
  pnpm: 9.1.0
Relevant Packages:
  next: 15.0.0-canary.168 // Latest available version is detected (15.0.0-canary.168).
  eslint-config-next: N/A
  react: 19.0.0-rc-5d19e1c8-20240923
  react-dom: 19.0.0-rc-5d19e1c8-20240923
  typescript: 5.3.3
Next.js Config:
  output: N/A

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

Navigation

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

next dev (local), next start (local), Vercel (Deployed)

Additional context

This works on v15 RC but broken on latest canary.

@tom-sherman tom-sherman added the bug Issue was opened via the bug report template. label Sep 25, 2024
@github-actions github-actions bot added the Navigation Related to Next.js linking (e.g., <Link>) and navigation. label Sep 25, 2024
@mateuszaliyev
Copy link
Contributor

I was about to create an issue myself when I found this one. This bug has been a source of many problems for us. It occurs only when calling redirect after revalidatePath or revalidateTag.

@abhi12299
Copy link
Contributor

abhi12299 commented Sep 30, 2024

Commenting out the following code fixes this issue:

if (redirectLocation) {
const newHref = createHrefFromUrl(redirectLocation, false)
// Because the RedirectBoundary will trigger a navigation, we need to seed the prefetch cache
// with the FlightData that we got from the server action for the target page, so that it's
// available when the page is navigated to and doesn't need to be re-fetched.
createSeededPrefetchCacheEntry({
url: redirectLocation,
data: {
flightData,
canonicalUrl: undefined,
couldBeIntercepted: false,
isPrerender: false,
postponed: false,
},
tree: state.tree,
prefetchCache: state.prefetchCache,
nextUrl: state.nextUrl,
kind: isPrerender ? PrefetchKind.FULL : PrefetchKind.AUTO,
})
mutable.prefetchCache = state.prefetchCache
// If the action triggered a redirect, we reject the action promise with a redirect
// so that it's handled by RedirectBoundary as we won't have a valid
// action result to resolve the promise with. This will effectively reset the state of
// the component that called the action as the error boundary will remount the tree.
// The status code doesn't matter here as the action handler will have already sent
// a response with the correct status code.
reject(
getRedirectError(
hasBasePath(newHref) ? removeBasePath(newHref) : newHref,
redirectType || RedirectType.push
)
)
return handleMutable(state, mutable)
}

More specifically, just commenting out the return handleMutable(state, mutable) line works too - this hints at the fact that either the router state tree is not being merged properly or the payload received from the server does not contain the re-rendered root layout.

However, commenting out the above lines results in refetching the RSC payload of the root layout twice instead of just once, which is obviously not an elegant solution.

@ztanner ztanner self-assigned this Oct 2, 2024
@ztanner ztanner added the linear: next Confirmed issue that is tracked by the Next.js team. label Oct 2, 2024
ztanner added a commit that referenced this issue Oct 4, 2024
…tes + redirects (#70715)

If a server action redirects, we reject the promise eagerly with a
redirect error, but that means we skip the handling that clears the
client router cache in case the response was revalidated.

This keeps the existing handling to seed the prefetch cache & reject the
promise but ensures we've had a chance to apply the flight data to the
router state.

Fixes #70483
Closes NDX-357
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 Oct 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants