aerogear / offix

GraphQL Offline Client and Server
https://offix.dev
Apache License 2.0
758 stars 45 forks source link

stop automatically applying cache helpers in `offlineMutate` #350

Closed darahayes closed 4 years ago

darahayes commented 4 years ago

Motivation

While the automatic cache helpers are actually a really cool feature, they are also a constant source of issues being opened in Offix. I think there are two main reasons why we see issues being opened.

Confusion about what the helpers actually do

We extend the mutate options with additional fields such as operationType and updateQuery and then all our main docs for offline mutations show these being used but we don't explain well (if at all) that these options are being mapped to an update option under the hood.

Our original intention was to provide utilities to make it easier to get started with offline use cases because cache updates are hard. But instead we just hid the fact that these cache updates even exist. This is confusing for new users and it usually means there are additional hoops to jump through in terms of learning about the cache.

See these issues for examples of confusion around cache helpers

https://github.com/aerogear/offix/issues/340 https://github.com/aerogear/offix/issues/344

The Cache Helpers do not cover every use case

The cache helper functions only cover some very basic use cases.

The cache helpers are good for these cases but they immediately stop working when you need to use wrapper types for pagination, filtering and relationships with other types.

See these issues for cases that cache helpers do not cover right now:

https://github.com/aerogear/offix/issues/63 https://github.com/aerogear/offix/issues/71 https://github.com/aerogear/offix/issues/70

Moving Forward

The reality is that cache updates are hard, they are tricky to maintain and it's really hard to cover all the cases in a generic way. That being said I still think they're valuable and I'd like for us to offer the best support we can but I think they should not be included in the core. Instead, the cache helper functions should be something that the user can import within their app and then pass explicitly as part of offlineMutate

Instead of something like

client.offlineMutate({
  mutation: ADD_TASK,
  variables: { ... },
  updateQuery: GET_TASKS,
  returnType: 'Task',
  operationType: CacheOperation.ADD,
  idField: 'id'
})

I think we should move towards something along the lines of:

// naming to be decided
const { addItemToQuery } from 'offix-cache';

client.offlineMutate({
  mutation: ADD_TASK,
  variables: { ... },
  update: addItemToQuery({
    query: GET_TASKS
    idField: 'id'
  })
  returnType: 'Task',
  idField: 'id'
})

// You could use `deleteItemFromQuery`
// These would also be applicable for subscriptions too

Obviously the exact API would need to be nailed down but I think moving to this more explicit approach will have many all round improvements.

darahayes commented 4 years ago

On more investigation I've found that there's not a whole load of improvements we can actually make on the API level without a huge amount of work. As part of the investigation, I did some minor refactoring of the offix-cache package in https://github.com/aerogear/offix/pull/353 to make the code a bit more readable/maintainable. Ultimately I think our solution to the issues outlined above will be to improve our docs. Going to close this one now.