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

Invalidate doesn't work as expected for some of the queries, or components not getting rerendered with new data #3254

Closed
3 tasks done
RIP21 opened this issue Jun 7, 2023 · 12 comments · Fixed by #3363
Closed
3 tasks done
Labels
needs more info ✋ A question or report that needs more info to be addressable

Comments

@RIP21
Copy link

RIP21 commented Jun 7, 2023

Describe the bug

Basically, I invalidate a bunch of root queries on mutation. It goes through successfully, but then queries that got invalidated are not getting reexeucted from the network. Or it does, and one of the components (freshly rendered) is updated, but the rest of the subscribed to the same query components hold on to old data.

This is how it looks in the dev tools.

Some did refetch as you can see with the green thing, but some are instant weird reexecutions that lead to nothing.
CleanShot 2023-06-07 at 22 03 59@2x

It looks like a series (15-20 of them) of teardown, execute, update, execute, update, teardown, execute, update.

It's hard to make a reproduction, but this is what my update function looks like.

  updates: {
    Mutation: {
      organizationCreateWithExistingSafe: (_parent, _args, cache, _info) => {
        cache.invalidate("Query", "organizations" as keyof Query)
        cache.invalidate("Query", "safes" as keyof Query)
      },

Maybe I don't understand something?

It's important to note that these two queries are shared extensively in UI through a hook like.

export const useSafesQuery = () => {
  return useQueryData({ query: UseSafes_SafesDocument })
}

And there are at least 2 consumer components on the page rendered at the point of invalidation.
Since there is no warning against such a thing in docs, it's quite a common pattern in the codebase for very reusable stuff like user info etc. that makes no sense to re-query all the time.

Also, it's important to note that network policy everywhere is the default one, so cache-first and not cache-and-network or network-only.

REPRO ATTACHED HERE IS NOT A REPRO. I don't have time to build a repro, especially since it DOES invalidate others correctly rather 90% of the time so to build one is virtually impossible.

I can hop on a call anytime to show it off to anyone interested if that would be easiest.

Reproduction

https://codesandbox.io/s/urql-issue-template-client-iui0o

Urql version

"@urql/core": "~4.0.10",
"@urql/devtools": "~2.0.3",
"@urql/exchange-graphcache": "~6.1.1",
"@urql/exchange-refocus": "~1.0.2",
"urql": "~4.0.3",
"wonka": "~6.3.2",

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct
@kitten
Copy link
Member

kitten commented Jun 7, 2023

I'll admit, I haven't used the devtools myself in a hot minute, and they may need an overhaul in terms of the data we're sending to it, so I can't reliably interpret this from the screenshot right now. That's totally my fault though, sorry!

Basically, first of all, I'm not sure how many queries are active in there and which ones you're expecting to see an update for. But, I'm assuming since you mentioned this that UseSafes_SafesDocument is currently mounted and that the cache.invalidate calls are partially covering its fields.

I believe there's two likely candidates of what's causing this, and we can confirm them, but you'd have to post a couple things here for me 🤔 Either:

  1. The fix in fix(graphcache): Add missing operations record on cache result #3193 may have been incomplete and we don't have the query operation around to re-execute for some reason
  2. The change in fix(core): Strictly deduplicate operations for cache-and-network and network-only #3157 is mistriggering and the Client is dropping the operations

To validate that it's one of the two, you could validate with the debugExchange that:

  • you see your mutation result (so an OperationResult with operation set to your mutation)
  • and sequently; a query operation for your query

Afterwards, it's still interesting for us to see whether there's any result, but in all likelihood, you won't see an operation for your query and then we can validate which one of these two issues (if any) it is.

To validate the first, we may have to modify the @urql/exchange-graphcache code you're using to get more log output. Same for the second 🤔

@RIP21
Copy link
Author

RIP21 commented Jun 7, 2023

@kitten gotcha. Thanks for the quick response.

UseSafes_SafesDocument is currently mounted and that the cache.invalidate calls are partially covering its fields.

Yes, it's mounted in 3-4 places, and cache.invalidate basically invalidates its only root field which is safes (returns an array of safes)

The idea is mutation organizationCreateWithExistingSafe should invalidate safes and organizations which should mark all of the related queries stale and cause them to reexecute (e.g. fetch from network) and so when we redirect to another page we see updates in the view in two components OrgSafeList (freshly rendered) that uses safes and SafePicker (this was on the view before and stayed in the nav bar). The actual result is that we have new data shown in OrgSafeList, but SafePicker shows old data as if it was never refetched. Other mutations that also invalidate safes stop working too surprisingly keeping old safes state as they were at the page load. Maybe it's SSR rehydration to blame? I don't know.
For reference, I also don't have any use of pause: true that may stop updates from coming into the hook. So it's not the reason.

As this refresh is not a critical bug for now (as at least it shows safes on a page we redirect to). I'll get back to this one probably next week or later this week (around Friday). I'll try to translate all you said into actionable debug steps and will report back with all the output.

Hopefully, it won't fix by itself (I'm in the middle of refactoring) and I'll have data to back bug-fixing efforts :)

I just want URQL to work as expected :D I don't want to learn Relay :D (and I hate Apollo)

@kitten
Copy link
Member

kitten commented Jun 7, 2023

Hm, 🤔 So, the query isn't mounted when this issue occurs, but you come back to it?
It's possible that if this only happens to queries that were previously unmounted that it's limited to a Client bug, but I can't be sure at this point.

It'd probably be helpful to know whether the bug happens exactly with queries that are unmounted/inactive (i.e. no component that is mounted is using it) when the mutation completes.

@kitten kitten added the needs more info ✋ A question or report that needs more info to be addressable label Jun 7, 2023
@RIP21
Copy link
Author

RIP21 commented Jun 7, 2023

@kitten no, no. The query is ALWAYS mounted. In the nav header, I have a SafePicker that is rendered all the time, so I expect him to update, but he didn't, while the newly rendered component gets the new data which is quite weird.

@kitten
Copy link
Member

kitten commented Jun 7, 2023

Gotcha 👍 then it's more likely that it's a bug in Graphcache

@nagarajay
Copy link

nagarajay commented Jul 7, 2023

@kitten Hi, I think I am also experiencing the same issue. I will articulate the case for you.

Navbar -I have a dropdown on the NavBar if a user is logged in that dropdown with user name is shown. If the user is not logged then regular buttons are shown. to Login or Register. Navbar is on all pages.

I am using NextJs so on a different page, all courses are given. The courses have "Enroll" button option or "Continue Course" option based on if a user is logged in or not. Similar to this there are many pages where the Query that checks if a user is logged in or not is there.

So when the user is not logged in I browse the Courses page and it shows Enroll --> Correctly. Then I click the navbar Login button and login with correct credentials. The login works as the Profile changes with the user name on the NavBar. This is happening as I have written and updates after Mutation
login: (result: LoginMutation, args, cache) => {
cache.updateQuery(
{ query: MeDocument },
(data: MeQuery | null) => {
if (result.login?.errors) {
data = data;
} else {
data = { me: result.login?.user };
}
return data;
}
);
},

But now when I access the courses page the page shows enrolled on the same course, where as the user is already enrolled. Once I reload the page it shows me "Continue Course" correctly.

Apologies for not sharing a repo with this. But any help in this direction would be helpful. I have tried invalidating the MeQuery - User fetching query but it is not working.

Regards
Ajay


The requestPolicy in the above case was set to default "cache-first". Once I change the request policy to "cache-and-network" it works as expected with an additional graphql request.

@kitten
Copy link
Member

kitten commented Jul 22, 2023

@RIP21: Friendly ping here ✌️ I'll close this if this is resolved or there's further inactivity, as I don't really have more indications of this being a bug that's affecting more users, and is more likely to come down to either a specific usage pattern or usage error.

If it's a bug, or if we get a reproduction here, I'm happy to come back to it.


@nagarajay: What's likely to be happening in your case is that a cache miss occurs here. I'd suggest replacing cache.updateQuery with cache.link first and checking what happens to queries then. It's not a safe assumption that result?.login.user from your mutation matches the selection set of the MeDocument's me field.

@RIP21
Copy link
Author

RIP21 commented Jul 22, 2023

Yeah, it's likely some weird Next bug rather than URQL bug. Also, other issues that I had and we discussed in discussions, about browser freezing etc. It's all was down to Next bugs that happen on redirects and only in production builds. As they were reproducing even when I moved everything back to Apollo.

Next just very shitty with Suspense enabled frameworks it seems and it causes some weird incompatibilities on route changes. But only in production.

@JoviDeCroock
Copy link
Collaborator

JoviDeCroock commented Aug 29, 2023

This is a minimal reproduction to the issue to run it:

  • run npm run server in one terminal
  • run npm start in the other

EDIT: stackblitz bugged out often so I changed it to a repo https://github.com/JoviDeCroock/urql-invalidate-repro

@kitten
Copy link
Member

kitten commented Aug 31, 2023

Since we don’t have a full validation here of what happened, do let me know (if you’ve got time of course) if @urql/core@4.1.2 alleviates the issue. So far, I believe it should help with some unexpected stalls in general, so even if it changes the behaviour you’re seeing there’ll hopefully be more information to follow up on

@ookil
Copy link

ookil commented Feb 23, 2024

I think there might still be an issue with graphcache. Or at least that's what I'm experiencing. I made a simple repro https://github.com/ookil/urql-graphcache. Or maybe it's a totaly different issue with Next app router but invalidated queries are not refetched. You have to reexecute the query to get new data

@thecynicalpaul
Copy link

I am experiencing the same issue these days with either requestPolicy, but I am running on react-native for what it's worth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more info ✋ A question or report that needs more info to be addressable
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants