jeddeloh / rescript-apollo-client

ReScript bindings for the Apollo Client ecosystem
MIT License
126 stars 18 forks source link

Read variables from within the `useMutation`'s `update` function #128

Open vknez opened 3 years ago

vknez commented 3 years ago

I just had a situation where it would be useful to read variables passed to a mutation from its update function. The official TypeScript definitions state:

export type MutationUpdaterFunction<
  TData,
  TVariables,
  TContext,
  TCache extends ApolloCache<any>
> = (
  cache: TCache,
  result: Omit<FetchResult<TData>, 'context'>,
  options: {
    context?: TContext,
    variables?: TVariables,
  },
) => void;

https://github.com/apollographql/apollo-client/blob/main/src/core/types.ts#L170-L182

My use case is that I wanted to create a hook that uses MySharedMutation.use() under the hood, passing it the update function, so that users of that custom hook do not need to think how to update cache for that specific mutation.

let useMySharedMutation = MySharedMutation.use(~update=updateLogic)
/// later, somewhere else...
useMySharedMutation(/* options, without the need to specify `update` logic*/ { /*variables*/ })

Do you think it makes sense to add options param to ReScript bindings as well? I'm asking because it would be a breaking change in the sense that all users of the update param would need to update their functions passed to update to include the third param ~options.

jeddeloh commented 3 years ago

Thanks for bringing this up! We try to map types 1:1 into ReScript, so the omission of options wasn't intentional, it probably just wasn't there when these bindings were written. So, yes, we should definitely add it. Sadly, inconsequential additions in javascript can create breaking changes here in ReScript, but not much we can do about it. Would you like to take a stab at it?

vknez commented 3 years ago

@jeddeloh I tried, but it wasn't too successful. The link between 'variables and 'jsVariables is not clear to me. I know we serialize 'variables to 'jsVariables and we can map 'jsVariables to 'jsVariables, but in this case do we need to do conversion from 'jsVariables to 'variables, since update accepts a callback function that is passed to the JS side? There are many types that need to accept another type parameter, so I guess someone more experienced in this codebase would be needed.

jfrolich commented 3 years ago

The function inside of the module generated by the ppx to do this is Query.parseVariables. This can either happen in the library or the user can do it, doing the conversion in the library is probably a bit nicer.