gqty icon indicating copy to clipboard operation
gqty copied to clipboard

Directives

Open redbar0n opened this issue 2 years ago • 21 comments

Would be awesome if GQty would support passing along and extracting (custom) directives.

Official description:

A directive can be attached to a field or fragment inclusion, and can affect execution of the query in any way the server desires. The core GraphQL specification includes exactly two directives, which must be supported by any spec-compliant GraphQL server implementation:

@include(if: Boolean) Only include this field in the result if the argument is true.
@skip(if: Boolean) Skip this field if the argument is true.

Directives can be useful to get out of situations where you otherwise would need to do string manipulation to add and remove fields in your query. Server implementations may also add experimental features by defining completely new directives.

https://graphql.org/learn/queries/#directives

Official Specification:

https://spec.graphql.org/

Highlights:

  • Directives can be used to describe additional information for types, fields, fragments and operations.

  • Directive order is significant

  • @include is a built-in directive

  • @skip is a built-in directive

Examples of custom directives actually in use today:

  • Apollo: https://www.apollographql.com/docs/apollo-server/schema/directives/
  • DGraph: https://dgraph.io/docs/graphql/queries/cascade
  • Neo4j: https://neo4j.com/docs/graphql-manual/current/type-definitions/cypher/
  • Hasura: https://hasura.io/blog/graphql-directives-you-should-know-about/

List of various directives, with good explanation.

Mostly, it seems like directives are only used on fields, for now. Top level, or indented.

Previous discussion:

This same issue in old gqless repo:

https://github.com/gqless/gqless/issues/210

There are various suggestions there for how the API could look.

redbar0n avatar Dec 19 '21 20:12 redbar0n

But I found none of the suggestions there compelling. I suggest this API instead:

// #4 - complex example, multiple directives
query MyCachedQuery($skipMember: Boolean!, )
    @cached(ttl: 120)
    @cascade(fields:["name", "city"]) {
  users {
    id
    name @skip
    city @include
    member @skip(if: $skipMember)
    url
    cdn: url @cdn(from: "urlA", to: "urlB")
  }
}

Can be represented as:


const result = query({
  '@cached': { ttl: 120 },
  '@cascade': { fields: ["name", "city"] }
}).users().map(
  u => ({
    id: u.id(),
    name: u.name('@skip'),
    city: u.city('@include'),
    member: u.member({ '@skip': { if: true } }),
    url: u.url(),
    cdn: u.url({ '@cdn': { from: "urlA", to: "urlB" } })
  })
)


Assuming that the query operation name MyCachedQuery is generated by GQty, for caching, and logging/debugging purposes. Also assuming that GQty will generate an alias in the graphql query (e.g. cdn:) upon the second access to the same field (here url()).

Most important presumption here is that all fields would always have to be functions (which again returns the value).
 So that the fields can take a potential directive as input into their function. Might be a breaking change. Note u.id().

Have included the '@' in the directive names. It will be a few more characters to type, but it will be more intuitive to read. If you've worked with GraphQL you would immediately know that the value represented a directive, simply from seeing the '@' key. It also allows avoiding to have an extra directive: {} wrapper or similar simply to make that clear. It could either be enforced, or merely supported by convention.

redbar0n avatar Dec 19 '21 21:12 redbar0n

@natew @thelinuxlich @jmsegrev @vicary what do you think?

redbar0n avatar Dec 19 '21 22:12 redbar0n

I like the idea but I think a more pressing issue is supporting aliases. Graphql-Zeus supports it, for example

thelinuxlich avatar Dec 19 '21 23:12 thelinuxlich

@redbar0n If you read the discussion in quoted issue https://github.com/gqless/gqless/issues/210, you may already know my opinion, that forcing every scalar into a function defeats the attractive design of gqty/gqless. This maybe resolvable if we figure out a way to do polymorphism for get and apply in proxies.

@thelinuxlich I am curious on how alias are needed. If it is only for multiple input for the same query, there is a current bug that incorrectly serialize the inputs. They are supposed to be managed internally, automagically.

vicary avatar Dec 20 '21 05:12 vicary

There are legit use cases: https://github.com/graphql/graphql-js/issues/522

thelinuxlich avatar Dec 20 '21 11:12 thelinuxlich

@redbar0n If you read the discussion in quoted issue gqless/gqless#210, you may already know my opinion, that forcing every scalar into a function defeats the attractive design of gqty/gqless.

Yeah, I read it. What about either naming all the functions getX() so that the usage is clearer? So that .url would become .getUrl() … Alternatively allow methods like urlFn() for fields that can also be accessed with .url if you want the simple scalar?

redbar0n avatar Dec 20 '21 15:12 redbar0n

@thelinuxlich This only shares the point that we need to explore the API design front. After some thoughts I think possible solutions are diverging from this issue, a new issue/discussion would allow easier follow-ups.

vicary avatar Dec 21 '21 05:12 vicary

@redbar0n I, for one, thinks the attractiveness comes from the similarity of a plain object. Any reserved names should be kept minimum and follows a predictable pattern, dynamic names and possible collisions with GraphQL schemas should be avoided.

The problem is focused on the fact that scalars, especially undefined does not allow proxy trapping.

You may try pushing it one level upwards and takes in a path parameter, but that creates ambiguity. I thought of $directiveFor but you may put whatever your favorite name to get the idea.

query.foo.$directiveFor("bar", "@myDirective", { directive: "arguments" });
// ambiguous
query.$directiveFor("foo.bar", "@myDirective", { directive: "arguments" });

