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

Directives #210

Open
natew opened this issue Apr 24, 2021 · 9 comments
Open

Directives #210

natew opened this issue Apr 24, 2021 · 9 comments

Comments

@natew
Copy link
Contributor

natew commented Apr 24, 2021

Support for directives would unlock stuff like caching in Hasura.

Not sure how best to enable them in a gqless style. Let's take a few examples:

// 1
query MyCachedQuery @cached(ttl: 120) {
  users {
    id
    name
  }
}

// 2
query {
  user(id: 1) {
    url
    cdn: url @cdn(from: "https://newapi.getpop.org", to: "https://nextapi.getpop.org")
  }
}

// 3
query ($skipTitle: Boolean!) {
  queryPost {
    id
    title @skip(if: $skipTitle)
    text
  }
}

The top level one is easy enough:

// 1.a
const result = query.directive('cached', { ttl: 120 }).users().map(u => ({ id: u.id, name: u.name }))
// 1.b
const result = query.directive({ cached: { ttl: 120 } }).users().map(u => ({ id: u.id, name: u.name }))

The leaf values are harder, they go could any number of ways...

// 2.a
const result = query.user({ id: 1 }).directiveOn({ url: { cdn: { from: '', to: '' } } }).url
// 2.b
const result = directive(query.user({ id: 1 }), { url: { cdn: { from: '', to: '' } } }).url
// 3 ??

I think I like a combination of 1.b and 2.b, but open to something more consistent.

Just wanted to open ticket to start discussion.

@thelinuxlich
Copy link

Cool!

@jmsegrev
Copy link

Hey @natew has there been any work on this, I'm actually in need of this feature. I need it to consume Dgraph graphql API, they have a cascade directive that is required for particular queries.

@vicary
Copy link

vicary commented Jun 30, 2021

Joining the brainstorming here.

Idea 1

I am pulling the same trick as I did in other places, resorting to a leading $ for meta-level to avoid naming collisions.

This one is easier to implement, but more cumbersome to use.

  1. query.$directives.cached({ ttl: 20 })
  2. In gqless you don't, and shouldn't, have direct control over alias. It should be serialized, compared and aliased automatically in the resulting query when multiple inputs are invoked in the same render. query.url; query.url.$directives.cdn({ from: ..., to: ... })
  3. @include and @skip is always controversial to me because developers can easily do a conditional query when you are the one providing the variables yourselves, maybe benefitial for persisted queries but that's unofficial. But if we have to, query.queryPost.title.$directives.skip({ if: true }) should be enough for the query proxy to hoist that boolean as a pseudo variable.

Idea 2

This one looks a bit more syntactically pleasing and aligns with the intuition when you think of the proxy as normal javascript objects. i.e. Actual value comes out from the last dot.

But this one requires polymorphism in the handler.apply proxy trap to support both .foo and .foo(...) at the same time.

  1. query({ "@cached": { ttl: 20 } })
  2. query.url; query.url({ "@cdn": { from:..., to:... } })
  3. query.queryPost.title({ "@skip": { if: true } })

Idea 3

Anything starts with @ can be treated as a directive in the query proxy trap.

For example,

  1. query["@cached"]({ ttl: 20 })
  2. query.url["@cdn"]({ from:..., to:... }) but this way it lacks an easy way to collect inputs for auto-alias.
  3. query.queryPost.title["@skip"]({ if: true })

@jmsegrev
Copy link

jmsegrev commented Jun 30, 2021

@vicary I like the first and third ideas, the second one will complicate things, since then query.url type is a string | function, when using it the developer will have to cast to the actual type in some cases, very inconvenient. I would go with the first idea, since that pattern of prefixing with $ would work for other functionality and $directives is a more intuitive API.

The only question would be how to handle multiple directives at the same level?

Some examples:

  1. query.$directives({ cached: { ttl: 20 }, cascade: { fields: ["reputation", "name"] } })
  2. query.$directives([ { cached: { ttl: 20 }, 'cascade' ])

The second one makes more cense since we could use directives without arguments, very clearly.

@vicary
Copy link

vicary commented Jun 30, 2021

To add multiple directives, it works the same way you query multiple fields from the same type. You either store the intermediate in a variable, or by destructuring.

For idea 1,

const directives = query.queryPost.$directives;
directives.foo; // no inputs
directives.bar({ ... }); // with inputs

const {
  queryPost: {
    $directives: {
      foo,
      bar
    }
  }
} = query;
bar({ ... }); // looks awkward, tbh.

For idea 3,

const post = query.queryPost;
post["@foo"]; // no inputs
post["@bar"]({ ... }); // with inputs

const {
  queryPost: {
    ["@foo"]: foo, // requires local alias
    ["@bar"]: bar
  }
} = query;
bar({ ... }); // same, looks awkward

For me, choosing between no-unused-vars and no-unused-expressions is the hardest part. selectFields(query, "*") won't work for directives.

@jmsegrev
Copy link

jmsegrev commented Jul 1, 2021

The first one, has less code between the two, for the case of no-unused-vars, we could have the proxy trap for the $directives to only accept functions with optional arguments, so no inputs directives will be query.queryPost.$directives.foo().

I'm not yet familiar with how the proxy traps and types are defined in gqless, but I think this could introduce complications for adding directives to graphql scalars, think query.queryPost.title.$directives.foo(), title is of type string, then the same type problem from previous idea 2 comes to mind, unless all native types are being extended?.

@vicary
Copy link

vicary commented Jul 2, 2021

Yes you're right. It only seemed mildly inconvenient to me at first, but it almost defeats the good feelings I had when looking at gqless.com the first time. I'd like gqless to keep that feeling for all new users in the future.

Honestly I have no proposals good enough for directives right now.

@jmsegrev
Copy link

jmsegrev commented Jul 2, 2021

Indeed, when having that in mind, it's not trivial.
What about a function like the one proposed by @natew in the first comment?

directives(query.queryPost.title, [ { foo: { ttl: 20 } }, 'bar' ])

@vicary
Copy link

vicary commented Jul 2, 2021

Not sure how React-style is supposed to look like. But this syntax kind of aligns with the resolved(...) API, I think it should be comfortable enough for core users.

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

4 participants