-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Comments
For me it seems to be complicated and far from graphql design. |
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 query {
posts {
title:uppercase(title)
}
} We can limit its length to 30 chars with field 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 ( query {
posts {
excerpt:if(isNull(excerpt),limitLength(uppercase(title), 30),excerpt)
}
} |
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: |
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 {
"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 For instance, we can create a composed ID 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 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 From #431: We can create a {
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 |
As I understand it, the consequence of your proposal is to mix the concepts of fields and directives. Right? |
And we get 2 tools that solve similar problems - directives and so called "global fields". |
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:
The cost of global fields is the addition of 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. |
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, |
This is my personal opinion - this proposal will be developed with a probability close to zero, as it contradicts to several guiding principals. |
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 |
Of course. Directives are already a means of indicating some kind of logic in API. |
Since it's been argued that this is not a good idea, then I'm closing this issue. |
We can use directives to modify the value of a field. For instance, let's say that the
date
field from typePost
returns the value in"YYYY-mm-dd"
format:... and we want to format it as
"d/m/Y"
. This could be done through a field arg, if the field supports it:If it does not, then we can create a custom directive:
So far, this is good enough. But then we have new requirements:
"d/m"
format instead"Y-m-d"
Here we run into a few problems:
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 neededHence, 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:
false
onesProposed 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: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 thedate
field. Hence, fieldformat
is composing fielddate
(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: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
andformat
, 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):Please notice the following:
if
,equals
,year
andcurrentYear
are global fields, which can be used under any typelang
anddate
belong to thePost
typeThis 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
directivesPlease 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 eithertrue
orfalse
. 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):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):
Example of a query with composable fields used with
@skip/include
directives(view response):The text was updated successfully, but these errors were encountered: