React-apollo: updateQueries gets extremely contrived when trying to update lists

[migueloller]

I'm using updateQueries to alter a list when a user "likes" or "dislikes" one of the list items but I've run into many issues which I will try to explain below.

My mutation query looked something like this:

mutation ($itemId: ID!) {
  item: toggleLike(itemId: $itemId) {
    id # needed for cache
    liked: viewerHasLiked
  }
}

The first issue I came across is that if I wanted to update the list, I needed to query a lot more than simply id and viewerHasLiked because the list query expects many more fields. After adding these fields I had another issue, I was using optimistic updates so now I had to change that as well in order to not get Apollo errors. At this point it gets very complicated because it's impossible to predict the optimistic result of the entire item and putting dummy data would break rendering.

So here's how I tried to solve it, I simply didn't try to update the query when the result was optimistic (by checking for fields present). The issue with that is that now that the query had all these new fields, the optimistic response HAD to include them.

Ultimately I had to patch up a very hacky solution. I used withApollo on the component so that I could access the client via ownProps. With that, my higher order component ended up looking like this:

export default graphql(gql`
  mutation ($itemId: ID!) {
    item: toggleLike(itemId: $itemId) {
      id # needed for cache
      liked: viewerHasLiked
    }
  }
`, {
  options: ({ itemId }) => ({ variables: { itemId } }),
  props: ({ ownProps: { liked, itemId, client }, mutate }) => ({
    toggleLike: () => mutate({
      optimisticResponse: {
        item: {
          id: itemId,
          liked: !liked,
          __typename: 'Item',
        },
      },
      updateQueries: {
        List: (prev, { mutationResult: { data: { item } } }) => {
          const { queryManager } = client;
          const [queryId] = queryManager.queryIdsByName.SomeQueryNameThatHasSufficientData;
          const itemLiked = queryManager.queryResults[queryId].data.items.edges
            .map(({ node }) => node).find(({ id }) => id === item.id);
          return update(prev, {
            viewer: {
              list: {
                edges: item.liked
                  ? { $push: [{ __typename: 'ItemEdge', item: itemLiked }] }
                  : { $apply: (edges) => edges.filter(({ item: { id } }) => id !== item.id) },
              },
            },
          });
        },
      },
    }),
  }),
});

As you can see, this is suboptimal.

Initially I thought the best way to solve this would be by passing the store as a last argument to the reducer map for updateQueries but then I realized that because this data is normalized, it's not useful. Given this, I had to go into the client's queryManager which is extremely undesirable.

How can we better prevent having to do something like this?

I could've changed the architecture of the app so that the list was only an array of ids and then each rendered item got it's details but that would've increased the number of requests to the server (I could've done query batching but I've been getting various errors with it so it wasn't an option).

The cleanest solution that comes to mind would be to expose all of the queries as the last argument to the reducers in updateQueries so that the user doesn't have to pass in the client and go into the queryManager. Perhaps, the query manager itself could be passed? (but then how do you access the query by name?

Would love to hear your thoughts on this @tmeasday, @helfer, and @stubailo.

Thanks!

[tmeasday]

@migueloller If I understand your use case correctly, the issue here is that when a user likes an item, you want to update the List query to either add the item (which is tricky because you need get other fields for that item) or remove it.

