Proposal: New user and client facing API for graphql-ppx

[jfrolich]

Because graphql-ppx is bringing about a lot of breaking changes I'd like to bring up the representation of GraphQL operations and fragments. This so we arrive at a solution that is the best representation for library authors free from historic / legacy considerations.

Currently, this is what a typical query looks like:

[%graphql
  {|
  query ($query: Int!) {
    argNamedQuery(query: $query)
    nestedObject {
      inner {
        inner {
          field
        }
      }
    }
  }
|}
];

This is compiled into:

module MyQuery = {
  module Raw = {
    type t_nestedObject_inner_inner = {field: string};
    type t_nestedObject_inner = {
      inner: Js.Nullable.t(t_nestedObject_inner_inner),
    };
    type t_nestedObject = {inner: Js.Nullable.t(t_nestedObject_inner)};
    type t = {
      argNamedQuery: int,
      nestedObject: t_nestedObject,
    };
    type t_variables = {query: int};
  };
  let query = "query MyQuery($query: Int!)  {\nargNamedQuery(query: $query)  \nnestedObject  {\ninner  {\ninner  {\nfield  \n}\n\n}\n\n}\n\n}\n";
  type t_nestedObject_inner_inner = {field: string};
  type t_nestedObject_inner = {inner: option(t_nestedObject_inner_inner)};
  type t_nestedObject = {inner: option(t_nestedObject_inner)};
  type t = {
    argNamedQuery: int,
    nestedObject: t_nestedObject,
  };
  type operation = t;
  type t_variables = {query: int};
  let parse: Raw.t => t =
    (value) => (
      {
        argNamedQuery: {
          let value = (value: Raw.t).argNamedQuery;
          value;
        },
        nestedObject: {
          let value = (value: Raw.t).nestedObject;
          (
            {
              inner: {
                let value = (value: Raw.t_nestedObject).inner;
                switch (Js.toOption(value)) {
                | Some(value) =>
                  Some(
                    {
                      inner: {
                        let value = (value: Raw.t_nestedObject_inner).inner;
                        switch (Js.toOption(value)) {
                        | Some(value) =>
                          Some(
                            {
                              field: {
                                let value =
                                  (value: Raw.t_nestedObject_inner_inner).
                                    field;
                                value;
                              },
                            }: t_nestedObject_inner_inner,
                          )
                        | None => None
                        };
                      },
                    }: t_nestedObject_inner,
                  )
                | None => None
                };
              },
            }: t_nestedObject
          );
        },
      }: t
    );
  let serialize: t => Raw.t =
    (value) => (
      {
        let nestedObject = {
          let value = (value: t).nestedObject;
          (
            {
              let inner = {
                let value = (value: t_nestedObject).inner;
                switch (value) {
                | Some(value) =>
                  Js.Nullable.return(
                    {
                      let inner = {
                        let value = (value: t_nestedObject_inner).inner;
                        switch (value) {
                        | Some(value) =>
                          Js.Nullable.return(
                            {
                              let field = {
                                let value =
                                  (value: t_nestedObject_inner_inner).field;
                                value;
                              };
                              {field: field};
                            }: Raw.t_nestedObject_inner_inner,
                          )
                        | None => Js.Nullable.null
                        };
                      };
                      {inner: inner};
                    }: Raw.t_nestedObject_inner,
                  )
                | None => Js.Nullable.null
                };
              };
              {inner: inner};
            }: Raw.t_nestedObject
          );
        }
        and argNamedQuery = {
          let value = (value: t).argNamedQuery;
          value;
        };
        {argNamedQuery, nestedObject};
      }: Raw.t
    );
  let serializeVariables: t_variables => Raw.t_variables =
    inp => {query: (a => a)((inp: t_variables).query)};
  let makeVariables = (~query, ()) =>
    serializeVariables({query: query}: t_variables);
  let definition = (parse, query, serialize);
};

As you can see it generates a number of functions (parse, serialize, makeVariables, serializeVariables), values (query, definition), and types (Raw.t, t + nested types and Raw.t_variables and t_variables + possible input objects).

