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.6k stars 448 forks source link

When maskTypename: true, a new reference is returned for useQuery data when the query becomes stale #3297

Closed karl-power closed 1 year ago

karl-power commented 1 year ago

Describe the bug

When a query is invalidated after a mutation selects the same typename, the query immediately returns a new reference to the existing data when stale becomes true and the query is re-executed. This only happens when maskTypename: true is set on the client instance, and seems to be a new behaviour since v4. The behaviour is the same whether the query is invalidated using additionalTypenames, or automatically based on the typenames that are selected both in the mutation and query. In our case (and likely all others), the new data reference is invalidating and re-running useEffect/Memo/Callbacks before the actual structure of the data has possibly changed when the query resolves.

In the attached reproduction link, the small React application will add or remove a "hooray" reaction to this issue when the respective button is clicked. After each mutation, the repository query is then re-executed to fetch all reactions for this issue. There is a useEffect that contains only the useQuery data value in its dependency array, so we can easily see when the data reference changes. With maskTypename: true, the reference changes once when the query is executed (the actual data is the same), and once again when it resolves. Without maskTypename: true, we see the "correct" behaviour where the reference only changes after the query has resolved.

Note: A GitHub personal access token (classic) is required to query the GitHub GraphQL API in the reproduction app. It can be created at https://github.com/settings/tokens/new, with the public_repo scope checked.

Reproduction

https://codesandbox.io/s/elegant-bhaskara-4qkldf

Urql version

urql v4.0.4

Validations

kitten commented 1 year ago

Just a disclaimer, I'm thinking of deprecating maskTypename, since it's only useful in case of "laziness" πŸ˜… to be more precise, it's only useful, if *three whole conditions are true:

So, that's a lot of "ifs" for a pattern that's basically not technically speaking best practice imho πŸ˜…

Anyway, that said, the fix for this particular situation is simple; I can do the maskTypename logic earlier in the chain.

That said, it doesn't fix any of the underlying issue, i.e. it'll mess with reference equality guarantees in Grapchache either way. Just stating this here, because, it's probably easier to document here that this old feature may not last, and that future fixes re. reference equality won't be applied πŸ˜…

karl-power commented 1 year ago

Hi @kitten Thanks for your response and the explanation. We will remove our usage of maskTypename in that case - we can survive without it πŸ˜„