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

Hooks & a Custom Reason Apollo PPX #192

Open
nikgraf opened this issue May 20, 2019 · 14 comments
Open

Hooks & a Custom Reason Apollo PPX #192

nikgraf opened this issue May 20, 2019 · 14 comments

Comments

@nikgraf
Copy link

nikgraf commented May 20, 2019

This is a proposal for the future of reason-apollo.

I just created a first version implementing a useQuery hook based on the new @apollo/react-hooks beta. You can find it here: https://github.com/nikgraf/reason-apollo-hooks/blob/master/src/Apollo.re
Here you can see how it's used: https://github.com/nikgraf/reason-apollo-hooks/blob/master/src/App.re

Personally I don't like the extra step of Apollo.CreateQuery. I was thinking we could create a fork of the graphql-ppx specifically for reason-apollo to achieve the following API:

module RestaurantsQuery = [%gql
  {|
    query {
      restaurants {
        id
        name
      }
    }
  |}
];

[@react.component]
let make = () => {
  let query = RestaurantsQuery.useQuery();

  switch (query.result) {
  | Loading => <div> {React.string("Loading...")} </div>
  | Error(error) => <div> {React.string(error##message)} </div>
  | Data(response) =>
    <ul>
      {response##restaurants
       ->Belt.Array.map(restaurant =>
           <li key={restaurant##id}> {React.string(restaurant##name)} </li>
         )
       ->React.array}
    </ul>
  };
};

The PPX would be named gql to avoid naming conflicts with graphQL PPX and the module would automatically expose useQuery

@peterpme
Copy link

YES 🔥 Great idea @nikgraf!

@nikgraf
Copy link
Author

nikgraf commented May 20, 2019

One thing I'm not sure of: could the PPX create different hooks based on the provided input e.g. for a mutation a useMutation function, while query should result in useQuery.

@peterpme
Copy link

Can you elaborate? I don't quite understand

@nikgraf
Copy link
Author

nikgraf commented May 20, 2019

@peterpme a mutation can have a optimisticResponse, while a query can have a fetchMore function. I guess that's the reason why also @apollo/react-hooks exposes a useQuery, useMutation & useSubscription.

@anmonteiro
Copy link

Not sure whether this matters for usage or not, but you may end up having to use query.Apollo.result to disambiguate the record type. Not sure what the impact of this in your design, but it's something I noticed right away.

@nikgraf
Copy link
Author

nikgraf commented May 21, 2019

@anmonteiro definitely matters from my perspective! Can you elaborate on why we need to access query.Apollo.result? Do you have a use-case in mind?

@sgrove
Copy link

sgrove commented May 21, 2019

It would be good to compare this to https://github.com/Astrocoders/reason-apollo-hooks/blob/master/README.md#usequery, and especially the "full" usage which we've found necessary in order to capture all the states via destructuring.

The gql ppx is nice, but I think it should probably be %apollo instead, since it's doing things specifically for the benefit of Apollo.

The hook-based approach looks great though!

And using an additional ppx to generate the other (confusing) boilerplate is a great idea as well. Generating specific functions/modules based on whether the operation is a query/mutation is definitely possible (fairly easy, in fact), one challenge might be the magic nature. I think you could take a lead from JST's Core playbook, where you always generate the same functions, but in the case of a query operation, the optimisticResponse function would just be a value of let optimisticResponse = 'Operation_is_a_query_not_mutation to give guidance about why this function wouldn't be generated.

@nikgraf
Copy link
Author

nikgraf commented May 21, 2019

Totally agree on the %apollo.

Exposing the full sounds good as well. Initially I thought about exposing different functions e.g. useQuery & useRefetchQuery to cover the most common cases. hmm instead of exposing full there could be a useRawQuery or useBaseQuery. What do you think?

What would be a good name for a function covering query, mutation and subscription?

@fakenickels
Copy link
Contributor

This is great, we were even thinking in adding support for the official lib (reasonml-community/reason-apollo-hooks#3) but here is definitely a better place to have it!

Also about the full query for different situations was a discussion we had way back then, I'm not sure about having different methods for each but I'm curious to see how that would look like

@anmonteiro
Copy link

@nikgraf I haven't tried to compile it, but based on past experience I think you would have to disambiguate record access in the following case:

type foo = { result: ... };

...

switch(query.Apollo.result) {
  ...
}

Granted, this is not a very common case, but still.

@hew
Copy link

hew commented May 24, 2019

I saw this, and thought I'd share it here: https://github.com/sync/hackerz/blob/master/app/GraphqlHooks.re

@jeddeloh
Copy link

Any status update on this effort? I think a dedicated Apollo PPX would bring a lot of value. There are a bunch of situations like fetchMore, subscribeToMore, optimisticResponse, Apollo’s local state management, and creating mock data where graphql_ppx feels insufficient or even incompatible. The approach of grapql_ppx to transform nullable values to options and unions to polymorphic variants seems appropriate to me when consuming data. In Apollo, though, we often have need to write data to the cache and the representation of that data in the cache is entirely different from what comes out of graphql_ppx’s parse.

So while I’m very excited about improved ergonomics, this hasn’t been my main pain point when working with Apollo. I feel like providing a Js.t type of what is in the cache or, even better, a full toJson function that is the reverse of parse would be the biggest advantage of a custom %apollo PPX.

At first I was fine with just going through the pain of writing JSON encoders and decoders combined with parse for everything in these situations, but then I would find that the parent query or something would get an addition of some data and we’d end up with a mismatch because now we’re only writing a subset of what’s in the cache and it fails. Worse, it seems that Apollo provides no mechanism for detecting when this situation happens. At best you get a console warning about a “Missing field …” if you’re in development.

@peterpme
Copy link

@jeddeloh its in a separate repo https://github.com/Astrocoders/reason-apollo-hooks

@sgrove
Copy link

sgrove commented Oct 23, 2019

We added code-gen support for Astrocoders' fork, you can see the current usage for both queries and mutations here: https://serve.onegraph.com/short/B8DR68 (open code exporter and choose Reason as the language)

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

No branches or pull requests

7 participants