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

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

Closed
lorensr opened this issue Sep 23, 2020 · 12 comments
Closed

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

lorensr opened this issue Sep 23, 2020 · 12 comments

Comments

@lorensr
Copy link
Contributor

lorensr commented Sep 23, 2020

Edit: see #7061 (comment) 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.

@benjamn benjamn self-assigned this Sep 23, 2020
@lorensr
Copy link
Contributor Author

lorensr commented Sep 24, 2020

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
Copy link

+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
Copy link
Contributor Author

lorensr commented Sep 25, 2020

@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)

@benjamn
Copy link
Member

benjamn commented Sep 25, 2020

@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
Copy link
Member

benjamn commented Sep 25, 2020

@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
Copy link
Contributor Author

lorensr commented Sep 25, 2020

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: GraphQLGuide/guide@34ae29e
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: #6209

@Nickersona
Copy link

@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
Copy link
Member

benjamn commented Sep 25, 2020

@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: #6394 (comment)

@Nickersona
Copy link

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.

@lorensr lorensr changed the title Document how to .modify() nested cache data Nesting .modify() results in React unmounted warning Oct 11, 2020
@kieranbenton
Copy link

Firstly, thanks for the information in here - I've struggled to find the information in #7061 (comment) 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.

@phryneas
Copy link
Member

phryneas commented Jan 9, 2025

Hi everyone 👋

This issue has changed quite a lot, I'll try to sum it up:

  • the original issue was solved somewhere down the line
  • the new issue about the React unmounted warning should not be a concern much longer as newer React versions do not show that warning anymore
  • the TypeScript types for cache.modifywere improved in v3.8.0 via Improve TypeScript type-safety of cache.modify #10895.

As long-living issues like this tend to get very confusing, I'm going to close this. If there were any additional points in here that you believe still need addressing, please go ahead and open a new issue for each of them so we can track them in isolation.

Thank you!

@phryneas phryneas closed this as completed Jan 9, 2025
Copy link
Contributor

github-actions bot commented Jan 9, 2025

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

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

No branches or pull requests

6 participants