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

Using redirect after revalidateTag(), revalidatePath() or cookies().set() in server actions do not update the layouts or the back cache #52075

Closed
1 task done
Fredkiss3 opened this issue Jul 1, 2023 · 11 comments
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@Fredkiss3
Copy link
Contributor

Fredkiss3 commented Jul 1, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:19 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T8103
    Binaries:
      Node: 18.16.0
      npm: 9.5.1
      Yarn: 1.22.19
      pnpm: 8.6.2
    Relevant Packages:
      next: 13.4.8-canary.14
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.6
    Next.js Config:
      output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true)

Link to the code that reproduces this issue or a replay of the bug

https://github.com/Fredkiss3/next-redirect-cache-bug

To Reproduce

  1. clone and run the server
  2. Go to / and click on either of the buttons, It will redirect you to /target
  3. Now go back using the browser back button, you will see stale data on the home page, you will also see that the data in the layout is also stale
  4. Now go to /target with either the browser forward button or the link in the home page, and click on the link Go home, now you will see the home page with fresh data, but the layout will still show stale data

For comparison :

  1. Now go to /working and and click on either of the buttons, you will see fresh data on the layout and the current page
  2. Click on the link Go to /target, you will also see fresh data
  3. Click on the browser back button, you will see fresh data
Enregistrement.de.l.ecran.2023-07-01.a.14.57.07.mov

Describe the Bug

When using redirect after changing cookies or calling revalidate methods, next do not clear the browser back cache, so if the user navigates back to the previous page, they still see stale data, the layout data is also not refreshed and still shows stale data.

If we dont' use redirect everything works as expected.
This issue can be a problem because redirecting to login is common pattern after logging out a user (clearing the cookie associated with them).

Expected Behavior

The browser back cache should be cleared after a revalidation or modification of cookies, same for the layout stale data.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-1414

@Fredkiss3 Fredkiss3 added the bug Issue was opened via the bug report template. label Jul 1, 2023
@github-actions github-actions bot added the area: app App directory (appDir: true) label Jul 1, 2023
@balazsorban44 balazsorban44 added the linear: next Confirmed issue that is tracked by the Next.js team. label Jul 7, 2023
@schimi-dev
Copy link

schimi-dev commented Jul 28, 2023

The reproduction example provided uses 13.4.8-canary.14 .

I think it is important to note that the behaviour described in this issue and in the reproduction example has changed with 13.4.11-canary.1. With 13.4.11-canary.1 another problem seemed to be introduced again concerning usage of revalidatePath in Server Actions, even without redirect. From that version onwards calling revalidatePath no longer invalidates other pages.

This can be tested with a simple Master - Detail scenario:
Assume on a Master page you have a list of dynamically fetched resources with links to the corresponding Details pages. When you navigate to the Details page of a resource and edit it via a Server Action that calls revalidatePath (even without redirect), once you navigate back via a Link to the Master page, stale data is shown on the Master page.

This was stated to be solved with below pull request in 13.4.5 but reintroduced in 13.4.11-canary.1:
#50848

I thought this might be relevant for the person solving this issue.

@timneutkens
Copy link
Member

@schimi-dev what you're explaining is actually the correct behavior. revalidatePath is for invalidating a specific path and anything below it (e.g. if you revalidatePath('/dashboard') only /dashboard and anything below it like /dashboard/settings would be invalidated.

For the case you're explaining where there is a list and an item, e.g. a blog overview and blog detail page, it's recommended to use revalidateTag with a common tag. E.g.

const postId = 1
revalidateTag('blog-list')
revalidateTag(`blog-post-{postId}`)

Or calling revalidatePath for both paths is fine too:

const postSlug = 'hello-world'
revalidatePath('/blog')
revalidatePath(`/blog/{postSlug}`)

The one thing that is missing currently but we'll likely work on soon is setting tags based on the data in the request. E.g. being able to tag "this request returned these posts" then only revalidateTag('blog-post-1') is needed.

