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

[RFC] Composable fields #682

Closed
leoloso opened this issue Feb 5, 2020 · 12 comments
Closed

[RFC] Composable fields #682

leoloso opened this issue Feb 5, 2020 · 12 comments

Comments

@leoloso
Copy link

leoloso commented Feb 5, 2020

We can use directives to modify the value of a field. For instance, let's say that the date field from type Post returns the value in "YYYY-mm-dd" format:

query {
  posts {
    date
  }
}

... and we want to format it as "d/m/Y". This could be done through a field arg, if the field supports it:

query {
  posts {
    date:date(format: "d/m/Y")
  }
}

If it does not, then we can create a custom directive:

query {
  posts {
    date@format(format: "d/m/Y")
  }
}

So far, this is good enough. But then we have new requirements:

  • If the year of the post is the current year, use "d/m" format instead
  • But, if the language of the post is Chinese, use "Y-m-d"

Here we run into a few problems:

  • The directive will start accumulating custom code, that possibly can't be reused for other projects/clients/etc, leading to plenty of bloat down the road
  • The directive will need to reference other fields (such as field lang to get the post's language) which are not needed on the client-side; we may need to add it to the query just to feed it into the directive, which is a hack and goes against the GraphQL philosophy of querying only what is needed

Hence, directives are not good enough to satisfy all possible use cases.

Alternative: code in client-side

We can also modify the field values on the application on the client-side. However, this approach produces a few problems:

  • Logic will be duplicated across the different clients (website, iOS app, Android app, etc), forcing the developer to keep them in sync, adding bureaucracy and increasing the chance of introducing bugs
  • Conditional logic needs to retrieve all values for all conditions to be processed on the client-side, including the false ones
  • It makes the user's device (eg: an old smartphone) run additional runtime code, making the application slower

Proposed solution: composable fields

It would be better to modify the field's value already at the GraphQL server, and have the final result already in the response to the query, without having to resort to custom coding. For this, I suggest using fields that can compose other fields, i.e. the result of a field can be an input value to another field.

Using the same example as before, we could have a new field format, like this:

query {
  posts {
    date:format(date: date, format: "d/m/Y")
  }
}

As it can be seen, field format must receive 2 inputs: the date and the format (they are both strings). The date string is itself the response from the date field. Hence, field format is composing field date (to make it more evident, we can add () to the composed field: format(date: date(), format: "d/m/Y")).

To be precise, format should not be called a field, but an operator, since its only purpose is to modify some other value, and it must necessarily be given the input values. This can be discussed. For the time being I call it a field, because:

  • The implementation between a field and an operator is exactly the same: through a resolver
  • It is a generalization. For instance, a field time returning the current time is not an operator, and not a property from the object either. In order to avoid establishing names for all different use cases (fields, operators, helpers, etc) we just call them all as fields.

Now, having the composable fields if, equals, year, currentYear and format, we can satisfy the previous requirement. For the sake of clarity, I have split the field into several lines; this may also need be part of the RFC, since having a very extensive logic in a single line is not easy to read (also discussable):

query {
  posts {
    date: format(date: date, format: if(
      condition: equals(
        value1: lang,
        value2: "ZH"
      ),
      then: "Y-m-d",
      else: if(
        condition: equals(
          value1: year(
            date: date
          ),
          value2: currentYear
        ),
        then: "d/M",
        else: "d/M/Y"
      )
    ))
  }
}

Please notice the following:

  1. Fields if, equals, year and currentYear are global fields, which can be used under any type
  2. Fields lang and date belong to the Post type

This will certainly impact the architecture of the server, in the following manner:

From 1., we can either add these fields to every single type, which is not a nice solution, or declare them once as "global fields", and have the types accept two sets of fields: the ones for that type plus the global ones.

From 2., we need to be able to evaluate a field against the object, and bubble this result upwards to its composing field, which will itself be evaluated and bubble this result upwards to its composing field, and so on with unbound levels deep.

Added benefit: dynamic @skip/include directives

Please notice the additional benefit to the last item: @skip/include can be evaluated against the object.

Currently, directives @skip and @include are pretty static, simply including the result or not if a variable is either true or false. However, with this proposal, the value to execute against can be a property of the same object, making it much more dynamic.

For instance, we could execute the following query (assuming that date doesn't produce an error from being defined twice):

query {
  posts {
    date:format(date:date,format:"Y-m-d") @include(if:equals(
      value1:lang,
      value2:"ZH"
    )
    date:format(date:date,format:"d/m/Y") @skip(if:equals(
      value1:lang,
      value2:"ZH"
    )
  }
}

Demonstration of this solution

I have already implemented this solution for this GraphQL server in PHP. Since the GraphQL syntax doesn't support this feature, I have so far used this alternative syntax.

Example of global fields: check the ones listed under property globalFields here.

Example of a query with composable fields (view response):

/?
format=Y-m-d&
query=
  posts.
    if (
      hasComments(), 
      sprintf(
        "This post has %s comment(s) and title '%s'", [
          commentsCount(),
          title()
        ]
      ), 
      sprintf(
        "This post was created on %s and has no comments", [
          date(format: if(not(empty($format)), $format, d/m/Y))
        ]
      )
    )@postDesc

Example of a query with composable fields used with @skip/include directives(view response):

/?
format=Y-m-d&
query=
  posts.
    sprintf(
      "This post has %s comment(s) and title '%s'", [
        commentsCount(),
        title()
      ]
    )@postDesc<include(if:hasComments())>|
    sprintf(
      "This post was created on %s and has no comments", [
        date(format: if(not(empty($format)), $format, d/m/Y))
      ]
    )@postDesc<include(if:not(hasComments()))>
@sungam3r
Copy link
Contributor

sungam3r commented Feb 5, 2020

For me it seems to be complicated and far from graphql design.

@leoloso
Copy link
Author

leoloso commented Feb 13, 2020

The examples I provided so far look complicated, but this proposal would also add simplicity into the queries.

For instance, the response of a field title can be made uppercase with a global field uppercase:

query {
  posts {
    title:uppercase(title)
  }
}

We can limit its length to 30 chars with field limitLength:

query {
  posts {
    excerpt:limitLength(title, 30)
  }
}

And we can combine the global fields:

query {
  posts {
    excerpt:limitLength(uppercase(title), 30)
  }
}

This same outcome can also be achieved using directives. However, this case below, which involves 2 fields (title and excerpt), would not be so simple to implement through directives (if doable, the resolver code will be messy):

query {
  posts {
    excerpt:if(isNull(excerpt),limitLength(uppercase(title), 30),excerpt)
  }
}

@michaelstaib
Copy link
Member

michaelstaib commented Feb 13, 2020

I would suggest that you first state the use-case you want to solve and then try to outline a solution to it with minimal change to GraphQL.

It is always best to not change/break it.

A change that breaks existing tooling should be worth it.

We should not start just introducing changes because they are fancy.

Here is some guiding principals for changes to the spec:
https://github.com/graphql/graphql-spec/blob/master/CONTRIBUTING.md#guiding-principles

@leoloso
Copy link
Author

leoloso commented Feb 14, 2020

I understand that this is a heavy change, but I think it's quite worth (and not just fancy). My motivation is to avoid duplicated code across clients consuming the API (as I mentioned in the original issue), but there are other applications too.

Reviewing the other issues, I identified the following 4 (3 of them still open, 1 already closed) which could either be solved with, or benefit from, composable fields:

From #476:

If we want to omit the null responses, as to go from this:

{
  "id": 1,
  "name": "John",
  "hobby": null
}

to this:

{
  "id": 1,
  "name": "John"
}

we could do:

{
  user(id:1) {
    id
    name
    hobby @skip(if:isNull(value:hobby))
  }
}

From #377:

If the @export directive is implemented, and the value of what is exported on query A doesn't match exactly what query B requires, then these 2 could be bridged through composable fields.

For instance, we can create a composed ID "name-surname" from composing fields name and surname with a global field concat:

query A {
  person {
    composedID: concat(elements: [name, "-", surname]) @export(as: "personID")
  }
}
        
query B($personID : String!) {
  person (id: $personID) {
    id
    hobby
  }
}

Alternatively, we could attempt to use composable fields to also modify inputs, and then we can use them on the query B like this:

query A {
  person {
    name @export(as: "personName")
    surname @export(as: "personSurname")
  }
}
        
query B($personName : String!, $personSurname : String!) {
  person (id: concat(elements: [$personName, "-", $personSurname])) {
    id
    hobby
  }
}

This could be useful for dealing with federation (eg: Apollo Federation supports compound keys).


From #248:

Using composable fields with inputs, instead of accessing the property from a complex input like requested:

{
  findDogByName(name: $complexInput.name) {
    name
  }
}

composable fields could resolve it like this:

{
  findDogByName(name: extract(object:$complexInput, property:"name")) {
    name
  }
}

IMO, this latter solution is nicer than the requested one, because it's more generic, and the extract global field can be implemented at the API level, so there is no need to introduce new syntax for any particular use case (in this case, $complexInput.name).


From #431:

We can create a serialize global field. It could tha takes an input of any type and return its representation as a String:

{
  date {
    year
    month
  }
}

returns:

{
  "date": {
    "year": 2018,
    "month": 04
  }
}

And doing:

{
  date:serialize(object:date)
}

returns:

{
  "date": "2018-04-28T04:53:23+03:00"
}

In this case using a directive @serialize would not work, since the type of the response is being changed from Date to String. Using a global field serialize would work because it can accept the type Date as an input (or a Serializable interface) and will return a String.

@sungam3r
Copy link
Contributor

As I understand it, the consequence of your proposal is to mix the concepts of fields and directives. Right?

@sungam3r
Copy link
Contributor

And we get 2 tools that solve similar problems - directives and so called "global fields".

@leoloso
Copy link
Author

leoloso commented Feb 14, 2020

I'm not saying that global fields will replace directives, but will complement them. For instance, right now you can't solve the null/omission issue (#476) with directives alone, but with directives+global fields you can.

I can see that global fields could be applied to:

  1. Other fields: { user { name:uppercase(name) } }
  2. Fields within directives: { user { name @skip(if:isNull(value:name)) } }
  3. Inputs: { user { posts(search: surname) } } <= searching for the user's posts which contain the user's surname

The cost of global fields is the addition of () to the syntax. However, given just that, I think that it can also tackle several different use cases, such as $complexInput.name, in a rather generic manner.

I know it's a heavy change, but it could be worth it: after adding support for this feature to my GraphQL server, queries got more powerful, so I can achieve much more through my API.

@leoloso
Copy link
Author

leoloso commented Feb 14, 2020

My 3rd example above is not about global fields, but just to allow fields as an input. But once again, fields could be composable, like this:

{
  user {
    posts(search: concat(elements:[name,"-",surname]))
  } 
}

In this case, concat is a global field, and name and surname are normal fields from the User type

@sungam3r
Copy link
Contributor

This is my personal opinion - this proposal will be developed with a probability close to zero, as it contradicts to several guiding principals.

@smolinari
Copy link

smolinari commented Feb 14, 2020

I have to agree this is unnecessary, but for another reason. This suggestion is along the same lines as asking for a permissions system inside the API gateway instead of just returning null for fields the user shouldn't see. A permissions system is a huge implementation detail. It belongs in the business/ view logic and as such, can be any of many solutions. Thus, it is impossible to standardize. Your suggestion isn't quite non-standardizable as a permissions system. But for sure, the things you've pointed out as examples are all business or view logic or rather can be solved within those layers, thus, the logic shouldn't need to go into GraphQL APIs.

The question should be asked all the time -> Am I trying to do too much with the gateway, as in, having business logic leak into it? The easiest way to answer that question is, if there is a problem or a change needed, where would I have to go to fix or amend it? The last place you should look is in the API. 😃

Scott

@sungam3r
Copy link
Contributor

Of course. Directives are already a means of indicating some kind of logic in API.

@leoloso
Copy link
Author

leoloso commented Aug 30, 2022

Since it's been argued that this is not a good idea, then I'm closing this issue.

@leoloso leoloso closed this as completed Aug 30, 2022
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