-
Notifications
You must be signed in to change notification settings - Fork 120
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
Quirks with revalidation on new and unpublished posts #97
Comments
Hi and thanks again for another wonderful report TL;DRThere's a known race condition due to a mismatch in how GROQ webhooks are intended to be used, and the constraints of Vercel's on-demand incremental static regeneration. nextjs-blog-cms-sanity-v3/pages/api/revalidate.ts Lines 35 to 38 in 419ada2
We've made it enabled by default in the latest version of next-sanity , and this starter will be updated to that version soon.
Deep dive into what's happening, why, and how we'll long-term fix itThese quirks are the result of a race condition where the GROQ webhook is dispatched before the indexes have reached eventual consistency. We designed our webhooks to have the lowest possible latency. It's functionally similar to the visibility=async option in our client libs. So it waits until there's an index available that can resolve possible GROQ queries defined in the That's why the subsequent queries in This is the result of a fundamental mismatch in the constraints GROQ webhooks are designed to solve really well and the architecture of Vercel's on-demand incremental static regeneration. That's why, while GROQ Webhooks are the best solution for implementing on-demand ISR that we have to offer, it's not a good solution. It might be tempting to just slap a "visibility" option on GROQ webhooks that lets you opt-in to only fire the webhook after eventual consistency is reached. But that would be like trying to improve a hammer's 🔨 ability to drive in screws 🔩. What we really want is a good screwdriver 🪛, right? While we're working hard to build that we'll continue to keep Hope this helps, and if anything is unclear I'm happy to elaborate 😄 |
Hi @stipsan, thanks so much for the detailed answer and short-term fix! I'll be checking those out shortly. How curious. When trying to troubleshoot, I had a sneaking suspicion that I might be hitting Sanity or Vercel too fast for the update to have propagated or been 'ready' perhaps. Somewhat relieved to hear I wasn't totally off the mark in this line of thought. Glad to hear that it's being looked into by Sanity along with Vercel, I think ISR is a wonderful addition to both services. Combined with NextJS's preview abilities I've found it wonderful, clients love getting a full fidelity preview and having the change go live immediately. Also spotted that you sorted out deleted/unpublished posts in #94, amazing! Thanks again for your quick and detailed replies, along with the excellent solutions. |
Hello, me again from #93. Thanks @stipsan for sorting that so quickly! I thought I'd make a new issue as this seems slightly different.
Unforuntately I've found a couple of other quirks with revalidation 😬
New posts:
When adding a new post, sometimes, some pages do not get revalidated. The webhook appears to send the right requests on all attempts, but some don't seem to update.
In the video below:
A similar thing (not in the video) happens when I update an authors image. The revalidation routes sent are correct, but only some actaully update (some posts have the old author image, or even a different author if that was changed). The behaviour seems the same as the post issue, with only some pages actually get the new/updated content via on-demand ISR, despite all being requested to do so.
Sometimes however, it seems to all work. I can't quite figure out a pattern behind when it does.
Deleted/unpublished posts:
When deleting a post, everything seems to work fine, revalidation occurs and the post is removed. However if I instead unpublish it, it only requests revalidation for the homepage and the unpublished route, leaving the now unpublished post in the 'More stories' section of other posts.
Nextjs_Sanity-ISR_Issue02.mp4
The text was updated successfully, but these errors were encountered: