Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutation creating a new object doesn't trigger a watcher of a list of the same item type #281

Closed
vandadnp opened this issue May 24, 2018 · 14 comments

Comments

@vandadnp
Copy link

vandadnp commented May 24, 2018

Hi and thank you for GraphQL and the wonderful Apollo iOS library 👍

Imagine that your server has a list of books which you can query like this:

query {
  books {
    id
    name
  }
}

And then you have a mutation using which you can create a book:

mutation book(name: "MyBook) {
  id
  name
}

Which sends back the created book. We also have set up our cacheKeyForObject property like so:

client.cacheKeyForObject = { [$0["__typename"], $0["id"]] }

We are also retaining our watcher object so we are sure it is not getting deallocated however, the watcher is not getting called telling us that a new book has been inserted and cached.

Could somebody please shed some light as to what we could be missing or if this scenario is even supported?

Thanks very much

@martijnwalraven
Copy link
Contributor

This scenario isn't really supported right now, and it can't be done automatically. There is no way for Apollo to know what the books field returns and whether a new object should be inserted in the list (and where). From the perspective of the client, it is an arbitrary field and it could return anything. If it helps, imagine what would need to happen when you queried a books(publishedAfter:) field: the client would need to know what publishedAfter represents and how to decide whether a new book belongs in the list.

The JavaScript Apollo Client handles this by allowing you to directly manipulate the cache. See here for a similar example. The groundwork for this has been laid in Apollo iOS (see ApolloStore.withinReadWriteTransaction), but it hasn't been exposed as a public API yet.

@vandadnp
Copy link
Author

Hi and thanks for your reply.

The book mutation returns a newly created book of type Book. The first query also returns an array of Book objects.

So this is quite obvious (at least to me) that the mutation creates a new book with an ID that is not in the local cache and hence should be inserted into the cache. Or am I missing something?

Thank you again for your reply

@martijnwalraven
Copy link
Contributor

martijnwalraven commented May 24, 2018

The problem is not that the new object itself won't be inserted in the cache, the problem is that that won't be reflected automatically in the books field.

Again, there is no way to tell which Book objects should be in that list (and in what order that list should be). The server can return any list of Book objects it wants for books. The fact that it is obvious to us it contains a list of all books in insertion order isn't part of the schema.

@vandadnp
Copy link
Author

Hi @martijnwalraven and thanks for your reply.

Would it help to use fragments to tell the Apollo client what we are expecting?

fragment BookFragment on Book {
  id
  name
}

query {
  books {
   ...BookFragment
  }
}

And also when we create a new book, we can do this:

mutation book(name: "MyBook) {
  ...BookFragment
}

This way we are hinting to the client that the list is of type [BookFragment] and so is the result of creating a new book, a BookFragment object.

The reason I believe this should work is that this tutorial also does the same thing, and it works for them (TM).

@martijnwalraven
Copy link
Contributor

martijnwalraven commented May 25, 2018

No, using a fragment has nothing to do with this. The reason the tutorial works is because the mutation itself returns the new list. If you look at the source, the mutation that gets executed is:

mutation CreatePokemon($name: String, $url: String!, $trainerId: ID) {
    createPokemon(name: $name, url: $url, trainerId: $trainerId) {
        ...PokemonDetails
        trainer {
            id
            ownedPokemons {
                id
            }
        }
    }
}

So the mutation result includes the updated list of ownedPokemons. That only works because there is a way of getting there from the mutation result root (going through trainer).

For now, the best workaround is probably to manually re-execute the books query after adding a new book.

@vandadnp
Copy link
Author

@martijnwalraven thanks for your reply

So either we get the server to return the list of books when we add a book or we just re-execute the books query after we have added the new book?

I spoke to one of our server developers and in his eyes returning all the books after creating a new one is an overkill from their perspective.

After creating a new book, if we execute the books query to get all the books, will our watcher on books trigger?

Thank you

@martijnwalraven
Copy link
Contributor

Yes, I think those are your two options. Note that you don't necessarily need to refetch any of the data for individual books, you only need the ids of the books:

query {
  books {
    id
  }
}

That would trigger your existing watcher and as long as the mutation returned all the fields of the new book you need for your original query, it should just load the whole list from cache.

@vandadnp
Copy link
Author

Thank you @martijnwalraven
What should we specify as cache policy for our watchers? Right now we are using returnCacheDataElseFetch is this correct?

@martijnwalraven
Copy link
Contributor

The cache policy only applies to the initial fetch, so this should be fine. When a watch is triggered, it will always try to load from cache.

@vandadnp
Copy link
Author

We seem to now have hit another problem with the watchers...

We are using Firebase and whenever we fire a new API, we ask Firebase for authorisation ID and then create a new ApolloClient. It seems like the watchers have a weak reference to the client so if we kill the old one, the watcher won't understand the change?

What is the best practice here? Because our token will change from time to time so we cannot hold onto the client object

@martijnwalraven
Copy link
Contributor

Each client has its own cache, so none of this will work if you create multiple client instances. Definitely don't do that, it will lead to all kinds of trouble!

If you have a need to change the token based on a Firebase call, you can create your own implementation of HTTPNetworkTransport and do whatever is needed there. There is an open PR to make this easier by adding a delegate, but that seems to have stalled.

@vandadnp
Copy link
Author

Thank you @martijnwalraven
I am trying to subclass HTTPNetworkTransport but it's not an open class, so I cannot subclass it!

@martijnwalraven
Copy link
Contributor

No, you should create your own copy because it isn't meant to be subclassed.

If you don't feel comfortable with this, you could also use this Alamofire transport.

@vandadnp
Copy link
Author

Thank you for your help @martijnwalraven our problem is now solved. We created a custom transport object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants