urql-graphql / urql

The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
https://urql.dev/goto/docs
MIT License
8.57k stars 444 forks source link

useQuery re-render even if cached when neighbor data (but not retrieved by useQuery) is newly retrieved #3423

Closed jet2jet closed 8 months ago

jet2jet commented 9 months ago

Describe the bug

In some cases data from useQuery will be updated even if the data is not changed.

Reproduction step

  1. Clone https://github.com/jet2jet/urql-test3 and npm install
  2. npm run start
  3. In http://localhost:3000/ , enter 1 to ID: field and click Get
  4. Expected behavior: Render count in List section will not increase
  5. Actual behavior: Render count will increase

Notes

I found that if data contains a scalar array (in the above reproduction tags field), the problem will occur.
In this case the program will reach to the following line that fieldValue (array instance) is not equal to input[fieldAlias]:
https://github.com/urql-graphql/urql/blob/%40urql/exchange-graphcache%406.3.3/exchanges/graphcache/src/operations/query.ts#L477

Reproduction

https://github.com/jet2jet/urql-test3

Urql version

urql v4.0.5 @urql/exchange-graphcache 6.3.3

Validations

kitten commented 9 months ago

I haven't fully looked at this admittedly, but looking at this, are you referring to tags i.e. a scalar list of string[] / [String]?

If so, if you have two queries that, instead of defining fragments and hoisting & spreading them in a single query, load the same scalar field then the two values cannot be equal.

This is because two queries will launch two operations that will both be fulfilled by the API, even if they overlap. In turn, this means that tags: [String] will be loaded twice and the resulting value, given that it's a reference, is not normalised and can never be reference equal to itself — i.e. since it's unrealistic to expect them to be deeply compared, since we treat all scalars as opaque values, they'll never be equal once they've been fetched from the API twice.

This could of course be prevented by somehow treating API results in a special manner and assuming that they're JSON-equal, and comparing that. However, this would have a significant impact on the performance of cache reads. After all, the cache reads are expected to visit each field in the query AST and process them.

So, I don't think that this is behaviour that's unexpected, and I'd recommend you to — if you expect this to behave differently in a real app — to hoist and combine queries as much as possible and otherwise either make sure that you deep-diff parts of data where you need (which of course shouldn't be needed often) or strategically memoise rendering results or computed values.

Sorry if that's not the answer you were looking for though!

jet2jet commented 9 months ago

Thanks for the reply. In the reproduction, individual Todo (query TodoDetail) does not include tags, but when query TodoDetail is executed, tags in TodoList is updated.
I think there might be a case that the client want to retrieve non-detailed list for the performance reason (server dependent, maybe), so the reproduction pattern would be a possible case.

JoviDeCroock commented 8 months ago

Yea, the reason is because the List of strings is seen as updated which is being queried from the list, and afterwards from the details/... Tbh, I would not really see this as a bug as much as a maybe-optimization but I reckon comparing those lists is more expensive than just doing a React-render that bails out anyway as it sees no changes.

JoviDeCroock commented 8 months ago

I am going to close this out as we can't really make assumptions about these scalars, generally speaking the issue would be here where the array reference isn't the same, this makes sense but we can't really deep compare as this could be a Date, ...