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

Add ability to update the cache after a mutation #52

Closed
jackdclark opened this issue Feb 25, 2019 · 7 comments · Fixed by #686
Closed

Add ability to update the cache after a mutation #52

jackdclark opened this issue Feb 25, 2019 · 7 comments · Fixed by #686
Labels
feature New feature or request graphql-hooks graphql-hooks package

Comments

@jackdclark
Copy link
Contributor

Currently the only way to update a record after a mutation is to refetch the entire query.

@jackdclark jackdclark added the feature New feature or request label Feb 26, 2019
@jackdclark jackdclark added the graphql-hooks graphql-hooks package label Mar 5, 2019
@bmullan91 bmullan91 self-assigned this Mar 14, 2019
@bmullan91
Copy link
Contributor

I've been thinking about this + auto refetching after a mutation. Here are the three methods that's been suggested so far:

  1. useMutation(MUTATION, { updateCache })
  2. useMutation(MUTATION, { refetchQueries })
  3. useQuery(QUERY, { refetchAfterMutations })

updateCache

useMutation(MUTATION, { updateCache })

Usage

import { LIST_USERS_QUERY } from "./somewhere";

const CREATE_USER_MUTATION = `...`;

function CreateNewUser() {
  const [createUser] = useMutation(CREATE_USER_MUTATION, {
    updateCache: (cache, result) => {
      const cacheKey = client.getCacheKey({ query: LIST_USERS_QUERY });
      const existingResult = cache.get(cacheKey);
      cache.set(cacheKey, {
        ...existingResult,
        data: {
          users: [...existingResult.data.users, result.data.createUser]
        }
      });
    }
  });
}

Implementation

// sumarised useClientRequest.js
fetchData() {
  return this.client.request(operation, options).then(result => {
    if (options.updateCache) {
      options.updateCache(cache, result)
    }
  })
}

For this to work correctly we need a way to re-render components that would be affected by the cache change.

Adding an event emitter interface to the cache would enable this. We would emit the following events:

  • cache.set(key, data) -> emitter.emit(key, data)
  • cache.delete(key) -> emitter.emit(key)
  • cache.clear() -> emitter.emit('CACHE_CLEARED')

For the component to re-render, we need an additional useEffect inside useQuery that registers the event handlers:

// summarised useQuery.js
const [queryReq, state] = useClientRequest(query, allOpts);

React.useEffect(() => {
  if (!client.cache || !client.cache.on) {
    return;
  }

  // This implies we'd also store the cacheKey in the
  // useClientRequest state
  client.cache.on(state.cacheKey, queryReq);
  client.cache.on("CACHE_CLEARED", queryReq);
  return () => client.cache.off(state.cacheKey, queryReq);
}, [state.cacheKey]);

refetchQueries

useMutation(MUTATION, { refetchQueries })

This is a feature that developers are familiar with in Apollo:

refetchQueries: (mutationResult: FetchResult) => Array<{ query: DocumentNode, variables?: TVariables} | string>
A function that allows you to specify which queries you want to refetch after a mutation has occurred

There is an additional option awaitRefetchQueries that relates to this:

Queries refetched as part of refetchQueries are handled asynchronously, and are not waited on before the mutation is completed (resolved). Setting this to true will make sure refetched queries are completed before the mutation is considered done. false by default

usage

Simple usage with no variables:

import { LIST_USERS_QUERY } from "./somewhere";

const CREATE_USER_MUTATION = `...`;

function CreateNewUser() {
  const [createUser] = useMutation(CREATE_USER_MUTATION, {
    refetchQueries: () => [LIST_USERS_QUERY]
  });
}

Usage with variables:

import { LIST_USERS_QUERY } from "./somewhere";

const CREATE_USER_MUTATION = `...`;

function CreateNewUser() {
  const [createUser] = useMutation(CREATE_USER_MUTATION, {
    refetchQueries: () => [
      {
        query: LIST_USERS_QUERY,
        variables: {
          limit: 3
        }
      }
    ]
  });
}

Implementation

Couple ways to do this:

  1. The client.request().then(handler) handler to be extracted and re-called with each options.refetchQueries - mimicking refetch.
  2. Store a map of cacheKey -> refetchFn in client.queries. It's up to useQuery to add and remove from this map via a separate useEffect().
// sumarised useClientRequest.js
fetchData() {
  return this.client.request(operation, options).then(result => {
    if (options.refetchQueries) {
      const queries = options.refetchQueries(result)
	    const refetchingPromise = Promise.all(queries.map(({ query, options }) => {
       const cacheKey = client.getCacheKey(query, options)
       const refetch = client.queries[cacheKey]
       return refetch()
     }))

     if (options.awaitRefetchQueries) {
        await refetchingPromise
     }
    }
  })
}

refetchAfterMutations

useQuery(QUERY, { refetchAfterMutations: [] })

As suggested by @jgoux in #74

Usage

Here's the simple use case, with no variables:

import { ADD_TODO_MUTATION } from "../somewhere";

const GET_ALL_TODOS_QUERY = `...`;

