Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Improved Typings #2322

Merged
merged 5 commits into from
Sep 20, 2018
Merged

Improved Typings #2322

merged 5 commits into from
Sep 20, 2018

Conversation

danilobuerger
Copy link
Contributor

  • Deprecated MutationFunc in favor of MutationFn.

Internally the graphql hoc uses the Mutation comp and passes the mutate
func along, so the typing should reflect this

  • Added missing callbacks to MutationOpts

These callbacks get passed from the hoc to the Mutation comp

danilobuerger and others added 5 commits August 28, 2018 13:23
Internally the graphql hoc uses the Mutation comp and passes the mutate 
func along, so the typing should reflect this
These callbacks get passed from the hoc to the Mutation comp
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @danilobuerger - thanks very much!

@williamboman
Copy link
Contributor

williamboman commented Sep 26, 2018

I think the return type of MutationFn is a bit weird now with void being a possible promise result value, why? This breaks our current types and we need to do a null check in every place where we consume the promise result.

Example of what we need to do in order to satisfy type safety:

   withHandlers<Props, HandlerProps>({
     handleConfirmSubmit: (props: GraphqlProps) => async (formData: ConfirmSubmitFormData) => {
-      const { data } = await props.createStuffMutator.mutate({ variables: formData })
-      doStuffWith(data)
+      const response = await props.createStuffMutator.mutate({ variables: formData })
+      if (typeof response === "object") {
+        const { data } = response
+        doStuffWith(data)
+      }
     }
   })

@hwillson
Copy link
Member

@williamboman Any chance you could submit a PR to improve this? Thanks!

@williamboman
Copy link
Contributor

Sure! I'm not sure whether it's a correct assumption to make that void is never a valid Promise result data type for mutations though, I'll have to 🔎 the code (unless anyone have any pointers 😊)

@danilobuerger
Copy link
Contributor Author

@williamboman the return type of MutationFn didn't change. When using the Mutation comp instead of the graphql hoc (as in your example) the void check was already necessary. However, it might not be needed at all as the return type of ApolloClient.mutate (which is used internally) is Promise<FetchResult<T>> (https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/src/ApolloClient.ts#L269)

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

Successfully merging this pull request may close these issues.

3 participants