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.61k stars 449 forks source link

`cache.updateQuery` works but `cache.link` doesn't #3253

Closed cinan closed 1 year ago

cinan commented 1 year ago

Describe the bug

I found a scenario where updating lists doesn't work with cache.link function.

Let's have a schema:

type Query {
  companies: [Company!]!
  customers: [Customer!]!
}

type Mutation {
  updateCustomerWithNewCompany(id: String!, companyId: String): Customer
}

type Company {
  id: String!
}

type Customer {
  id: String!
  companyId: String
}

Read customers and companies:

const query = gql`
  query {
    customers {
      id
      companyId
    }

    companies {
      id
    }
  }
`

What updateCustomerWithNewCompany does is that takes an existing customer ID, creates a new company with companyId id and connects customer with the company. When calling updateCustomer mutation, I create optimistic Customer response in cache exchange, and then also I create a new link for customers items (e.g. cache.link('Query', 'companies', companies)).

// cache.link method: (doesn't work)
const companies = cache.resolve('Query', 'companies')

if (Array.isArray(companies)) {
  const company = { __typename: 'Company', id: args.companyId }
  const index = companies.indexOf(cache.keyOfEntity(company))

  if (index === -1) {
    companies.push(company)
    cache.link('Query', 'companies', companies)
  }
}

// updateQuery method: (works)
const query = gql`
  query {
    companies {
      id
    }
  }
`

cache.updateQuery({ query }, data => {
  data.companies.push({ __typename: 'Company', id: args.companyId });
  return data;
})

I attach an example with both cache.link and cache.updateQuery methods in src/client.js.

Reproduction

https://github.com/cinan/urql-cache/tree/link

Urql version

urql: 4.0.3 @urql/exchange-graphcache: 6.1.1

Validations

kitten commented 1 year ago

This is expected behaviour.

https://github.com/cinan/urql-cache/blob/6aaa8b06bc46485656da08c42b6a7b58c5c2b156/src/Customers.jsx#L19-L22

Here, you're returning a customer, but there's no selection set for a company.

https://github.com/cinan/urql-cache/blob/6aaa8b06bc46485656da08c42b6a7b58c5c2b156/src/client.js#L31

Here, you're then adding the company to the list but the company does not exist in the cache.

https://github.com/cinan/urql-cache/blob/6aaa8b06bc46485656da08c42b6a7b58c5c2b156/src/client.js#L22

Compare that to this approach (cache.updateQuery), where you're providing a query that writes a company to the cache via a selection set.

In short, if your mutation aims to create or fetch a company, and you plan to then do something with the selection set — optimistic or not — the company object needs to be written to the cache.

The cache.link method only writes a link, i.e. a relation to the cache.

cinan commented 1 year ago

thanks for clarification!

Shouldn't urql print warnings, when linking non-existing item to cache? I and my colleagues often struggle with debugging cache, because we're not sure what operations/actions/anything is "allowed" and what isn't.

kitten commented 1 year ago

Honestly, this isn't as simple unfortunately as somehow printing warnings, and there's many cases where this shouldn't trigger a warning. In other words, the caching logic, even on a field level, isn't as simple as “there's either a hit, a miss, or a partial hit“. Even reducing it to those three states, there’s other conditions where queries should refetch.

Optimistic updates are also special since they can never cause a query immediately, and must wait for the mutation to complete. This is basically another mechanism in the background that can cause apps from getting into a bad state.

And, logging the entire caching logic somehow is likely not going to help, as it'd be hard to interpret.

Especially in the case of cache updates, it’s impossible to tell whether a modification has gone wrong until a subsequent query is updated against the cache. It's totally valid to link to a non-existing object that's written afterwards, for instance.

But when we're in the cache query logic, we can only continue as usual, and it's hard to communicate or even detect when a mistake may have occurred there.

So, for now, I don't have a solution for making this more visible.