I think the API you want here, rather than getting the results of some known query to get the item details, is instead to query the store for the item. Something like:

      updateQueries: {
        List: (prev, { mutationResult: { data: { item } }, store }) => {
          const itemLiked = readObjectFromStoreById(store, item.id);
          return update(prev, {
            viewer: {
              list: {
                edges: item.liked
                  ? { $push: [{ __typename: 'ItemEdge', item: itemLiked }] }
                  : { $apply: (edges) => edges.filter(({ item: { id } }) => id !== item.id) },
              },
            },
          });

There is an issue floating around (I can't find it, maybe someone else can point you to it) about refactoring updateQueries et al to attach to the query rather than the mutation (or allowing it both ways). If this solves your problem nicely it probably makes sense to add the store as an argument to updateQuery functions.

[tmeasday]

I'm not an expert, but I think the code for readObjectFromStoreById(store, item.id); is literally just store.data[client.dataId(item)], and also you can get store right now with client.store.

[migueloller]

@tmeasday, exactly. I basically just implemented readObjectFromStoreById in the lines:

const { queryManager } = client;
const [queryId] = queryManager.queryIdsByName.SomeQueryNameThatHasSufficientData;
const itemLiked = queryManager.queryResults[queryId].data.items.edges
  .map(({ node }) => node).find(({ id }) => id === item.id);

Keep in mind, though, that it would have to be something along the lines of readObjectFromStoreById(store,${__typename}:${item.id}) or whatever dataIdFromObject returns.

[migueloller]

I'm not an expert, but I think the code for readObjectFromStoreById(store, item.id); is literally just store.data[client.dataId(item)], and also you can get store right now with client.store.

In this case, item wouldn't be available in the implementation because you're just passing the id.

[tmeasday]

I assume dataId just uses item.id and maybe item.__typename?

[migueloller]

I'm seeing this this line that it is whatever is passed into dataIdFromObject. Given that, the API could be:

const itemLiked = readObjectFromStoreLike(store, item);

Internally, readObjectFromStoreLike would call store.data[client.dataId(item)].

There are a couple of issues with this though.

• The user has to be sure that the passed item will yield a good id. This is mostly DX though and there are ways to throw errors if the object can't be read (i.e., it's not found in the store).
• store.data is normalized so you can't really simply get the object like that. This is why I used queryManager.queryResults above. That data is the data as the GraphQL server returns it which is what you really want, not normalized store data.

[tmeasday]

Oh, right, yeah if the item has subfields, you'd need an API like readSelectionSetFromStore. I'm not sure we have one to do so by id.

[tmeasday]

Perhaps we just need a "special" root field for reading by id, so you could do:

readSelectionSetFromStore(store, gql`{
  __objectFromId__(id: ${id}) {
     // fields you want
  }
}`);

[migueloller]

I'm liking where this is going. And I'm assuming it will use graphql-anywhere internally, no?

Perhaps whenever you want to query anything from the store you could simply call a special function that will allow you to do this with GraphQL. Moreover, this special function could be made available as an argument to apollo-client options (e.g., updateQueries). This special function is really just a Redux selector that speaks GraphQL and doesn't need the store.

The API could be something like what you commented above but without the need to pass in the store.

Here's how it could be exposed in updateQueries:

updateQueries: {
  List: (prev, { mutationResult, select }) => {
    const data = select(gql`
      # get what you want
    `);
    // update your queries using `data`
  },
},

[tmeasday]

That seems sensible. Thinking ahead, I am thinking you would also want the shape of the GraphQL query to be the same as a (sub-part) of the GraphQL query you are updating. I'm not sure AC needs to go too far to support this, but what is the best way to do that? Fragments?

[migueloller]

Right now if you try to update the query result and there are fields missing from its selection set, AC will complain. I think this is the right behavior, we simply need better error messages.

If we wanted to implement something a bit more in-depth then I think fragments would be the way to go. You would be able to make a fragment that contains all the data that is needed by the query. That fragment would then be used by the query itself and it would also be used by the select function's query. Then you can be 100% sure the queried fields are the same so there are no fields missing. The one thing that would be missing would be to make sure that you're actually querying an object that exists in the store. This circles back to what I mentioned here regarding DX.

[migueloller]

How finalized is the current API for apollo-client? I know that there have been various changes recently and that there are many more in the works but I would be more than happy to collaborate if it's something you guys are looking for. I've perused over the source code and have seen some room for improvement in documentation and architecture. For example, we could separate different parts of apollo-client into separate modules so that other tool builders could consume them. There could be a module that simply takes care of caching queries, another module could take care of query diffing, etc. I know that @stubailo has mentioned this before (I can't find the comment and the issue where he mentioned it). But ultimately I think that would be what's best for the community as a whole.

[tmeasday]

Yeah, I think there is an effort underway at the moment to do exactly that. I'm not sure what the exact status is right now.

[migueloller]

@tmeasday,

Gotcha, well when I have some time (probably mid October) I'll try to help and get some PRs done. 😄

[stubailo]

We are going to try to knock out some big refactors on Thursday and Friday. The biggest one that will enable huge improvements is refactoring to use graphql-anywhere.

That's kind of like our "power tool" for GraphQL - it will make it super easy to build tons of modules that consume the store and queries in tons of different ways.

[tmeasday]

@stubailo / @helfer is it now possible to do what @migueloller was hoping for - a query like the one here (kind of like a client-side Relay node query) ?

If so, let's close this. If not, let's open a feature request on AC to add it.

[migueloller]

@tmeasday,

With the new result reducers feature it's possible to populate the store with very high flexibility. This is a step in the right direction and all that is missing now is a way to query the store with as much flexibility as we can write to it.

[helfer]

@migueloller I think there are some great ideas in this issue thread here with regards to querying the store. The best way to proceed would be to open an issue with the ideas on apollo-client so we can continue the discussion there and hopefully implement something soon!

[tmeasday]

I'm going to close this issue, I hope that's OK w/ everyone.