-
Notifications
You must be signed in to change notification settings - Fork 237
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
docs: improve custom directives doc with federation #1009
docs: improve custom directives doc with federation #1009
Conversation
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.
lgtm
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.
lgtm
docs/custom-directive.md
Outdated
|
||
# Federation and Custom Directives | ||
|
||
If we are using Federation, we must follow a different approach to create custom 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.
It would be helpful to explain why our existing approach won't work if we are using federation. And also to introduce readers to what we will be doing next (i.e. demonstrate by creating a new @upper
custom directive)
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 added more info and reviewed this part.
Let me know if it's better now, please.
docs/custom-directive.md
Outdated
|
||
If we are using Federation, we must follow a different approach to create custom directives. | ||
With Federation, we use special syntaxes and custom directives already defined; for instance, `extends` or `@key`. These directives are not defined in the schema but are added by the `buildFederationSchema` function from the `@mercuriusjs/federation` library. Hence, to work with the Federation, we must follow a different approach to maintain the Federation's custom directives and add ours as well. |
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'd suggest phrasing this paragraph like this:
Because schemas involved in GraphQL federation may use special syntax (e.g.
extends
) and custom directives (e.g.@key
) that are not available in non-federated schemas, there's some extra steps that needs to be run to generate the executable schema, involving the use ofbuildFederationSchema
from the@mercuriusjs/federation
library andprintSchemaWithDirectives
from the@graphql-tools/utils
library.
To see how this works, we will go through another example where we create a custom directive to uppercase the value of a field in a federated environment.
When I read "we must follow a different approach" I get the impression that I need to do something completely different, but, as I understand it, it's the same approach just with more steps to cater for the fact that we don't work with types but with entities.
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.
Ok, I fixed the introduction as you suggested.
Thanks @d4nyll
I'm going to merge this as it's just documentation. @mcollina I hope you're ok with this, feel free to reply with your thoughts after the PR is merged if you have any concerns. |
Improve documentation for custom directives with Federation
Close: mercurius-js/mercurius-federation#28