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

On-demand revalidation shows outdated data when using <Link> #38306

Closed
1 task done
jopesh opened this issue Jul 4, 2022 · 25 comments
Closed
1 task done

On-demand revalidation shows outdated data when using <Link> #38306

jopesh opened this issue Jul 4, 2022 · 25 comments
Labels
bug Issue was opened via the bug report template.

Comments

@jopesh
Copy link

jopesh commented Jul 4, 2022

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 21.5.0: Tue Apr 26 21:08:29 PDT 2022; root:xnu-8020.121.3~4/RELEASE_ARM64_T8101
Binaries:
  Node: 16.15.1
  npm: 8.13.2
  Yarn: 1.22.19
  pnpm: 7.5.0
Relevant packages:
  next: 12.2.1-canary.3
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

Vercel

Describe the Bug

When using on-demand revalidation with Sanity.io GROQ webhooks, I encountered an issue similar to the one described in #35195. The revalidation proceeds without any errors. When visiting the page directly, the content updated as intended. But when visiting the page via the <Link> component, the page shows the stale data. This does not change after time (I've waited 12+ hours so far). It seems to be affecting dynamic routes especially (though I can't be too sure).

Below is a video of the reproduction repo showing the bug. The fresh dummy post data has no prose, whilst the previous version had. The dummy post shows the fresh data on external navigation or reloading the page. The old data shows on client-side navigation with <Link>.

CleanShot.2022-07-04.at.21.13.02.mp4

Expected Behavior

Revalidation of both the cached static page and the data route.

Link to reproduction

https://github.com/jopesh/nextjs-issue-odr

To Reproduce

  • Create a dynamic route with data from Sanity.io based on the slug
  • Fill in your data in Sanity Studio for the desired slug
  • Build the Next.js app
  • Update the content for the desired slug in the Sanity Studio
  • Revalidate the desired slug via on-demand revalidation
@jopesh jopesh added the bug Issue was opened via the bug report template. label Jul 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2022

Please verify your issue reproduces with next@canary. The canary version of Next.js ships daily and includes all features and fixes that have not been released to the stable version yet. Think of canary as a public beta. Some issues may already be fixed in the canary version, so please verify that your issue reproduces by running npm install next@canary. If the issue does not reproduce with the canary version, then it has already been fixed and this issue can be closed.

@github-actions github-actions bot added the please verify canary The issue should be verified against next@canary. It will be closed after 30 days of inactivity label Jul 4, 2022
@jopesh
Copy link
Author

jopesh commented Jul 6, 2022

Verified against next@12.2.1-canary.3

@ijjk
Copy link
Member

ijjk commented Jul 8, 2022

Hi, can you verify the data being sent as props in getStaticProps is different than the existing props?

We may expose an option to warn when this occurs as a revalidation without change isn't needed.

@balazsorban44 balazsorban44 removed the please verify canary The issue should be verified against next@canary. It will be closed after 30 days of inactivity label Jul 8, 2022
@jopesh
Copy link
Author

jopesh commented Jul 8, 2022

All right, I tried by just logging the data in getStaticProps. I repeated the original steps and got the same result as described above in production.

Data at build time:

{
  _createdAt: '2022-07-04T18:48:06Z',
  _id: 'f0386451-352f-476c-a3d9-42614dfececd',
  _rev: 'o1F9UMohcy1KTaXxhtEZjW',
  _type: 'post',
  _updatedAt: '2022-07-08T11:50:45Z',
  body: [
    {
      _key: '9665ccf36e49',
      _type: 'block',
      children: [Array],
      markDefs: [],
      style: 'normal'
    },
    {
      _key: '58010428a71a',
      _type: 'block',
      children: [Array],
      markDefs: [],
      style: 'normal'
    },
    {
      _key: '82635b3e5a64',
      _type: 'block',
      children: [Array],
      markDefs: [],
      style: 'normal'
    },
    {
      _key: 'e385fdfe4fc8',
      _type: 'block',
      children: [Array],
      markDefs: [],
      style: 'normal'
    },
    {
      _key: '6829d75ce89f',
      _type: 'block',
      children: [Array],
      markDefs: [],
      style: 'normal'
    }
  ],
  image: {
    _type: 'image',
    asset: {
      _ref: 'image-a7c0ba547263879831c7d617356d938188090764-1000x500-jpg',
      _type: 'reference'
    }
  },
  slug: { _type: 'slug', current: 'test' },
  title: 'Test'
}

Then I updated the post once more, removing the body text.

Revalidation run data:

{
  _createdAt: '2022-07-04T18:48:06Z',
  _id: 'f0386451-352f-476c-a3d9-42614dfececd',
  _rev: 'IFAPOPWlCM0oi0S7fHBjQT',
  _type: 'post',
  _updatedAt: '2022-07-08T11:54:49Z',
  image: {
    _type: 'image',
    asset: {
      _ref: 'image-a7c0ba547263879831c7d617356d938188090764-1000x500-jpg',
      _type: 'reference'
    }
  },
  slug: { _type: 'slug', current: 'test' },
  title: 'Test'
}

Edit:
Important to add: I could not reproduce this issue on the local environment with next start and a revalidation call via ngrok. The page updates as intended and shows the updated content on both hard reload and client-side navigation.

@jmotes
Copy link

jmotes commented Jul 11, 2022

Strangely, I am having the opposite issue. Thought I'd mention incase it could help diagnose.

Client-side nav works fine, but on a full page load I briefly see the updated content before it reverts back to the original. Oddly enough, I cannot reproduce the problem on preview deployments - only production ones.

