reasonml-community / reason-apollo-hooks

Deprecated in favor of https://github.com/reasonml-community/graphql-ppx
https://reasonml-community.github.io/reason-apollo-hooks/
MIT License
137 stars 28 forks source link

Fix/mutate promise error #97

Closed mbirkegaard closed 4 years ago

mbirkegaard commented 4 years ago

All credit to @Emilios1995. I'm just the one putting up the PR.

This fixes the type of the promise returned by mutate.

On the ts side, useMutation returns a tuple with this definition:

export type MutationTuple<TData, TVariables> = [
  (
    options?: MutationFunctionOptions<TData, TVariables>
  ) => Promise<ExecutionResult<TData>>,
  MutationResult<TData>
];

where

export interface ExecutionResult<T = Record<string, any>> {
  data?: T;
  extensions?: Record<string, any>;
  errors?: GraphQLError[];
}

The previous reason types assumed that mutate returned Promise<MutationResult<TData>> instead of Promise<ExecutionResult<TData>>

This is a breaking change. Particularly annoying after #89.

fakenickels commented 4 years ago

Will review soon

mbirkegaard commented 4 years ago

@fakenickels When it comes time to release this, can you do a release candidate first? We're starting to migrate a lot of users to our Reason platform so there's a chance that we'll discover more bugs. It would be annoying to keep making braking changes to major releases as we fix them.

Edit: Tagged the wrong person (sorry @baransu)

fakenickels commented 4 years ago

Of course! We have been churning a lot of the last changes

fakenickels commented 4 years ago

Sorry about the delay to merge, we have been a bit busy these past few weeks

mbirkegaard commented 4 years ago

Sorry about the delay to merge, we have been a bit busy these past few weeks

No worries at all! It's OSS! Noone is paying, so noone is owed anything ;)

fakenickels commented 4 years ago

🙏 @mbirkegaard are you on ReasonML Discord server? If so what's your handle there?

mbirkegaard commented 4 years ago

I'm am, though I only pop in occasionally. My handle is FreddieFreeloader