@schimi-dev
Copy link

@timneutkens sorry if I was unclear, i meant calling revalidatePath("/") in the scenario I described. This was correctly invalidating the cache for the Master Page when called on the Detail Page until 13.4.11-canary.0. Starting with 13.4.11-canary.1 this no longer invalidates the cache for the Master page. Should I prepare a reproduction repository if that helps?

The general reproduction process:

  1. Being on /dashboard we navigate to the /dashboard/[id] page of a specific resource.
  2. We edit the resource on the /dashboard/[id] page via a Server Action that calls revalidatePath("/").
  3. We navigate back to the Master page via a Link .

Then the Master Page shows stale data.

In my opinion, from how I understand the docs, the Master Page should not have stale data in that case.
I also checked aspects like route segment config options and so on, but it seems to be related to revalidatePath .

@schimi-dev
Copy link

I created a reproduction repository for this:
https://github.com/schimi-dev/client-side-router-cache-invalidation-bug

Here again the reproduction scenario and my thoughts on this topic:

  1. Being on /dashboard navigate to the /dashboard/[id] page of a specific resource.
  2. Edit the resource on the /dashboard/[id] page via a Server Action that calls revalidatePath("/").
  3. Navigate back to the Master page via a Link .

Result: Then the Master Page shows stale data.
Expected: The Master Page should show the updated data.

In my opinion, from how I understand the docs, the Master Page should not have stale data. Using revalidatePath("/") should invalidate all dynamic segments underneath. Via the route segment config it is ensured that both of the segments in the reproduction example are dynamic segments.

This example worked until v13.4.11-canary.0. Starting with v13.4.11-canary.1 this problem is present. This can be tested when changeing the corresponding next version in this project.

From my (almost entirely blackbox-like) perspective there was a similar problem until v13.4.5 related to prefetching that was solved by: #50848

Version v13.4.11-canary.1 has a pull request that looks to me like a similar area: #52949

Is it possible that this problem was unintentionally (re-)introduced by that pull request?

@Myks92
Copy link

Myks92 commented Jul 28, 2023

I also had such a problem.

It doesn't work with redirect():

await api.put(`/admin/store/catalog/products/${id}/edit`, data)
revalidatePath('/admin/store/catalog/products')
redirect('/admin/store/catalog/products')

It work without redirect():

await api.put(`/admin/store/catalog/products/${id}/edit`, data)
revalidatePath('/admin/store/catalog/products')

@timneutkens
Copy link
Member

@schimi-dev thanks, that's much clearer indeed, yeah that sounds like a bug, we'll have a look!

@Fredkiss3
Copy link
Contributor Author

Hello as of next@13.4.13-canary.8 the bug seems to have been fixed (at least on my end), here is the PR that adress this.

So i am going to close this issue.

Thanks you @timneutkens for your job

@Fredkiss3
Copy link
Contributor Author

@timneutkens

There are still some bugs :

  • redirecting to the same URL causes the page to do a browser refresh, this bug only happens in edge runtime and if the redirect target URL is different than the current one, it does a client side navigation (the correct behavior)

  • previously when calling redirect in a server action it would keep the history, so the user could go back and forward to the previous URL, but now it seems like redirect replaces the history (maybe this is the expected behavior ?)

Enregistrement.de.l.ecran.2023-08-01.a.03.10.42.mov

I updated the source code with the bug reproduction in this branch : https://github.com/Fredkiss3/next-redirect-cache-bug/tree/another-bug

@Fredkiss3 Fredkiss3 reopened this Aug 1, 2023
@pedro757

This comment was marked as spam.

@Fredkiss3
Copy link
Contributor Author

This issue only happens in edge runtime, And it does not depend wether redirect() has been called after revalidatePath/Tag.

I will close this and create a new one as this one is fixed.

@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 Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked
Projects
None yet
Development

No branches or pull requests

6 participants