Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Begin migration to @reasonml-community/graphql-ppx, starting with client.execute* methods. #221

Merged
merged 1 commit into from
Oct 14, 2020

Conversation

parkerziegler
Copy link
Contributor

This PR beings to address #219. Thanks to @jfrolich and @jeddeloh for their ideas and help here 🙌

Summary

Now that the Reason community is beginning to coalesce around a standard (from what I can tell) for a GraphQL PPX rewriter, we want to ensure reason-urql is compatible with that standard. Because this process will lead to breaking changes, I'm planning to roll it into a v3 release alongside fixes for #220 and #206.

Major Changes

The major breaking changes in this particular PR involve those related to working with the graphql-ppx API. Fortunately the changes aren't that wild. Previously we'd do:

module LikeDog = [%graphql
  {|
  mutation likeDog($key: ID!) {
    likeDog(key: $key) {
      name
      breed
      likes
    }
  }
|}
];

let request = LikeDog.make(~key="12345", ());

Client.executeMutation(~request, ())
  |> Wonka.subscribe((. result) => {
    /* Handle result, error, etc. */
  });

The new API uses a feature of OCaml called first-class modules to allows us to use a graphql-ppx module almost like a regular variable. The new API looks like this:

module LikeDog = [%graphql
  {|
  mutation likeDog($key: ID!) {
    likeDog(key: $key) {
      name
      breed
      likes
    }
  }
|}
];

Client.executeMutation(~request=(module LikeDog), ~variables=LikeDog.makeVariables(~key="12345"), ())
  |> Wonka.subscribe((. result) => {
    /* Handle result, error, etc. */
  });

I like this quite a bit more! It would allow us to match urql naming a bit more closely as well by changing ~request to ~query if we like.

Drawbacks

I'm not sure if you'd necessarily call this a drawback, but this change does make the source less approachable for newcomers. Supporting graphql-ppx in this way requires the use of GADTs. Succinctly, GADTs allows us to specify the type of the output of a function based on the type of the input – a great way to handle polymorphism! However, they do take a bit of work to understand. Similarly, first-class modules require some ramping up on to get comfortable with (though I do think they are more approachable than functors)!

Next Steps

After this PR gets merged, we can begin moving the hooks APIs over to the use of first-class modules. When that is complete, we'll migrate urql to a peerDependency and complete the last steps of v3!

@jfrolich
Copy link
Contributor

Nice!

With the extendQuery configuration in bsconfig.json you could simplify to:

module LikeDog = [%graphql
  {|
  mutation likeDog($key: ID!) {
    likeDog(key: $key) {
      name
      breed
      likes
    }
  }
|}
];

LikeDog.execute({key: "12345"})
|> Wonka.subscribe((. result) => {
    /* Handle result, error, etc. */
  });

// or even
LikeDog.subscribe({key: "12345"}, (. result) => {
  /* Handle result, error, etc. */
})

Using applied functors under the hood in the generated code but the user doesn't have to know :)

@jfrolich
Copy link
Contributor

I don't understand where GATD's are used?

@BlueHotDog
Copy link

SOOOO excited for this landing!!!

@parkerziegler
Copy link
Contributor Author

@jfrolich GADTs are used here: https://github.com/FormidableLabs/reason-urql/blob/26fad3868de7e89d993fe4f28754514788a8e53e/src/Client.re#L234 It's opaque to the user, so the drawback I discuss is more for contributors – for users the experience is better from my perspective.

Since we need to correlate the type of Operation.t to the return type of the data after it's been parsed by clientResponseToReason, we need GADTs to help handle that polymorphism. I took this approach from reason-apollo-client, which does a very similar trick (and is a nice solution for handling polymorphism introduced by every GraphQL query / mutation having a different shape).

I think the LikeDog.execute API is a more controversial choice for us, and one I'll want to discuss with the urql core team. A major motivation we have for this project is to keep the APIs as similar as possible while also taking advantage of the benefits of Reason to improve the ergonomics. But the farther we get away from the core urql API the harder it is for us to keep releases synced and ensure users feel confident switching between the two. Hope that makes sense!

@jfrolich
Copy link
Contributor

Hey, I think the name for that is locally abstract type. I wrote the early iteration of that heavily inspired by @fakenickels.

Totally understand the intention of staying close to the JS syntax. Just wanted to make sure you didn't miss this option.

@parkerziegler
Copy link
Contributor Author

Ah gotcha, thanks @jfrolich! Definitely will also keep the API extensions in mind as v3 develops here – it may be that we move towards that API-style a bit more, or provide an opt-in for users.

@parkerziegler parkerziegler merged commit bf7a6a3 into v3 Oct 14, 2020
@parkerziegler parkerziegler deleted the task/migrate-client-methods-to-graphql-ppx branch October 14, 2020 14:53
parkerziegler added a commit that referenced this pull request Nov 12, 2020
parkerziegler added a commit that referenced this pull request Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants