controlledVariant type mismatch from useMutation promise result

[peterpme]

Hi

  333 ┆   |> Js.Promise.then_(response => {
  334 ┆        switch (response) {
    . ┆ ...
  340 ┆        Js.Promise.resolve();
  341 ┆      })
  342 ┆   |> ignore
  343 ┆ | EditFunction(customCodeUuid) =>

  This has type:
    Js.Promise.t(ApolloHooks.Mutation.controlledVariantResult('a)) =>
    Js.Promise.t(unit)
  But somewhere wanted:
    Js.Promise.t(ApolloHooks.Mutation.result({. "addCustomCode": {. "body":

After upgrading to 6.0.0 I've been getting a whole ton of these. A user named light.wave was able to figure this out by annotating:

Js.Promise.then_((result: ApolloHooks.Mutation.result('a)) => 

is this the right approach? if so can we add this to the README / examples? Happy to help. Thanks!!

[fakenickels]

Hey there!
This is a bit strange indeed, it should have been inferring correctly. I'll look into it that but having it in the README would be helpful

[mbirkegaard]

TL;DR That is the correct approach.

When you write

  333 ┆   |> Js.Promise.then_(response => {
  334 ┆        switch (response) {
    . ┆ ...
  340 ┆        Js.Promise.resolve();
  341 ┆      })
  342 ┆   |> ignore

are you doing something like?

switch (response) {
  | Data(data) => doSomething(data)
  | Error(error) => ohNo(error)
  | _ => ()
}

Things are type checked a bit unintuituvely. Not from first line to last, but from inner to outer scope. It doesn't go "okay this is a promise with a certain type, so the thing after |> should handle a promise of that type". Rather it goes... "the thing inside the then_ has a certain type, therefore then_ needs a promise that resolved to that certain type, therefore the thing before |> should be such a promise".

You can see this at work if you define a handler in an empty .re file.

let handleResponse = r =>
  (
    switch (r) {
    | Data(_) => "data"
    | _ => "something else"
    }
  )
  |> Js.Promise.resolve;

This will give a compile error saying something like "The variant constructor Data can't be found.". If you open ApolloHooksMutation above, the compiler finds the first type that works and types the handler accordingly. The first type that matches is the controlledVariantResult('a), so handleResponse is becomes

ApolloHooksMutation.controlledVariantResult(
  'a
) => Js.Promise.t(string)

Prior to #95, the mutate function was (incorrectly) returning

Js.Promise.t(ApolloHooksMutation.controlledVariantResult('a))

That mean you could do mutate() |> Js.Promise.then_(handleResponse) and have the typing work.

In 6.0.0, the type of mutate function was changed so that it returns

Js.Promise.t(ApolloHooks.Mutation.result('a))

To make this type check, you need to make sure that the compiler doesn't guess the controlledVariantResult type for responseHandler (or the anonymous function inside then_), which you can do like light.wave figured out.

[mbirkegaard]

is this the right approach?

@peterpme Did you see the explanation? Does it make sense?

if so can we add this to the README / examples? Happy to help. Thanks!!

It would be greatly appreciated if you added something to the README!

[D34THWINGS]

@mbirkegaard Although your explanation is pretty complete and makes sense, we didn't managed to make it work. Even copy/pasting README examples gives compilation errors. This is highly frustrating because it's like we can't use the mutation hook at all. We tried typing result with both: ApolloHooksMutation.controlledVariantResult('a) and ApolloHooksMutation.executionVariantResult('a), still it does not work. It gives error:

This has type:
Js.Promise.t(ApolloHooksMutation.controlledVariantResult('a)) =>
    Js.Promise.t(unit)
But somewhere wanted:
    (Js.Promise.t((ApolloHooks.Mutation.executionVariantResult(...), Js.Promise.t((ApolloHooks.Mutation.executionResult(...)) => 'b

I did a bit of extra digging and the promise does not return result but a tuple (simple, full). This does correspond to the README at all and is highly misleading. We tried to type the result with the tuple but it gives an error:

 The variant constructor Data can't be found.

V6 mutations seems unusable for now 😞I can't find a way to use them other than hacking my way through using simple returned directly by the hook and put a useEffect around it to handle side effects after mutation success/error.

Help on this would be really appreciated. We could update the README later on when we understand how to make it work.

(cc @WhyThat)

[D34THWINGS]

After fighting some more, finally find a way to make it work that is not too heavy:

let handleClick = _ => {
    mutate(~variables=MyMutation.makeVariables(~simeVar=123, ()), ())
    |> Js.Promise.then_((( result: ApolloHooksMutation.executionVariantResult('a), _ )) => {
      switch(result) {
        | Data(data) => Js.Console.log(data##insert_user)
        | Errors(error) => Js.Console.log(error)
        | _ => Js.Console.log("🤷")
      };
      Js.Promise.resolve();
    })
    |> ignore
  }

The key being, destructuring the tuple and focring the type of the part that interest you (simple being executionVariantResult('a)). I can update the readme accordingly if you want to match what needs to be done and be explain why because it's trivial.

[BlueHotDog]

Hi all, just spent 3 hours fighting this too.. an update to the readme would be great! happy to assist.

[mbirkegaard]

@mbirkegaard Although your explanation is pretty complete and makes sense, we didn't managed to make it work.

Yeah... unfortunately, between my explanation and your following message it was updated to return the (simple, full) tuple.

I can update the readme accordingly if you want to match what needs to be done and be explain why because it's trivial.

It would be greatly appreciated if you added something to the README!

[mbirkegaard]

Hi all, just spent 3 hours fighting this too.. an update to the readme would be great! happy to assist.

Will happily review a PR!

[RawToast]

Just in case anyone else gets stuck with this, a quick workaround is to use -> instead of pipe:

mutation(~variables=MyMutation.makeVariables(~stuff, ()), ())
      -> Js.Promise.then_(( result: ApolloHooksMutation.result('a))=> {
        switch(result) {
            | ApolloHooks.Mutation.Data(_data) => ()
            | Error(_error) => ()
            | NoData => ()
          };
        Js.Promise.resolve(result);
      }, _) |> ignore;

This looks to be handled better with the new (simple, full) response type.

[EnriqueVidal]

Not sure if this has become stale at this point but I'm having a somewhat related issue that I can't find info for, here's my code:

        mutate(
          ~variables=SignInMutation.makeVariables(~email, ~password, ()),
          (),
        )
        ->Promise.Js.fromBsPromise
        ->Promise.Js.map(
            ((simple: Mutation.executionVariantResult('a), _)) => {
            switch (simple) {
            | Data(data) =>
              let {user} = data##signIn;
              switch (user.email, user.createdAt, user.updatedAt) {
              | (Ok(email), Ok(createdAt), Ok(updatedAt)) =>
                form.reset();
                form.notifyOnSuccess(None);
                User.make(~email, ~createdAt, ~updatedAt)
                ->UserContext.SignIn
                ->dispatch;
              | _ => form.notifyOnFailure(UnexpectedServerError)
              };
            | NoData => form.notifyOnFailure(UnexpectedServerError)
            | Errors(errors) =>
              Js.Console.log2("This never logs");
              form.notifyOnFailure(BadCredentials);
            };
          })
        |> ignore;
      },
    );

My problem is that whenever graphql responds with a GraphQLError rather than make it into the | Errors(erros) => it automatically rejects, I have added this to the end:

->Promise.Js.catch(error =>  {
  Js.Console.log2("this does trigger", error);
  Promise.Js.rejected(error);
});

My graphql mutations do respond with errors based on variables or wether things exist or nat and I would like to be able to sue Error(errors) to respond accordiginly, what am I doing wrong here?

[mbirkegaard]

@EnriqueVidal This doesn't look like it has to do with reason-apollo-hooks. Have you set the errorPolicy? If you haven't the policy is none and errors will cause the promise to be rejected. You'll probably want to set it to all.

https://www.apollographql.com/docs/react/data/error-handling/#error-policies

[EnriqueVidal]

@mbirkegaard thanks for your quick reply I can see how to add that to useQuery but not useMutation:

I mean:

#101 (comment)

vs:

https://github.com/Astrocoders/reason-apollo-hooks/blob/master/src/ApolloHooksMutation.re#L90

Am I' looking at the wrong files?

[mbirkegaard]

You'll need to set it as the global default. Check the docs for defaultOptions. errorPolicy in the ueMutation hook was only recently fixed in apollo-hooks (apollographql/apollo-client#5863) and these bindings haven't been updated to match yet.

[EnriqueVidal]

@mbirkegaard thanks for responging, defaultOptions is not in reason-apollo's createApolloClient would you suggest making a new binding or is there a cleaner way I'm not looking yet?