There are also some utility functions that we might generate, such as for constructing input objects, or converting enums to strings (coming up).

Representation

There are a number of ways we can represent queries:

• Value representation
• Module representation
• Combined representation

The types cannot just be generated, because we to have explicit access to them as records sometimes need annotation. This means every record type needs to be accessible to the user.

Value representation

This representation would be the ideal representation because values are easily passed to functions and very composable.

We can wrap the values and functions in a containing value. The most ideal candidate would be a record. Where the respective values are accessible by their names.

Note that we need to ship a global module with graphql-ppx to be able to do this as we cannot generate this record for each query as they are nominally typed. But that is not a problem as we need to this for other reasons already. The alternative is a tuple (like we have now).

The problem with this representation is that the types are now dropped in the current module. And can clash with existing types, so we have to give them unique names for each query. Also because the Raw types and the parsed types are so similar it is better for type inference to wrap them in separate modules. This means that even if we inject the types in the current module, we still need to create a module to wrap the Raw types.

Jumping through all these hoops to avoid name clashes can be easily solved with the combined representation.

Module representation

In this case the contents are wrapped into a module. This is what we do currently sans the definition.

Combined representation

This is what we do currently. We have the module that wraps all values and functions, so we avoid name clashes, and then we have the definition tuple that includes the "guts" of the module it is (query, parse, serialize) and these are the values that most clients need to operate.

Current state (value in module)

The definition tuple is nice because we can pass this to any client and the client will get the information it needs. It can infer the most important types t and Raw.t from the function signatures of parse and serialize.

This works like this:

ApolloClient.use(MyQuery.definition)

Problems

When we are implementing a use function (most common use case) to execute the query, we pass in the definition, variables, and perhaps some other parameters.

Because definition doesn't contain any variable related values, these functions cannot infer if the variables that are passed are actually the variables of this query (it might be a completely different data structure).

To tackle this we have to put another value in the definition tuple, for instance makeVariables. Now it has access to the t_variables and Raw.t_variables.

The problem here is that we need to keep adding values into this tuple (a breaking change for libraries), in order to make it possible for libraries to have access to certain types or values.

Another problem is around dead code elimination. When we bundle these values together in a data structure it potentially makes it harder to do dead code elimination or tree-shaking operations.

Alternatives

An alternative is not to have the definition tuple, but just the module. And find ways for libraries to consume the module and it contents. There are two ways to do this.

First-Class Modules

First-class modules allow you to pass a module just like a value. In case of a use function it would look like this:

  ApolloClient.use((module MyQuery))

This has been discussed at length in both the Apollo and Urql clients, and was actually implemented in reason-apollo-hooks as an experiment. General consensus after this discussion seems to be that:

• This is leaking advanced language concepts to the end user, we'd like the API to be simple to understand

• The syntax is slightly awkward with the extra braces and the need to add module

Functors

Functors was the way most libraries used graphql-ppx previously (before the definition).

The main reason definition was added was that applying a functor to each query module was adding a lot of boilerplate. However eventual user-facing API was quite nice:

MyQuery.use()

Also functors are the most flexible solution (together with FCMs). They can access all types and all values within the module. And most importantly they don't break if we add new functionalities into GraphQL ppx because modules are structurally typed.

Optimal solution

First-class modules look like the best solution from a technical solution, it however is not great in terms of simplicity of the API, and it doesn't seem to have consensus from GraphQL library authors.

Value in module has the problem that their use is constraint in what we pass into the value. If we change the structure of the value, all GraphQL clients break, which can result in bad user experiences when people are using GraphQL in Reason. Arguably passing MyQuery.definition is also not the most beautiful API but opinions differ.

Functors provide a beautiful API for the user, they are also the most flexible solution and and they are less prone to cause breaking changes. But they have the downside of the added boilerplate of applying the functor for every Operation/Fragment.

Can we let the PPX do the heavy lifting?

The code necessary for applying the functor is the following:

module MyQuery' = GraphQLClient.MakeQuery(MyQuery);

