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

[🐛 Bug]: revalidateTag not working as expected #451

Closed
1 task done
marbiano opened this issue Sep 7, 2023 · 9 comments
Closed
1 task done

[🐛 Bug]: revalidateTag not working as expected #451

marbiano opened this issue Sep 7, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@marbiano
Copy link

marbiano commented Sep 7, 2023

next-on-pages environment related information

System:
Platform: darwin
Arch: x64
Version: Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:22 PDT 2023; root:xnu-8796.121.3~7/RELEASE_X86_64
CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Memory: 16 GB
Shell: /bin/zsh
Binaries:
Node: 18.14.0
Bun: N/A
pnpm: N/A
Yarn: 1.22.19
npm: 9.3.1
Package Manager Used: npm
Relevant Packages:
@cloudflare/next-on-pages: 1.6.0
vercel: N/A
next: 13.4.19

Description

Based on this comment and this PR, it looks like support for revalidateTag was introduced but when I'm trying to use it on a demo project, it's simply not working. The POST request is being triggered as expected by Next.js but there seems to be no results. However, revalidatePath does seem to work as expected.

Here's an URL where you can test this out, the first two buttons use revalidateTag under the hood only to revalidate the current page, and the third button just revalidates everything that is under the /time/ URL.

I'm looking to know if 1.6.0 only introduced support for revalidatePath but not for revalidateTag, or if it's a bug and revalidateTag should also work. Unfortunately, for my use case I need granular revalidation and that can only be achieved with revalidateTag currently on Next.js 13.

Reproduction

No response

Pages Deployment Method

Pages CI (GitHub/GitLab integration)

Pages Deployment ID

ab81c1b6-3b35-4055-8d61-3884b69e8219

Additional Information

  • Using a custom domain as the Cache API only works with those, in case you might wanna think it's because of that.
  • It's all working as expected in local with npm run dev so it's definitely something CF related, not a misuse of Next.js.

Would you like to help?

  • Would you like to help fixing this bug?
@marbiano marbiano added the bug Something isn't working label Sep 7, 2023
@james-elicx
Copy link
Contributor

Hello, thank you for raising this issue. Unfortunately, I'm not having much luck trying to reproduce it - would it be at all possible for you to share a reproduction, or further steps that can be used to try and reproduce this issue?

You can see a test app that I've deployed using Next.js v13.4.19 at https://next-cache-demo.eli.cx/.

I'm looking to know if 1.6.0 only introduced support for revalidatePath but not for revalidateTag, or if it's a bug and revalidateTag should also work.

revalidatePath uses revalidateTag under the hood in Next.js, so if one works, the other should work fine.

It's all working as expected in local with npm run dev so it's definitely something CF related, not a misuse of Next.js.

I would also note that this isn't necessarily always the case - I've noticed inconsistencies/bugs between next dev and builds when working on features/fixes for next-on-pages. I would recommend using next build && next start for the best comparison, just to be on the safe side. 🙂

@marbiano
Copy link
Author

marbiano commented Sep 7, 2023

@james-elicx thank you for responding. Just to confirm that I get it right, are you saying that if you go to this URL and hit the Refresh tag button time changes for you? That's using revalidateTag with a proper key under the hood and it's what it's not working for me.

Here's the Next.js page that handles that route and here's the server action that is calling revalidateTag with a particular tag that is unique per route.

You can see a test app that I've deployed using Next.js v13.4.19 at https://next-cache-demo.eli.cx/.

Do you happen to be able to share the source code for that example so that I can compare?

@marbiano
Copy link
Author

marbiano commented Sep 7, 2023

I would recommend using next build && next start for the best comparison, just to be on the safe side. 🙂

Just tried it that way in local, revalidateTag works just fine. It's only on CF deploy that it seems to not be doing anything.

@james-elicx
Copy link
Contributor

james-elicx commented Sep 7, 2023

@james-elicx thank you for responding. Just so that I confirm that I get it right, are you saying that if you go to this URL and hit the Refresh tag button time changes for you? That's using revalidateTag with a proper key under the hood and it's what it's not working for me.

