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?

[EnriqueVidal]

@mbirkegaard This is indeed not an issue with apollo-react-hooks I created my own poor man's client binding and using ReasonApollo types (for the most part and it worked), I only added the bits I needed but this is how it looks:

type defaultOptions = {
  .
  "query": {
    .
    "fetchPolicy": string,
    "errorPolicy": string,
  },
  "mutate": {. "errorPolicy": string},
};

type apolloObjectParams = {
  cache: ReasonApolloTypes.apolloCache,
  link: ReasonApolloTypes.apolloLink,
  ssrMode: bool,
  defaultOptions,
};

[@bs.module "apollo-client"] [@bs.new]
external createApolloClient:
  apolloObjectParams => ApolloClient.generatedApolloClient =
  "ApolloClient";

Then I just used like:

let buildClient = (~uri, ~ssrMode=false, ()) => {
  let cache = ApolloInMemoryCache.createInMemoryCache();
  let link = ApolloLinks.createHttpLink(~uri, ~fetch, ());

  createApolloClient({
    cache,
    link,
    ssrMode,
    defaultOptions: {
      "mutate": {
        "errorPolicy": "all",
      },
      "query": {
        "fetchPolicy": "network-only",
        "errorPolicy": "all",
      },
    },
  });
};

I use my own function since I build the client differently based on wether I I'm doing it for server o browser; that did the trick for me, but I always get a little nervous writing bindings as I don't know if there was a legit reason for the reason-apollo devs to not add this.

Can you see a safer cleaner way to do this? or is it safe to just do this until all libraries catch up to their Js counterparts?

[mbirkegaard]

Can you see a safer cleaner way to do this? or is it safe to just do this until all libraries catch up to their Js counterparts?

That's the right approach. You could make defaultOptions safer by using variants instead of strings and I don't think you need to use objects now records map directly to js objects, but for your use case this will probably do.

See also #122.

[RawToast]

@EnriqueVidal You don't need to use the promise for mutations, something like:

// Don't set variables yet
let (signInMutation, result, _) =
    ApolloHooksMutation.useMutation(SignInMutation.definition);

let signIn = (email, password) => {
      signInMutation(
        ~variables= ~variables=SignInMutation.makeVariables(~email, ~password, ()),
        (),
      )
      |> ignore;
    };

switch(result) {
  | ApolloHooksMutation.NoData => ...
  | Error(error) => ...
}

simple will be an ApolloHooksMutation.controlledVariantResult set to NotCalled until you fire the hook.

[EnriqueVidal]

@RawToast this is interesting although I having difficulty understanding this comment because of the conflicting documentation etc, in this example where is result coming from what wouldn't signIn just be unit?

My previous example worked once I figured how to set the error-policy but I'm always down for dropping promises and simplifying would you mind elaborating on this?

[RawToast]

Yeah, signIn will just be (string, string) => unit, you'd assign that to the login button.

What you're really interested in is result. Calling signIn will cause result to change from NoData to Loading, and then Data(something)

Something like this using the readme example:

[@react.component]
let make = () => {
  /* Both variant and records available */
  let ( _screamMutation, simple, _full ) = useMutation(ScreamMutation.definition);
  let scream = (_) => {
    screamMutation(~variables=ScreamMutation.makeVariables(~screamLevel=10, ()), ())
      |> Js.Promise.then_(result => {
          switch(result) {
            | Data(data) => ...
            | Error(error) => ...
            | NoData => ...
          }
          Js.Promise.resolve()
        })
      |> ignore
  }

  <div>
    <button onClick={scream}>
      {React.string("You kids get off my lawn!")}
    </button>
    <div>
    switch(simple) {
      |  ApolloHooksMutation.NoData => {React.null} 
      | NotCalled => {React.string("You haven't screamed")}
      | Loading(_) => {React.string("You are about to scream")}
      | Data(_) => {React.string("You screamed")}
      | Error(_) => {React.string("You can't scream")}
   }
  </div>
  </div>
}

[mbirkegaard]

That works if you don't need side effects. If you need to call things like
form.reset and form.notifyOnError you have to use the promise. (Well... some useEffect hooks could work as well, but that's not a good way to do it)

[mbirkegaard]

Closing due to no progress and having moved off topic.