Multiple requests when using react suspense about urql HOT 13 CLOSED

Hey @danielkaxis, thanks for opening this issue. I'll have to check into this as well because I just ran across this same problem when using React.lazy – useQuery underneath it doesn't seem to keep the Suspense boundary in a loading state 🤔

from urql.

I have been trying to make this work as good as I can but ultimately this is not really a structure we want to encourage, you can ofcourse make it work with an exchange that does the deduping for you.

The pattern here is a bit counter to GraphQL where you are creating a lot of requests throughout your application that are inter-dependent rather than one big selection-set.

Alright, thanks @JoviDeCroock. I'm beginning to understand the issue. My ideas were pointing at that direction when I wrote the previous message. We do have several queries accessing the same entities like in my example. @HitomiWin tried the deduping you linked and said it seems to work.

The better way is to do one query. We will try this out. Thank you so very much!

from urql.

Hey,

It's really unclear what you are expecting to happen here or how we would observe the issue. I would encourage you to give us clear reproduction/observation steps and upgrade your urql dependencies.

The logger API can also provide insight into what's happening https://commerce.nearform.com/open-source/urql/docs/api/graphcache/#cacheexchange

from urql.

Hey @JoviDeCroock,

Thanks for your reply. I have now updated the example and intructions.
Thanks for the tip about the logger API. I will check that out.

from urql.

Hey,
I have tried to console log the issues with the logger function.
It looks that queries once called are not cached ??? 🤔

from urql.

As I've said before, it's quite hard to deduct what's going on in your reproduction. I'll try to spend some time on this over the next week, something that could go a long way would be for you to check whether it helps to

• Hoist client creation to the global scope (as it stands you do initClient in a Suspense tree without memo/... which is dangerous in and of itself)
• Identify at what version this started happening

from urql.

As I've said before, it's quite hard to deduct what's going on in your reproduction. I'll try to spend some time on this over the next week, something that could go a long way would be for you to check whether it helps to

• Hoist client creation to the global scope (as it stands you do initClient in a Suspense tree without memo/... which is dangerous in and of itself)
• Identify at what version this started happening

Sorry if my new example is also unclear. I really really tried 😿 I'm not sure how I can explain it better. Thanks for the tip. I will try it.

from urql.

It is def weird that it's being re-queried, it almost feels like GraphCache is just removing data. This might just come down to layers and the mutation layer skewing here 😅 mainly because all operations are in-flight at the same time.

Currently don't have time to debug this but ye, might be something in there. Your thesis is correct though that this produces multiple requests, when using the debugExchange I can clearly see that every request is duplicated.

Update after spending the evening on this, the offending lines are situated here. Basically we should change this up to check whether it's not queued and whether the dispatched does not contain it yet like:

if (!queued && (!dispatched.has(operation.key) || operation.context.requestPolicy === 'network-only')) {
  queue.push(operation);
  Promise.resolve().then(dispatchOperation);
} else {
  dispatched.delete(operation.key);
  Promise.resolve().then(dispatchOperation);
}

That however re-introduces the issue we faced in #3254 where we do an optimistic mutation which results in the dispatch and when the response comes in the updater results in a second dispatch this second one does not go throug has the optimistic reexecute is seen as dispatched. Will need some more time on this but thought I'd update this issue already 😅

from urql.

It is def weird that it's being re-queried, it almost feels like GraphCache is just removing data. This might just come down to layers and the mutation layer skewing here 😅 mainly because all operations are in-flight at the same time.

Currently don't have time to debug this but ye, might be something in there. Your thesis is correct though that this produces multiple requests, when using the debugExchange I can clearly see that every request is duplicated.

Update after spending the evening on this, the offending lines are situated here. Basically we should change this up to check whether it's not queued and whether the dispatched does not contain it yet like:

if (!queued && (!dispatched.has(operation.key) || operation.context.requestPolicy === 'network-only')) {
  queue.push(operation);
  Promise.resolve().then(dispatchOperation);
} else {
  dispatched.delete(operation.key);
  Promise.resolve().then(dispatchOperation);
}

That however re-introduces the issue we faced in #3254 where we do an optimistic mutation which results in the dispatch and when the response comes in the updater results in a second dispatch this second one does not go throug has the optimistic reexecute is seen as dispatched. Will need some more time on this but thought I'd update this issue already 😅

Wow! This is amazing news. I can try to test this locally and see if the issue goes away :)

On a side note, hoisting of urql did not help.

from urql.

Thanks @JoviDeCroock! We will upgrade our packages and make sure it works. Much appreciated.

from urql.

Sadly, these changes did not make it for us. I have an updated example project with the upgraded packages.

"@graphql-tools/schema": "10.0.3",
"@urql/core": "5.0.2",
"@urql/devtools": "2.0.3",
"@urql/exchange-execute": "2.2.2",
"@urql/exchange-graphcache": "7.0.2",
"graphql": "16.8.1",
"urql": "4.0.7",

createClient is now used from @urql/core
The updated example can be seen here. I have trimmed down the example even more to remove bloat code that is confusing. I have also added a query to an entity that exists directly under Query. The schema now looks like this in graphql.ts.

Query
- User
- Vehicle

Vehicle
- cars
- car(id)

Car
- id
- name

Compared to the example in the issue description Car:name is now called 10 times instead of 8 so there was a change with the update.

I will start from scratch. I have 4 cars.
In Vehicles.tsx I have a query to fetch cars under Vehicles under Query. That query contains the id and name and that data should now be cached.

For every car I return a new component Car. In Car I have a query to fetch car with a specified id. That query should in theory give me the cached data. However it doesn't. It calls the resolver again.

This image shows only cars query in Vechicles.tsx. is removed and not querying any data. Car:name is called 4 times. That is good and what I expect.

This image shows together with car querying data for a specific id. For every car id I query Query:Vechicle:car(id). I expect that query to give me cached data. But it doesn't. It calls the resolver again. Car:name is now called 10 times.

As in the initial example I also remove the mutation before the lazy loading is complete. Car:name is now only called 8 times.

Interesting as well is that I also added a user query in Car.tsx but that query is returning cached data and only calls User query: 1 which is good and what is expected.

To be honest I don't think this is even related to anymore. When I remove Suspense and just render the components normally I get the same behaviour.

On a side note:
In our real project we managed to workaround the issue of duplicate requests. We did that by using createClient from @urql/core instead of urql and downgrading @urql/core from 4.2.0 to 4.1.1. This did not work in my example though.

from urql.

I have been trying to make this work as good as I can but ultimately this is not really a structure we want to encourage, you can ofcourse make it work with an exchange that does the deduping for you.

The pattern here is a bit counter to GraphQL where you are creating a lot of requests throughout your application that are inter-dependent rather than one big selection-set.

from urql.

I want to post an updated example project for anyone interested in the outcome of this issue.
https://github.com/danielkaxis/urqlsuspensetest/tree/multiplerequestsresolution

I removed the additional query document for fetching a single car. I now only have one document fetching all cars.
The schema is still as before.

// Schema
Query
  vehicle
  
Vehicle
  car
  cars
  
Car
  name

// Query
export const VehiclesDocument = gql`
  query {
    vehicle {
      cars {
        id
        name
      }
    }
  }
`

To demonstrate:
In Vehicles.tsx I query VehiclesDocument.
https://github.com/danielkaxis/urqlsuspensetest/blob/multiplerequestsresolution/src/components/Vehicles.tsx

In Car.tsx I also query VehiclesDocument
https://github.com/danielkaxis/urqlsuspensetest/blob/multiplerequestsresolution/src/components/Car.tsx

I have 4 cars
I also see them called only 4 times 😄

from urql.