Note we still need the MyQuery around because as we noted before, we need to have the types around to use in our implementation. The number of types are variable (dependent on the structure of the query), and not fixed so there is no fixed module type. This means the functor cannot extend the module.

To extend the query properly (so we have a single module that contains the original query and the extension) we have to add even more boilerplate:

module MyQuery' = {
  include MyQuery;
  include GraphQLClient.MakeQuery(MyQuery);
};

The result is that we have a single Query module that is incredibly extendible. For instance clients can add:

Query.use, Query.execute, etc. etc.

Prior art is reason-relay, this client doesn't use graphql-ppx and relies on a relay compiler plugin to generate modules types and values. It also has custom build GraphQL ppx, that is deeply integrated with the client. Users seem to love the simplicity and ease of use of the Query.use (and other) API.

It seems very repetitive and not worth to let the user do this for every query. Especially since in a single project 99% of the case the user will use a single GraphQL client. Also just like FCM this is exposing advanced language features to the user.

So how about letting the PPX do the heavy lifting? We can hide the functor application from the user. And extend every operation or fragment automatically as part of the PPX.

The functor application is now an implementation detail. The most important result is the capability to extend modules that graphql-ppx generates, but without adding custom code to the PPX for each GraphQL client (that would be impractical).

It would expose the advanced language features to the client author (that's fine).

The only downside to this is that we need to have some global configuration. We can configure graphql-ppx currently in three ways.

• Adding configuration per query
• Adding configuration as ppx arguments
• Configuration in the bsconfig.json

We should be able to expose the functors in these three ways as well. The easiest way would be to use the bsconfig.json configuration.

{
  "graphql": {
    /* for some clients we need add this configuration for the ppx to play well
       with the client */
    "client": "apollo",
    /* extra configuration. */
    "extendQuery": "Apollo.ExtendQuery",
    "extendMutation": "Apollo.ExtendMutation",
    "extendSubscription": "Apollo.ExtendSubscription",
    "extendFragment": "Apollo.ExtendFragment"
  }
}

This is an extra step of configuration. But it's pretty easy to do, and the user already has to change bsconfig.json to set up graphql-ppx and the GraphQL client that they use.

I think this strikes a balance between most flexibility for GraphQL clients and the nicest API for users, and being the most future proof going forward.

I am interested in your opinion about this as we cannot proceed without consensus as it will affect all GraphQL clients in the Reason ecosystem.

PS: The cool thing is that we can now extend an operation to add definition. If some clients prefer to keep using definition, they can simply ship with a simple functor that composes the values they need in a definition. This definition is now defined by the client. And will never break. If the client needs more values in the definition they can simply update the functor (and the ppx doesn't need updating). This will allow new versions of the client to have a new definition and old versions of the library will keep working.

[zth]

Thanks for writing this up @jfrolich ! One thing that struck me when reading this is that I worry that graphql_ppx tries to do a bit too much, and cater to too many use cases. I really like the idea of graphql_ppx being a building block for other clients, but I'm not sold on graphql_ppx bending over backwards to provide an API that's flexible enough to implement every GraphQL client on top of it, without their own abstractions.

I'm obviously colored by my experience building reason-relay, but I'd personally rather see graphql_ppx just handle the building blocks (as in "here's how you generate types and serializers/helpers for this particular GraphQL operation/fragment"), and then let each client implement the API that makes sense for them using what graphql_ppx gives them. So Apollo could have it's own PPX that uses the building blocks from graphql_ppx to generate what's needed, but then tie that together and present that to the end user through an API that make sense for Apollo (Query.use for instance). And the same goes for urql, and so on.

My 5 cents! Again, thanks for writing this up.

[jfrolich]

I really like the idea of graphql_ppx being a building block for other clients, but I'm not sold on graphql_ppx bending over backwards to provide an API that's flexible enough to implement every GraphQL client on top of it, without their own abstractions.

This proposal is mainly trying to tackle the problems with having a definition tuple. To recap, the main problems are that its

• Not the most elegant API to pass this tuple around to clients
• All clients break if we decide to add an element to the tuple in future versions of graphql-ppx
• Not the most flexible API (the client can only use the values and functions that are in the tuple)

The previous API clients used (definition was only implemented recently) was using a functor to extend the Query module. But definition was introduced because a functor added too much boilerplate for the user. Now that we need the record types to be accessible, the previous functor application adds even more boilerplate.

Using the ppx to apply this functor solves this. I wouldn't really call this bending over backwards, because it's a pretty simple addition to the ppx.

I'm obviously colored by my experience building reason-relay, but I'd personally rather see graphql_ppx just handle the building blocks (as in "here's how you generate types and serializers/helpers for this particular GraphQL operation/fragment"), and then let each client implement the API that makes sense for them using what graphql_ppx gives them. So Apollo could have it's own PPX that uses the building blocks from graphql_ppx to generate what's needed, but then tie that together and present that to the end user through an API that make sense for Apollo (Query.use for instance). And the same goes for urql, and so on.

I might be misunderstanding how you'd like a separate ppx for each client to solve this problem. You can extend modules easily in ReasonML with functors, where you don't have a need to use a ppx (which is a pretty heavy handed approach).

Let's talk about about a possible implementation with a ppx. graphql-ppx would add a module annotation to the query module (say [@graphql_ppx]). So that clients ppx's know which modules are queries and can rewrite the module and add anything they want.

This approach basically arrives at the same end result. But there are several downsides to this:

• Now each client needs to maintain a ppx, so if I'd like to fork a client, I need to also maintain a ppx with all the build tooling involved for each platform (believe me it's pretty tricky to maintain ppx's especially if you need windows support).

• ppx's are generally more brittle. A functor only breaks if we change the module interface that the functor touches. A ppx that rewrites queries can break at any point when we change the implementation (we don't know what each client is doing in the module AST).

• the ppx's need to be provided in the right order (can cause confusing problems)

A small benefit of extension by ppx is that we don't have the original query module around (Query') anymore.

