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] Repeatable directives #472
Changes from 3 commits
52d397f
96aab71
1ba4e19
5c8c4bd
218e0da
477919e
71ccd58
8391a1e
69669c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -187,6 +187,7 @@ type __Directive { | |
description: String | ||
locations: [__DirectiveLocation!]! | ||
args: [__InputValue!]! | ||
unique: Boolean! | ||
} | ||
|
||
enum __DirectiveLocation { | ||
|
@@ -417,3 +418,4 @@ Fields | |
locations this directive may be placed. | ||
* `args` returns a List of `__InputValue` representing the arguments this | ||
directive accepts. | ||
* `repeatable` must return a Boolean which permits using the directive multiple times at the same location. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we already have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with both alternatives. Though, I also find consistency argument quite compelling. I renamed it for now 5c8c4bd, but I can chnage it later if somebody raises a concern. I wonder what others think about it? |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1440,7 +1440,7 @@ query @skip(if: $foo) { | |||||
**Formal Specification** | ||||||
|
||||||
* For every {location} in the document for which Directives can apply: | ||||||
* Let {directives} be the set of Directives which apply to {location}. | ||||||
* Let {directives} be the set of Directives which apply to {location} and not marked as `repeatable`. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
* For each {directive} in {directives}: | ||||||
* Let {directiveName} be the name of {directive}. | ||||||
* Let {namedDirectives} be the set of all Directives named {directiveName} | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great to include a more complex example that illustrates how
repeatable
is useful or at least add a note that directive order is significant and you can use it to create chains of directives.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I extended the example 218e0da and the description 477919e.
I struggled with it a bit since I find it hard to strike the right balance between completeness of the example including its description and succinct focused content required from a perspective of the specification. For this reason I also hesitated mentioning the order significance since it has its own dedication section with a separate example.
WDYT?