remove type annotations

tcoopman commented 4 years ago

This makes reason-apollo-hooks work with https://github.com/apollographql/reason-apollo/pull/228

fakenickels commented 4 years ago

Nice, we can merge here as soon as they ship theirs

yawaramin commented 4 years ago

This can be merged without waiting for that one. It would help people who are temporarily using a branch with the fix, because they wouldn't have to do the same with reason-apollo-hooks.

kutyel commented 4 years ago

The fixed version of react-apollo is now released under 0.19.0, please merge this as well so that we can benefit from the change 😄

fakenickels commented 4 years ago

Thanks @kutyel! Will be shipping soon to npm

nirvdrum commented 4 years ago

I don't think this change should ship. It doesn't appear to fix compatibility with reason-apollo 0.19.0. The problem is the ApolloClient.queryObj type definition changed between reason-apollo 0.18.0 and 0.19.0. In 0.18.0 is was a Js.t and in 0.19.0 it's a record. As far as I can tell, this PR removed an immediate compilation failure by allowing toQueryObj to return an object of the old type. But, as a result, toQueryObj no longer returns an ApolloClient.queryObj instance.

To help illustrate the problem, this is a compilation error I'm receiving when using reason-apollo-hooks master (after this PR was merged):

  16 ┆ ApolloHooks.useMutation(
  17 ┆   ~refetchQueries=
  18 ┆     _ => [|ApolloHooks.toQueryObj(GraphQL.Queries.Profiles.make())|],
  19 ┆   GraphQL.Mutations.CreateProfile.definition,
  20 ┆ );

  This has type:
    {. "query": ReasonApolloTypes.queryString, "variables": Js.Json.t}
  But somewhere wanted:
    ApolloClient.queryObj

I don't see a way to maintain compatibility with reason-apollo < 0.19.0 and >= 0.19.0, so we probably should just adopt the new record type and make reason-apollo 0.19.0 the new minimum version. I've been running a branch with my proposed fix on a project with reason-apollo master (now 0.19.0) for the past month and it's been working fine for me. (Please ignore the part where I removed Husky... I couldn't figure out what the commit message rules were and eventually gave up, but that can be backed out easily enough).

I'm happy to open up a PR with what I think the fix should be, but I'm worried that I've missed the mark somewhere. As far as I can tell, the "persons" example in the reason-apollo-hooks repo shouldn't even work with this PR + reason-apollo 0.19.0 because the example makes use of refetch queries. If I'm mistaken, please let me know.

kutyel commented 4 years ago

It's true, if now I depend on the unreleased version of master, I get similar errors! 😟

reasonml-community / reason-apollo-hooks

remove type annotations #108