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

Type inference in executeQuery/etc #91

Closed
Schmavery opened this issue Aug 1, 2019 · 5 comments
Closed

Type inference in executeQuery/etc #91

Schmavery opened this issue Aug 1, 2019 · 5 comments

Comments

@Schmavery
Copy link
Collaborator

I was just looking at making some graphql queries from a non-react (nodejs) project and realized that it seems like the type of the response from Client.executeQuery is a Wonka.Types.sourceT('a), so it seems like it exhibits the same issues as #80 and #67. Same with executeMutation and executeSubscription.

Does that sound right? I might be misreading how to use it.

@parkerziegler
Copy link
Contributor

Yep I think you're right on @Schmavery. I think we'll need to implement the same pattern of passing the full graphql_ppx module as we do on the components and hooks. Noticed the same thing while working on docs. Definitely happy to pick this up or, if you're keen on working on it, happy to do a quick review and release for it!

@Schmavery
Copy link
Collaborator Author

Schmavery commented Aug 2, 2019

If I have a chance this weekend I could try to get something out. Another thing I was wondering about is why the constructing the request and executing the query happen separately. Do you know if this request is expensive to create or something (I guess I can look into it when I get a chance).

Right now in my local project, I wrote a little wrapper as a workaround that does this:

module Graphql = {
  let executeQuery = gqlReq => {
    let req =
      ReasonUrql.Request.createRequest(
        ~query=gqlReq##query,
        ~variables=gqlReq##variables,
        (),
      );
    let parse = processResponse(gqlReq##parse);
    ReasonUrql.Client.executeQuery(~client, ~query=req, ())
    |> Wonka.map((. a) => parse(a));
};

// Gets called like this:
module MyQuery = [%graphql {| ... |}];
Graphql.executeQuery(MyQuery.make()) |> Wonka.forEach((. a) => Js.log(a));

Is there a reason I should be avoiding doing something like that? If we don't do it this way, we'd have to do something like having the ReasonUrql.Request.createRequest accept the object returned from MyQuery.make() and then return a tuple or something containing both the constructed request and the parse function.

@parkerziegler
Copy link
Contributor

Yeah I think the only reason I originally architected it that way was to match the urql API. Over in urql, executeQuery (and its associates) require you to pass in query as a GraphQLRequest object, which is basically just a packaged up object containing the query and variables of the request alongside a key that uniquely identifies the operation: https://github.com/FormidableLabs/urql/blob/master/src/types.ts#L25-L30 That key gets placed on there by createRequest, so this is why it happens separately from executing the query.

I think what you have is totally legit. I'd like to wrap it up nicely for consumers so that they get a fully type safe response, which means I'd expect them to just pass the graphql_ppx module and we can parse it as you're doing above inside of map. In fact, I think your impl. may be the one we use in here. I'll play around with this right now and see if I can arrive at something that works nicely. Will add you as a co-author on the commit as well!

@parkerziegler
Copy link
Contributor

Closed in #92 and #94. Will be released in 1.0.0-beta.3.

@parkerziegler
Copy link
Contributor

Published in 1.0.0-beta.3.

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

No branches or pull requests

2 participants