vicary avatar Dec 21 '21 07:12 vicary

Also it was discussed in #proposals channel in Discord.

vkrol avatar Dec 22 '21 10:12 vkrol

The latest GraphQL spec allows directives to duplicate on the same scope, none of the proposals here are compatible.

thelinuxlich avatar Dec 22 '21 12:12 thelinuxlich

@thelinuxlich duplicate on the same scope? link?

redbar0n avatar Dec 22 '21 12:12 redbar0n

https://github.com/graphql/graphql-spec/issues/429

thelinuxlich avatar Dec 22 '21 13:12 thelinuxlich

Also it was discussed in #proposals channel in Discord.

I just made that channel public if you all prefer discussing this there, which should be better, and also suggest to the other people that already have commented and suggested stuff to read the stuff I wrote there a couple months ago with the challenges and design constraints

PabloSzx avatar Dec 22 '21 15:12 PabloSzx

The latest GraphQL spec allows directives to duplicate on the same scope, none of the proposals here are compatible.

@thelinuxlich My proposal is compatible. You can turn this:

member: u.member({ '@skip': { if: true } }),

Into this:

member: u.member({ '@skip': { if: true }, ‘@include’: { something: true } }),

redbar0n avatar Dec 22 '21 15:12 redbar0n

@PabloSzx could you summarise the relevant conclusions from your previous discussions?

Chatrooms are good for rapid back-and-forth to hash out suggestions then and there. But I am generally in favor of using github for such discussions, since github is: 1. indexed by search engines, 2. more async, 3. open to the public (while chatrooms are more exclusive / for insiders), 4. easier to contribute to (doesn't require an extra account etc.) 5. contained, i.e. one doesn't have to wade through endless chat-logs to find the relevant tidbits of information.

redbar0n avatar Jan 31 '22 15:01 redbar0n

I finally found the invite link to the Discord chat.. https://discord.com/invite/U967mp5qbQ

redbar0n avatar Jun 29 '22 13:06 redbar0n

Ok, I realise you are using a query object as a proxy object for detecting changes to it.

What about this suggestion? Trying to preserve the object-like API.

// #5 - complex GraphQL example, multiple directives, plus duplicate directives on same scope
query MyCachedQuery($skipMember: Boolean!, $includeMember: Boolean!)
    @cached(ttl: 120)
    @cascade(fields:["name", "city"]) {
  users {
    id
    name @skip
    city @include
    member @skip(if: $skipMember) @include(if: $includeMember)
    url
    cdn: url @cdn(from: "urlA", to: "urlB")
  }
}

Represented in GQty JS as:

const skipMember = false
const includeMember = true
query.directives = {
  '@cached': { ttl: 120 },
  '@cascade': { fields: ["name", "city"] },
  'users.name': [{ '@skip': true }],
  'users.city': [{ '@include': true }],
  'users.member': [{'@skip': skipMember, '@include': includeMember }],
  'users.url': [{ '@cdn': { from : "urlA", to: "urlB" } }],
}
const result = query.users.map(
  u => ({
    id: u.id,
    name: u.name,
    city: u.city,
    member: u.member,
    url: u.url,
    cdn: u.url,
  })
)

If the directives are all defined up front like that, the user could probably even avoid having to do the mapping in the last part, and simply do:

const result = query.users

You might ask: Doesn't the query.directives definition here look a lot like the original GraphQL query, so what have we gained? Well, we've contained it to only the directives (if those are needed), and preserved the simple and intuitive API when it is used. GQty will also still generate the queries at runtime, like it normally does, so there is greater flexibility of usage, compared to using GraphQL queries directly. It's only the fields with directives that will be "locked down" by the initial query.directives declaration.

What do you think @PabloSzx and @vicary ?

PS: @skip and @include are duplicate directives on same scope, and wouldn't necessarily make sense, but are here used for illustration purposes.

redbar0n avatar Jun 29 '22 15:06 redbar0n

@redbar0n The more I think of this, the more I feel this is a fundamental incompatibility between GraphQL and TypeScript.

But on the other hand I am very inclined to move this forward.

I can see that query.directives being a conservative way to not break the existing API, how about we go all the way and hide the whole definition behind util function? The directives definition is a variable only accessible internally.

Developing from the idea of the core resolved(), I can see something like this:

setDirectives(query, {
  '@cached': { ttl: 120 },
  '@cascade': { fields: ["name", "city"] },
  'users.name': [{ '@skip': true }],
  'users.city': [{ '@include': true }],
  'users.member': [{'@skip': skipMember, '@include': includeMember }],
  'users.url': [{ '@cdn': { from : "urlA", to: "urlB" } }],
});

vicary avatar Jul 03 '22 18:07 vicary

That’s interesting. It for sure is some differences when it comes to patterns and ease of representability.. but TS being turing-complete, I don’t see why it couldn’t in theory represent/output everything GraphQL does. But it does come down to convenience and ease of use, to be worth it, of course..

What would be the intention of hiding query.directives behind a util function? How would users be likely to misuse it if not? Deref the proxy object, or something?

Would query.setDirectives({…}) be an alternative?

redbar0n avatar Jul 03 '22 23:07 redbar0n

The idea is that every property from the query proxy is a possible naming collision for the top level Query, wrapping it in a util function allows us to move forward before a perfect solution comes up.

vicary avatar Jul 05 '22 05:07 vicary

Doesn’t GQty control the top level Query, as well as the query proxy object?

redbar0n avatar Jul 05 '22 16:07 redbar0n