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

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

Open
vknez opened this issue Oct 13, 2021 · 3 comments · May be fixed by #140
Open

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

vknez opened this issue Oct 13, 2021 · 3 comments · May be fixed by #140

Comments

@vknez
Copy link

vknez commented Oct 13, 2021

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

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

vknez commented Oct 13, 2021

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

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.

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

Successfully merging a pull request may close this issue.

3 participants