function TodosList() {
  const { loading, error, data } = useQuery(GET_ALL_TODOS_QUERY, {
    refetchAfterMutations: [ADD_TODO_MUTATION]
  });
}

Usage with variables

Here we only want to refetch if the UPDATE_USER_MUTATION was on the same user and the only we're currently querying.

import { UPDATE_USER_MUTATION } from "../somewhere";

const GET_USER_QUERY = `...`;

function UserDetails({ userId }) {
  const { loading, error, data } = useQuery(GET_USER_QUERY, {
    refetchAfterMutations: [
      {
        mutation: UPDATE_USER_MUTATION,
        filter: result => result.userId === userId
      }
    ]
  });
}

Implementation

// summarised useQuery.js
function useQuery(query, options) {
  const mutationHandlers = useRef({})

  useEffect(() => {
    if (options.refetchAfterMutations) {
      // probably best to map them to be all objects
      // with { mutation, filter }
      options.refetchAfterMutations.forEach((mutation) => {
        const mutationEvent = typeof mutation === string
	  ? mutation
	  : mutation.mutation

	const handler = (result) => {
          if (mutation.filter(result)) refetch()
        }

	mutationHandlers.current[mutation] = handler

	 // we either need a separate event emitter
	 // or share the event emitter between client + cache
	 client.on(mutation, handler)
      })

      // clean up listeners
      return () => {
        // iterate over mutationHandlers
        // client.off(event)
      }
    }
  })
}

Overall thoughts

CacheKey

Currently, the cacheKey is an object from client.getCacheKey(operation, options), this is then passed directly to the cache - where it has its own hashing mechanism so it can be stored as a string to represent an object key.

To make updateCache work each event emitter method would also have to re-hash the same cacheKey object, for example:

cache.on = (cacheKeyObj, listener) =>
  emitter.on(generateKey(cacheKeyObj), listener);

The reason the cache was responsible for hashing the object was:

  • It's an implementation detail of the cache that may change
  • If someone wanted to create a normalised cache, they would need the operation in order to parse the query and create the key ${__typename}_${ID}

How this relates to the new features

  • useMutation(MUTATION, { updateCache }) - Current implementation will support this
  • useMutation(MUTATION, { refetchQueries }) - The implementation where queries are stored in map under client.queries would require a hash key or some sort of string use as the key. - The other implementation is a little more fiddly
  • useQuery(QUERY, { refetchAfterMutations: [] }) - Not directly related, but we'd also need a way to create a nice event name for the mutation.

Possible Changes

  1. Export generateKey(obj): String from graphql-hooks-memcache
  2. Alter client.getCacheKey() to return either String or Object depending if the cache exported it's generateKey method.
  3. Alter client.getCacheKey() to only return a String

Shared event emitter

If we decided to add support for useQuery(QUERY, { refetchAfterMutations }) this requires us to listen to mutation events - this is only something the client can do, since the cache never deals with mutations.

It'd be ince to be able to share the same emitter across client and cache in that case.

Possible Changes

  • Force the end user to pass in the emitter to both cache and client: Not ideal
  • Add a setEmitter to cache API, which the client would pass in the emitter which it would create in it's constructor: Feels a little clunky.

Summary

This just a brain dump and completely open to discussion; I'd like to hear what peoples thoughts and opinions are on the matter :)

@olistic
Copy link
Contributor

olistic commented Jun 6, 2019

@bmullan91 I can give option 3 a try and send a PR, is that ok?

@olistic
Copy link
Contributor

olistic commented Aug 4, 2019

I've been playing with the library trying to implement refetchAfterMutations inspired by @jgoux's example. I reached a point in which my queries refetch when any of the specified mutations run, but only when the component that execute those queries are mounted (because I'm subscribing to the mutations inside useQuery, and not at the client level. I have some ideas in mind that could solve this issue. @bmullan91 @edvinasbartkus would any of you have time to jump on a quick call today to help me define the next steps? If you can't do it no problem, I will write my ideas down later.

@bmullan91
Copy link
Contributor

bmullan91 commented Aug 7, 2019

@olistic could you write down your thoughts / share your WIP, then what your questions are. It'll be worthwhile capturing this info + responses to document why changes have been done for others who come to this issue at a later date.

After that then we could arrange a call 👍

@salmanm
Copy link
Contributor

salmanm commented Aug 19, 2021

This isn't resolved/intentionally closed it seems, so reopening.

@salmanm salmanm reopened this Aug 19, 2021
@github-actions github-actions bot removed the Stale label Aug 20, 2021
@simoneb
Copy link
Member

simoneb commented Aug 21, 2021

@salmanm this was closed automatically by the stale action we introduced a short while ago to begin shrinking down the list of open issues and PRs which didn't receive any activity in a long time.

There is a chance the stale action will close this again if there is no activity, but let's keep an eye on it. Thanks for spotting it.

@simoneb
Copy link
Member

simoneb commented Nov 20, 2021

Closed by #686

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request graphql-hooks graphql-hooks package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants