apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.36k stars 2.66k forks source link

Nesting .modify() results in React unmounted warning #7061

Open lorensr opened 4 years ago

lorensr commented 4 years ago

Edit: see https://github.com/apollographql/apollo-client/issues/7061#issuecomment-699041546 for a description of the current issue


https://www.apollographql.com/docs/react/caching/cache-interaction/#cachemodify has examples of modifying a root query field and the field of a normalized object. I'm wondering how to modify a nested field of a root query field, Query.currentUser.favoriteReviews:

The old way with read/writeQuery:

const [removeReview] = useMutation(REMOVE_REVIEW_MUTATION, {
  update: (cache) => {
    const { currentUser } = cache.readQuery({ query: READ_USER_FAVORITES })
    cache.writeQuery({
      query: READ_USER_FAVORITES,
      data: {
        currentUser: {
          ...currentUser,
          favoriteReviews: currentUser.favoriteReviews.filter(
            (review) => review.id !== id
          ),
        },
      },
    })
  },
})

I'm looking for something like this (which doesn't work because fields.currentUser is supposed to be a function):

  const [removeReview] = useMutation(REMOVE_REVIEW_MUTATION, {
    update: (cache) => {
      cache.modify({
        fields: {
          currentUser: {
            favoriteReviews: (currentUserRef, { readField }) =>
              readField('favoriteReviews', currentUserRef).filter(
                (reviewRef) => readField('id', reviewRef) !== id
              ),
          },
        },
      })
    },
  })

In this case, the currentUser object is not normalized. If it were, I could do:

  const [removeReview] = useMutation(REMOVE_REVIEW_MUTATION, {
    update: (cache) => {
      const { currentUser } = cache.readQuery({ query: READ_USER_FAVORITES })
      cache.modify({
        id: cache.identify(currentUser),
        fields: {
          favoriteReviews: (favoriteReviewRefs, { readField }) =>
            favoriteReviewRefs.filter(
              (reviewRef) => readField('id', reviewRef) !== id
            ),
        },
      })
    }
  })

Although it feels weird to mix readQuery and modify.

lorensr commented 4 years ago

I also don't see how to delete an object from the cache? Looks like the DELETE sentinel is just for deleting an object's field. And when you remove a comment ref from a list of comment refs, the comment object is still in the cache.

image

Lmk if I should open a separate doc issue or FR for this 🤗

Nickersona commented 4 years ago

+1 On this one. I'm trying to re-order the results of a paginated connection object which comes from a root query. I've got a fieldPolicy on the query that handles a cursor based, load more pagination.

When I go to re-order the items in the cache with writeQuery like I could in AC2, It runs through the fieldPolicy which will append my re-ordered items, not replace them with the new node list. cache.modify seems like the right solve here but I can't figure out how to Identify the root query to use it. Any thoughts here?

A hacky work-around I'm thinking of trying is adding additional arguments to my query which strictly influence how the fieldPolicy operates since I have access to the args. I'd have a default append nodes mode, but with an additional argument my cache update could replace instead. Doesn't really seem right, but seems to theoretically work...

lorensr commented 4 years ago

@Nickersona you can leave out the id property for a root query field.

Or if you mean it's hard to identify which version of root query field with differing pagination arguments, Ben recommends setting keyArgs: false, which will result in a single cache entry with no arguments. https://github.com/apollographql/apollo-client/issues/6394#issuecomment-641633060

benjamn commented 4 years ago

@Nickersona You can modify root query fields by omitting the options.id property when calling cache.modify:

cache.modify({
  // This would work, but you can just omit the id to achieve the same effect.
  // id: "ROOT_QUERY",
  fields: {
    paginatedItems(items, details) {
      return reorder(items);
    },
  },
})

Edit: thanks to @lorensr for answering this 10 hours before me—I left this issue open overnight and didn't see his comment this morning when I wrote mine!

benjamn commented 4 years ago

@lorensr A nested call to cache.modify should work?

const [removeReview] = useMutation(REMOVE_REVIEW_MUTATION, {
  update: (cache) => {
    cache.modify({
      fields: {
        currentUser(currentUserRef) {
          cache.modify({
            id: currentUserRef.__ref,
            fields: {
              favoriteReviews(reviews, { readField }) {
                return reviews.filter(review => readField('id', review) !== id)
              },
            },
          })
          // Keep Query.currentUser unchanged.
          return currentUserRef
        },
      },
    })
  },
})

I also don't see how to delete an object from the cache? Looks like the DELETE sentinel is just for deleting an object's field. And when you remove a comment ref from a list of comment refs, the comment object is still in the cache.

You can use cache.evict({ id: cache.identify(...) }) to delete the whole object, though I'm open to adding options to cache.modify to facilitate deletion.

lorensr commented 4 years ago

nested call to cache.modify

🤯😄 Thanks @benjamn! It works but results in this console warning: Can't perform a React state update on an unmounted component.

image

Does not produce warning:

  const [removeReview] = useMutation(REMOVE_REVIEW_MUTATION, {
    update: (cache) => {
      cache.modify({
        fields: {
          reviews: (reviewRefs, { readField }) =>
            reviewRefs.filter((reviewRef) => readField('id', reviewRef) !== id),
        },
      })
    },
  })

Produces warning:

  const [removeReview] = useMutation(REMOVE_REVIEW_MUTATION, {
    update: (cache) => {
      cache.modify({
        fields: {
          reviews: (reviewRefs, { readField }) =>
            reviewRefs.filter((reviewRef) => readField('id', reviewRef) !== id),
          currentUser(currentUserRef) {
            cache.modify({
              id: currentUserRef.__ref,
              fields: {
                favoriteReviews: (reviews, { readField }) =>
                  reviews.filter((review) => readField('id', review) !== id),
              },
            })
            return currentUserRef
          },
        },
      })
    },
  })

Diff: https://github.com/GraphQLGuide/guide/commit/34ae29ebc63048e2783754a13662b9d33d1a5dc0 Repro: https://github.com/GraphQLGuide/guide/tree/repro-7061 Let me know if you'd like instructions on running & triggering the warning

The line referenced in the error is:

      {reviews.map((review) => (
        <Review key={review.id} review={review} />
      ))}

(the mutation code is inside a <Review> that is being removed from reviews)

https://github.com/GraphQLGuide/guide/blob/repro-7061/src/components/ReviewList.js#L40

Related but probably broad issue: https://github.com/apollographql/apollo-client/issues/6209

Nickersona commented 4 years ago

@Nickersona you can leave out the id property for a root query field.

Or if you mean it's hard to identify which version of root query field with differing pagination arguments, Ben recommends setting keyArgs: false, which will result in a single cache entry with no arguments. #6394 (comment)

Thanks for the response guys. So I didn't realize I could just not pass an Id to get the root query. That got me half way there 🎉 , but now I'm running into the problem of identifying my Connection field with it's set of variables. I've setup the @connection directive to ignore my pagination arguments. So are you telling me that with cache.modify, It's impossible to distinguish between multiple cache entries for connections with different argument sets?

Would something like this make sense?:

  cache.modify({
    fields: {
      [cache.identifyFieldConnectionKey('queryName', variables)]: (cachedSearchIssues) => {
        // replace specific query cache entry
      },
    },
  });

Where identifyFieldConnectionKey() would return the corresponding query cache key with all the identified keyArgs.

I'm not quite sure what you mean by

Ben recommends setting keyArgs: false, which will result in a single cache entry with no arguments.

For my use case is that there can potentially be many of these paginated lists on a given page. I need to be able to have separate cache entries for both. Are you saying that in this case I need to build a custom caching mechanism in the fields read/write functions which can map the arguments back to distinct cached lists, basically ignoring the @connection directive?

benjamn commented 4 years ago

@Nickersona If you're using keyArgs to achieve the same separation that @connection allows, then you have the right idea, since that's what it's for. In fact, you probably do not need to use @connection if you're using keyArgs: [...].

So are you telling me that with cache.modify, It's impossible to distinguish between multiple cache entries for connections with different argument sets?

If you're using field arguments to differentiate field values, the cache.modify modifier function will be called for each version of the field. You can tell the calls apart by examining the details object that's passed as the second argument to the modifier function. In particular, details.storeFieldName will be different for different arguments. If you can perform your modifications without worrying about the arguments, that certainly makes things easier. If your modifications depend on the arguments, here's my recommendation for accessing the arguments without having to parse details.storeFieldName: https://github.com/apollographql/apollo-client/issues/6394#issuecomment-656193666

Nickersona commented 4 years ago

Amazing. Thanks @benjamn I wasn't aware of what was available on the details field. Appreciate the help 🙏, and apologies for derailing the original issue.

kieranbenton commented 3 years ago

Firstly, thanks for the information in here - I've struggled to find the information in https://github.com/apollographql/apollo-client/issues/7061#issuecomment-698996938 documented anywhere. Am I missing something or should the documentation be updated with a more complex example to show how to modify nested data, which I thought was quite a common scenario but perhaps not?

Secondly, does anyone know if there is any way to get Typescript types for cache.modify? Right now although this method works it feels extremely fragile to me as its not tied to any of my generated TS types for my queries.