apollo-feature-requests icon indicating copy to clipboard operation
apollo-feature-requests copied to clipboard

Feature request: updateQuery method that can operate on all variables a query has been cached with

Open jedwards1211 opened this issue 5 years ago • 13 comments

@benjamn

With the removal of cache.modify, there's still no simple way to remove a deleted item from all cached queries, since they could be called with many different variables. (https://github.com/apollographql/apollo-client/pull/6289#issuecomment-632842327, https://github.com/apollographql/apollo-client/pull/6289#issuecomment-631072849).

To solve this without the problems cache.modify had, I propose an updateQuery method, as illustrated below. This would be so similar to the existing readQuery/writeQuery methods that it wouldn't risk confusing user in the way that cache.modify did, and it would be very easy to document. But unlike readQuery and writeQuery, it would allow us to update query results for all cached variables in one fell swoop by omitting the variables option.

It would also be more convenient for most existing cases where readQuery followed by writeQuery is used (I've been using a userland updateQuery method for awhile, albeit without the all-cached-variables capability).

type Entry {
  id: String!
}

type Query {
  entries (search: String!): [Entry!]!
}

type Mutation {
  removeEntry (id: String!): String!
}

const query = gql`
  query ($search: String!) {
    entries(search: $search) {
      id
    }
  }
`

async function removeEntry(variables: RemoveEntryVariables, client: ApolloClient<NormalizedCacheObject>): Promise<void> {
  await client.mutate<RemoveEntry, RemoveEntryVariables>({
    mutation: removeEntryMutation,
    variables,
    optimisticResponse: {
      removeEntry: variables.id
    },
    update(cache, {data: {removeEntry}}) {
      cache.updateQuery({
        query,
        // variables: would be optional, if omitted updater is called with all cached query variables and corresponding data
        updater: ({data, variables}: {data: QueryData, variables: QueryVariables}): QueryData => ({
          ...data,
          entries: data.entries.filter(e => e.id !== removeEntry.id),
        })
      });
      cache.gc();
    }
  })
}

jedwards1211 avatar May 22 '20 22:05 jedwards1211

One problem with this structure is just that the cache doesn't store variables for queries. It could support iteration like this but it'd have to pass you the unique storeFieldName instead of the variables.

danReynolds avatar May 22 '20 22:05 danReynolds

@danReynolds I mean it sort of does already, because the variables are serialized in the cache keys. But yeah ideally they should be stored unserialized somewhere, as well as efficient data structures that can point us to all the variable bindings and corresponding data for a given query or field.

Choice of implementation details (like store field names) should support the goals of an ideal API design, not the other way around. Even if something is not possible with the current implementation, let's keep open minds about what can be made possible.

I'm sure the Apollo team will make these improvements to the cache sooner or later because it's going to be the only way to manage cache updates in a large app in a sane way unless you force the responsibility for storing variables into userland. Right now refetching is virtually the only option in a lot of cases.

For example, if you want to insert a newly created item in the correct sorted position in all applicable lists in the cache, you probably need to know variables that determine the sort order and list range for each of those queries.

jedwards1211 avatar May 22 '20 22:05 jedwards1211

Yea you can't just parse the store field name though because field directives for example are also added to it. I'd vastly prefer for them to store variables separately as a first class entity of fields that we can access. I assume the concern with that is mostly bloating the cache size.

danReynolds avatar May 23 '20 00:05 danReynolds

It's hard to imagine a case where the variables take up even close to as much space as the data though

jedwards1211 avatar May 23 '20 01:05 jedwards1211

I must say that when I started using Apollo for the first time on my latest project, I was quite confused about the whole read/writeQuery stuff, it's probably the most commonly used feature (I mean how many projects don't update or delete items?), and yet it just doesn't feel up to scratch for such important functionality. Definitely a +1 from me for this.

lukeramsden avatar May 23 '20 10:05 lukeramsden

It's hard to imagine a case where the variables take up even close to as much space as the data though

Imagine having a search field in your query: variables size can easily get huge.

darkbasic avatar May 25 '20 17:05 darkbasic

@darkbasic you're saying if the search is really long? Or that each keystroke would create another set of variables? (I'm assuming they wouldn't remain cached forever unless data gets loaded, which would probably be larger than the search term)

jedwards1211 avatar May 25 '20 17:05 jedwards1211

Not each keystroke because you definitely want to debounce this, but you will end up with plenty of search terms anyway. Lots of them could have little to no results.

darkbasic avatar May 25 '20 18:05 darkbasic

Only one will be active though so others would get garbage collected if the cache gets too big right?

jedwards1211 avatar May 25 '20 19:05 jedwards1211

Also since the variables already get stringified into the cache keys, any size problems are already present. Storing the unstringified variables wouldn't be a very substantial increase relative to that.

jedwards1211 avatar May 25 '20 19:05 jedwards1211

@benjamn although you're restoring cache.modify, I think it will still be hard to perform some kinds of updates without a feature like this, especially inserting a newly created item into all the sorted lists where it belongs. (In my project the sort order and page range are determined by variables.)

jedwards1211 avatar May 26 '20 18:05 jedwards1211

Thanks for the great discussion here @jedwards1211. We'd like to keep issues here for bugs only, so I'm going to transfer this issue over to our FR repo. Thanks again!

hwillson avatar May 27 '20 01:05 hwillson

I think I probably failed to notice the link in the feature request template, my apologies!

jedwards1211 avatar May 27 '20 03:05 jedwards1211