My issue seems to be caused by the caching headers on the index.json files requests - so may be related. But, I'll work on a minimal reproduction and submit my own bug report soon.

@ijjk
Copy link
Member

ijjk commented Jul 11, 2022

@jopesh I'm having trouble reproducing this, are you able to reproduce this on a more minimal example without querying sanity e.g. just returning a timestamp in props and seeing the timestamp fail to update after a revalidation?

@denisjankovic
Copy link

Have the same issue here

@jopesh
Copy link
Author

jopesh commented Jul 12, 2022

@ijjk Gotcha, I was able to reproduce the same issue with your idea and put up another repo: https://github.com/jopesh/nextjs-issue-odr-simple (Link: https://nextjs-issue-odr-simple.vercel.app/).

I've added a non-dynamic route (/static.tsx) with its own revalidation API route (/api/revalidate-static) and it turns out that the issue only appears on the dynamic route (/blog/[slug].tsx with API route /api/revalidate-dynamic) as suspected.

After revalidating both example routes manually via their API routes, the following happens in this reproduction (Build time was 10:25:08):

Dynamic route shows build-time data on client-side navigation
CleanShot 2022-07-12 at 12 31 03

Dynamic route shows revalidated data on hard reload
CleanShot 2022-07-12 at 12 31 29

The static route shows the revalidated data on both ways as intended
CleanShot 2022-07-12 at 12 32 33

@mediabeastnz
Copy link

mediabeastnz commented Jul 13, 2022

Same issue on 12.2.2. Is this an issue or are we all misunderstanding how ISR / On demand works?

The issue seems to be that the json for that page is never cleared. So when navigating by <Link> the old stale data is used not the updated data.

You can verify this by checking the pageProps

@ijjk
Copy link
Member

ijjk commented Jul 13, 2022

@jopesh thanks for that reproduction, I've narrowed in on the issue here and will respond here when we have an update!

@mediabeastnz
Copy link

@jopesh i've tested the cms-sanity-example repo and this issue occurs there also just FYI

@Sinhalite
Copy link

This seems to be the same as what I described in previous issue.
It appears to occur under the condition that you specify paths in addition to using dynamic route.
#35195 (comment)

It also seems to be occurring only in Vercel.

At that time, the data and HTML were revalidated separately, so there was no guarantee that the data would be identical.

However, the event that is occurring this time appears to be just the old cache remaining.

@jmotes
Copy link

jmotes commented Jul 14, 2022

Thanks @Sinhalite! That helped me finally create a minimal reproduction for my issue. I didn't realize it was only static/pre-rendered pages that had the problem! I went ahead and opened #38653 incase my issue is not the same bug.

@mediabeastnz
Copy link

Sorry one last thing I've noticed is that this isn't necessarily a "On demand ISR" issue, it also occurs when you simply rely on ISR (no on demand) to rebuild pages in the background.

@BrandonNoad
Copy link

I am using On-Demand ISR on my site. I have a job that runs daily that adds new data, then calls revalidate on any pages that need to be updated. It seemed to be working fine, but at some point today, my site reverted to showing pages that were several days out-of-date.

styfle pushed a commit to vercel/vercel that referenced this issue Jul 15, 2022
This updates our `allowQuery` generating to ignore all query values for build-time prerender paths as these will match before dynamic routes since they are filesystem routes and the query values will not be overridden properly like they are for fallback prerender paths. This also adds testing for both prerender path types with on-demand ISR to ensure the cache is updated as expected regardless of the query.  

Deployment with patch can be seen here https://nextjs-issue-odr-simple-hrjt2dagm-ijjk-testing.vercel.app/

### Related Issues

x-ref: vercel/next.js#38306
x-ref: vercel/next.js#38653
@ijjk
Copy link
Member

ijjk commented Jul 15, 2022

Hi, a related fix for this has now been landed, please re-deploy your project and give it a try!

A new deployment of the reproduction with the fix applied can be seen here https://nextjs-issue-odr-simple-riad3lztu-ijjk-testing.vercel.app/blog/dynamic

@ijjk ijjk closed this as completed Jul 15, 2022
@Sinhalite
Copy link

I think it's working fine!

Thanks guys😀

@alexrabin
Copy link

Is this issue only fixed on Vercel? I'm using Microsoft Azure and I am having this issue

@jmotes
Copy link

jmotes commented Jul 20, 2022

@alexrabin I would say so since v12.2.3 isn't out yet. I think it's in the canary though.

@alexrabin
Copy link

@jmotes this is working for all platforms on latest canary release or just Vercel?

@jmotes
Copy link

jmotes commented Jul 20, 2022

@alexrabin I can't say for sure. @ijjk would be the one to say, but from looking at his PR (vercel/vercel#8158) I'd say it should be working for all platforms if you're on canary.

EDIT: nevermind, I just saw that PR targets the Vercel platform and not Next.js specifically - so I cannot say. I'd suggest opening a new bug report and reproduction since the OP's issue was Vercel specific.

@ijjk
Copy link
Member

ijjk commented Jul 20, 2022

@alexrabin it sounds like you may have a CDN configured in front of your Next.js application causing your Azure deployment not to be able to fetch the updated content. For On-Demand ISR to work properly your CDN either needs to be aware of the feature like on Vercel or disable CDN caching for those paths.

@alexrabin
Copy link

@ijjk we don't have a CDN configured in front of our Next.js application. Are there any other possibilities?

@ijjk
Copy link
Member

ijjk commented Jul 20, 2022

@alexrabin it's hard to say without a deployment/reproduction to investigate since it does sound related to your custom setup.

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. 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 20, 2022
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.
Projects
None yet
Development

No branches or pull requests

9 participants