But you might have other ideas in how this might work?

[anmonteiro]

I'm still digesting the full proposal, but here are some more thoughts:

• A functor approach isn't such a nice API for the user either, because it leaks implementation details:

  • a practical example is your proposal for configuration. Folks who use Reason have finally freed themselves of the JavaScript shackles, and are using a nice and strong typed language. The last thing they need is to use a library that introduces yet another stringly typed API (your proposal for config in bsconfig.json)

• One of your arguments is that adding another element to the tuple breaks clients. That's fair. But the functor approach introduces syntax that's too heavy, with no clear benefit (at least I haven't understood it yet).

  • What about a record? Records have clear named fields, can be polymorphic, and extended without breakage in this particular scenario.

Side note: I agree that other clients shouldn't add a PPX on top of this one. BuckleScript's PPX semantics are already terrible as they are, and we don't need something to make it even worse (PPX composition, etc.)

[jfrolich]

Another approach could be that graphql-ppx would provide a ppx toolkit for clients, so that they can use this to build their own ppx's. There are some advantages:

• There is more flexibility, the client ppx can do some inspection of the GraphQL AST and use that to customize what they inject in the module

We still have these problems:

• Each GraphQL client or tool that uses GraphQL needs to build their own ppx
• Now we have different ppx-extension points for each client
  • This causes problems with editor integration (they need to support all of these)
  • We also lose the ability to share fragments/queries between different clients. This might be an esoteric use-case, but we use this at my company for sharing fragments between gatsby and apollo-client and it's pretty great!
• If bugs are fixed in graphql-ppx it will only be fixed when the client ppx is published, so updates are now a two-step process. And it also moves burden to clients that they need to keep the ppx up to date. Most of the logic is in the graphql-ppx, and there are many intricacies, so I would foresee quite a number of small fixes that would happen in graphql-ppx, now all clients that might just add a simple use function need to update frequently as well.

Actually I think this is certainly quite powerful, and there might be clients that need to do things that are not possible in a functor. So there is a case to be made to offer this functionality for libraries where the benefits outweigh the costs (reason-relay?).

But I also think that most libraries don't need this kind of power, and the cost doesn't stack up to the benefits for them.

[jfrolich]

• a practical example is your proposal for configuration. Folks who use Reason have finally freed themselves of the JavaScript shackles, and are using a nice and strong typed language. The last thing they need is to use a library that introduces yet another stringly typed API (your proposal for config in bsconfig.json)

Yes I agree configuration is not great. But I'd like to add that configuration is necessary anyway because for instance for Apollo (most popular GraphQL client) we need to pass configuration to the ppx for graphql-ppx to work well with it (to account for adding typename automatically to each object, and to add the template string literal to the query).

What about a record? Records have clear named fields, can be polymorphic, and extended without breakage in this particular scenario.

Yes I think that is superior to a tuple (also mentioned in the body text). (I do have mentioned some other downsides to passing a definition around in general.) If we ship a definition it being a record is definitely better than a tuple. With the small downside that now the ppx also needs to be a BuckleScript code dependency so the project has access to a GraphQL_PPX module that contains the record type. Actually having that module is already necessary for a deepMerge function that we need for serialization.

[jfrolich]

A functor approach isn't such a nice API for the user either, because it leaks implementation details

The functor is only for implementing clients. The user will not even know that the module extension happened because of a functor. They will just have a use function added to a query or mutation.

[anmonteiro]

Yes I agree configuration is not great.

Configuration is not the problem I'm describing. Configuration that will result in a functor application based on the name of the module you write as a string is the problem I have.

Having a small runtime code that's shipped as part of a PPX is no big deal -- PPXes do it all the time and users don't necessarily need to declare a dependency on it -- in this case Apollo would, so you'd get it as a transitive dependency.

The user will not even know that the module extension happened because of a functor.

Sounds like the user would have to write "extendQuery": "Apollo.ExtendQuery",. This is a pretty leaky abstraction in the PPX code and why I'm not a big fan of the proposal.

[jfrolich]

@anmonteiro Yes you have to provide the module name as a string. But that is the best that I could come up with, any ideas on how we can do this differently?

[jsiebern]

@jfrolich I don't like the configuration bit either. I'm trying to come up with a way to determine the client automatically without creating coupling.

A possibility could be a convention. Let's say graphql-ppx uses a dynamic extension point. Example:

• [%graphql_apollo ...]
• [%graphql_urql ...]

If used with this postfix, it automatically extends the modules by way of using the postfix:

• Apollo.ExtendQuery
• Urql.ExtendQuery

Where the names like ExtendQuery are the convention client libs need to adhere to if they wish to use the functor solution.

I'm sure this has a lot of drawbacks as well that I'm not thinking of right now (it being 4:30 am and all), but I usually like convention over configuration.

[anmonteiro]

It's what I've been trying to suggest: by not lifting things to the module level, or at least by using first class modules, you can just pass values to the library from within the application (and therefore going by the regular type checking pipeline).

It sounds like you're gonna have to ship some kind of "runtime" anyway, otherwise clients won't see your module type.

I'd love to echo what some others have said too:

• GraphQL PPX is in a great position to provide building blocks for clients to build upon
  • that doesn't mean it has to cater to every need. Function application and composition is pretty cheap in OCaml, and I don't see a problem in users of a client library having to pass the module generated by the PPX to the client. This actually looks pretty neat:

  ApolloClient.use((module MyQuery))

You can also go a step further, and have GraphQL PPX generate something like:

module MyQuery = [%graphql ... ]
// generates:
let my_query = (module MyQuery: GraphqlPPX.QueryType)

// usage:
ApolloClient.use(my_query)

[jfrolich]

You can also go a step further, and have GraphQL PPX generate something like:

module MyQuery = [%graphql ... ]
// generates:
let my_query = (module MyQuery: GraphqlPPX.QueryType)

// usage:
ApolloClient.use(my_query)

Very interesting approach. That is actually quite neat!

(With the sidenote that probably we also probably need to keep MyModule to access types for the user.)

[jsiebern]

You have written something about

ApolloClient.use((module MyQuery))

having been discussed and disliked before. I agree with @anmonteiro though, it's the easiest to write in my opinion (easier than the definition way and esp. easier than manually applying functors).

Generally I'm very much for keeping the module structure around. It feels more natural for me coming from a web based background.

[jfrolich]

@jfrolich I don't like the configuration bit either. I'm trying to come up with a way to determine the client automatically without creating coupling.

A possibility could be a convention. Let's say graphql-ppx uses a dynamic extension point. Example:

• [%graphql_apollo ...]
• [%graphql_urql ...]

If used with this postfix, it automatically extends the modules by way of using the postfix:

• Apollo.ExtendQuery
• Urql.ExtendQuery

Where the names like ExtendQuery are the convention client libs need to adhere to if they wish to use the functor solution.

I'm sure this has a lot of drawbacks as well that I'm not thinking of right now (it being 4:30 am and all), but I usually like convention over configuration.

I do like this approach. But I would probably like [%graphql.apollo ...] a bit more. However, I realize now that this ties fragments and queries to a client (and it's pretty awesome to be able to reuse them between clients).

Another option is to just have one configuration (client: "Apollo") and then assume that ExtendQuery, ExtendFragment etc. are submodules of Apollo. But this is not great because fragments might not need extension for instance.

[anmonteiro]

Here's another proposal that I just proposed in Discord to @jsiebern, that I like even more:

Assuming for a second that module type Query is the thing that has Raw.t and the likes:

module type Query_inner = {type t;};

module type Query = {
  include Query_inner;

  let this: (module Query_inner);
};

You can have this sort of recursive module that includes itself as a first class module for clients to use. The API that this allows becomes really natural:

module MyQuery = [%graphql ...];

ApolloClient.use(MyQuery.this)

MyQuery is still what GraphQL PPX has today. It includes one more binding, this (or whatever name you wanna call it), that is a let binding for a first class module of the module MyQuery itself!

[jfrolich]

Love @anmonteiro proposal.

I actually was thinking over the feedback of @zth, about the fact that some clients need to adjust the behavior of this ppx slightly.

What if we have a configuration option for clients that need more customization. Instead of having an extra ppx that need to be maintained by the client that in some way is dependent on this ppx. What if we would have a preset option for clients that need this?

So for instance we have preset: "apollo" in the bsconfig.json file. This makes sure we do some extra stuff inside of the ppx to make the ppx play well with apollo. And possibly if we would like to support relay in the future have a preset: "relay".

This has a few advantages:

• Most clients don't need any configuration
• Some complex clients only need one option: preset
• We don't need multiple ppx'es
• We can set up the testing infrastructure in this ppx so all resolved bugs etc. are always part of a single ppx release

Let me know what you think!

[jsiebern]

This makes sure we do some extra stuff inside of the ppx to make the ppx play well with apollo

I like the idea in general and see your points on the advantages. What strikes me though is the coupling of these packages. I believe your first instinct to have been the best: Provide points of extension / variation that are safe from breaking when the PPX updates.

I work with a lot of version coupled projects at work, this just gets nasty really fast. Providing some switches (like __typename for apollo) is fine I believe, but implementation details should reside with the client lib in my opinion.

[anmonteiro]

I agree with @jsiebern. Another option which I've seen other PPXs do is to release a library of GraphQL PPX tools. GraphQL PPX can itself depend on it and provide the primitive building blocks for interacting with GraphQL in OCaml / Reason, and this library could be used by other clients that wish to augment GraphQL PPX's functionality.

[jfrolich]

This makes sure we do some extra stuff inside of the ppx to make the ppx play well with apollo

I like the idea in general and see your points on the advantages. What strikes me though is the coupling of these packages. I believe your first instinct to have been the best: Provide points of extension / variation that are safe from breaking when the PPX updates.

I work with a lot of version coupled projects at work, this just gets nasty really fast. Providing some switches (like __typename for apollo) is fine I believe, but implementation details should reside with the client lib in my opinion.

Ok yes that is indeed a valid concern.

I agree with @jsiebern. Another option which I've seen other PPXs do is to release a library of GraphQL PPX tools. GraphQL PPX can itself depend on it and provide the primitive building blocks for interacting with GraphQL in OCaml / Reason, and this library could be used by other clients that wish to augment GraphQL PPX's functionality.

@zth Would this make sense for reason-relay?

[jfrolich]

Ok naming wise, what do we think of Query.self?

[jfrolich]

See #138 for the implementation

[wokalski]

Why isn't Query' just inside Query?

[jfrolich]

Why isn't Query' just inside Query?

Yes, that's probably better though in the end it's an implementation detail. It will be better especially when we generate a module type so we can actually hide the original module with this approach.

[wokalski]

Btw it's a detail but I'd encourage you to think about the naming convention. If users name modules after the thing that's about to be fetched for instance:

User

Then probably nicer name for this or self would be query. I might be getting to inspired by relay at this point though.

[jfrolich]

The only thing is that query is already taken as the string with the "query". And we also have to think about other operations (mutation, subscription) or fragments.

[anmonteiro]

I'm fine with this and self. operation could be the other option.

[jsiebern]

I'd prefer self over this as this might be confusing to people coming fresh from a JS background. operation would give more context of course. I'd be on board with both.

I agree with @jfrolich, that query might be confusing as it could be mistaken for only being for the query operation.

[jfrolich]

I'm fine with this and self. operation could be the other option.

Operation doesn’t capture fragments 😅

[jfrolich]

I think “this” is better when it refers to the containing module if the code is in the module itself, and “self” is better when referring to a module when outside that module linguistically.

[anmonteiro]

definition covers both operations and fragments.

It's also the terminology used by GraphQL, and OGS:
https://github.com/andreas/ocaml-graphql-server/blob/940e86f9ff1a017be2ff64b3a35c71804d9a4729/graphql_parser/src/ast.ml#L70

[jfrolich]

definition covers both operations and fragments.

It's also the terminology used by GraphQL, and OGS:
https://github.com/andreas/ocaml-graphql-server/blob/940e86f9ff1a017be2ff64b3a35c71804d9a4729/graphql_parser/src/ast.ml#L70

Yes, that is also what we use in the graphql-ppx codebase. But I still think self is a bit better because we are talking not just about the definition (operation or fragment), but about passing the whole reason module.

[jfrolich]

I ran into an issue while implementing this:

In order for clients to support this we need to have this at implementation site (generated by graphql-ppx):

  let self:
    module GraphQL_PPX.Mutation with
      type Raw.t_variables = MyQuery'.Raw.t_variables and
      type t = MyQuery'.t and
      type Raw.t = MyQuery'.Raw.t =
    (module MyQuery');

When you are building a client that accepts the packed module you need to have the exact same annotation for unpacking:

let useMutation:
  type t raw_t raw_t_variables.
    (
      (module GraphQL_PPX.Mutation with
         type t = t and
         type Raw.t = raw_t and
         type Raw.t_variables = raw_t_variables)
    ) => ...

This is a bit problematic because if we add something in the definition code that is generated by the ppx, now the consuming signatures need to be updated as well. So this means we have the same problem as the tuple definition.

Interestingly if we go for the functor approach, the only annotation is needed for the consumer. So we don't even have to annotate what is exposed. This means it will always be backwards compatible unless we break the type signature.

It is stringly typed. But so are some other parts of the API (like decoders).

Thoughts?

[jfrolich]

Also interestingly, if we don't pre-pack into a value but just use this API:

ApolloClient.use((module MyQuery))

We do not have this problem because in that use case the type is inferred and doesn't need to be annotated.

[jfrolich]

It looks like packing syntax will improve. Actually this is already possible in Reason:

ApolloClient.use(module MyQuery) (not the awkward extra braces)

But this needs to become the default in refmt, right now it adds the braces.

The following will be the potential new syntax:

ApolloClient.use(module(MyQuery))