No, sorry for the confusion, I meant that I wasn't having any luck trying to reproduce it in another project. It does happen when I visit your site.

Here's the Next.js page that handles that route and here's the server action that is calling revalidateTag with a particular tag that is unique per route.

Ah, I see now. Thank you for the link to the repro.

I believe this is because URLSearchParams appears to encode URI parameters, so when it reaches Next.js, the param has %2F instead of / when requesting a tag revalidation.

image

I noticed this in one of my projects recently too. I haven't looked into it enough to see if there is a way to work around this in next-on-pages without breaking the intentional use of encoded strings, but you could use decodeURIComponent(...) on your params in your page as a workaround perhaps.

You can see a test app that I've deployed using Next.js v13.4.19 at next-cache-demo.eli.cx.

Do you happen to be able to share the source code for that example so that I can compare?

https://github.com/james-elicx/next-geolocation-pages

@marbiano
Copy link
Author

marbiano commented Sep 7, 2023

Wow, great find @james-elicx! I added a decode for params and now it's working like charm! :)

Not entirely sure why params.location is somehow different than what I get on my local computer but I think that's unrelated to the main topic of this issue so I'm gonna close it as revalidateTag is working perfectly fine.

Thank you for your help and for sharing that repo!

@marbiano marbiano closed this as completed Sep 7, 2023
@james-elicx
Copy link
Contributor

james-elicx commented Sep 7, 2023

Wow, great find @james-elicx! I added a decode for params and now it's working like charm! :)

Not entirely sure why params.location is somehow different than what I get on my local computer but I think that's unrelated to the main topic of this issue so I'm gonna close it as revalidateTag is working perfectly fine.

Thank you for your help and for sharing that repo!

Glad to hear! Yeah, you can open another issue about the inconsistency there if you'd like, but I have a feeling that it's going to be because workerd is based on web standards, whereas next dev/next start aren't.

@susemeee
Copy link
Contributor

susemeee commented Nov 22, 2023

Hello! I have been debugging revalidateTag inconsistency and found out some breaking changes of upstream.

Internal cache handling behavior of Next.js seems to be changed starting from version 13.5 and above. IncrementalCacheValue no longer provides tags attribute, results in inability of flushing cache with tags. (despite revalidate code works fine)

If I put a log console.log('Updating suspense cache', cacheKey, body) right below at when cache is updated

Here below is the result from Nextjs version 13.4.19, which is the version of @james-elicx 's test project mentioned above.

Updating suspense cache c3c3b153d94af60d330eb4f37c189014c5b6e750848b1a5296299f2bee3aba30 {
  kind: 'FETCH',
  data: {
    headers: {
      ...
    },
    body: ...
    status: 200,
    tags: [ 'announcements', '/announcement/page' ],
    url: ...
  },
  revalidate: 31536000
}

Result from Nextjs version 13.5.4:

Updating suspense cache 3cfe9129900a127998b22d2968385374d1c5bc43da93f5b9bad367c9b7a7594d {
  kind: 'FETCH',
  data: {
    headers: {
      ...
    },
    body: ...
    status: 200,
    url: ...
  },
  revalidate: 31536000
}

There is no longer tags property exists;

If I clone and build the test project locally with Next 13.5 or 14+, and simulate environment using Wrangler, I can reproduce revalidateTag inconsistency. 'Revalidate Tags' button does not properly work with newer version of Next.js. Version equal to or lower than 13.4 works as expected.

@james-elicx
Copy link
Contributor

Hello! I have been debugging revalidateTag inconsistency and found out some breaking changes of upstream.

Internal cache handling behavior of Next.js seems to be changed starting from version 13.5 and above. IncrementalCacheValue no longer provides tags attribute, results in inability of flushing cache with tags. (despite revalidate code works fine)

looks like the type signature changed in next.js :(

https://github.com/vercel/next.js/blob/canary/packages/next/src/server/response-cache/types.ts

would you be able to open a new issue for this @susemeee? i recall on your other issue you ticked the box that you'd be interested in helping fix the bug. would you like to give this issue a go? if not, just lmk and i'll be happy to look into a fix

@susemeee
Copy link
Contributor

Sure! Let's talk on in #556

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants