Skip to content

Edits to short directives article#4979

Merged
StephenBarlow merged 4 commits intomainfrom
sb/directive-edits
Mar 9, 2021
Merged

Edits to short directives article#4979
StephenBarlow merged 4 commits intomainfrom
sb/directive-edits

Conversation

@StephenBarlow
Copy link
Copy Markdown
Contributor

Setting the stage for editing the much larger custom directives article

@StephenBarlow
Copy link
Copy Markdown
Contributor Author

@trevor-scheer @glasser Hopefully a pretty quick one for whoever

@abernix abernix requested review from glasser and trevor-scheer March 4, 2021 12:34
@abernix abernix added the 📝 documentation Focuses on changes to the documentation (docs) label Mar 4, 2021
Comment thread docs/source/schema/directives.md Outdated
description: Using schema directives to transform schema types, fields, and arguments
title: Directives
sidebar_title: Directives
description: Configure schema types, fields, and arguments
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there are essentially two kinds of directives in GraphQL: executable directives that you put in your query/mutation (like @skip), and schema directives that you put in your schema (like @deprecated). I mean the spec doesn't literally differentiate between them but in practice most directives are declared to go on a set of executable locations or a set of schema locations.

Previously this document specifically talked about "schema directives" a lot. Though it then mentioned @skip and @include anyway.

Changing the document to be about directives in general does make sense to me, but I think at some point you should actually explain that some directives go on operations and some on schemas? Especially since at some point in the file you do start talking specifically about defining schema directives. (ie, these docs don't explain how you'd write code to react to an executable directive, which perhaps may just be that we don't have a great pattern, I dunno.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points here. Revisiting.

Comment thread docs/source/schema/directives.md Outdated

## Valid locations

Each directive's definition specifies _where_ it can appear in a GraphQL document. For example, here's the GraphQL spec's definition of the `@deprecated` directive:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably explain somewhere that directives have definitions, which are part of the schema; a directive needs to be defined in the schema in order for you to use it in that schema or in an operation being validated against the schema.

@StephenBarlow
Copy link
Copy Markdown
Contributor Author

@glasser lemme know how this looks

Copy link
Copy Markdown
Member

@glasser glasser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw as a heads up for when you get to the schema directives page: AS now specifically links in an outdated version of graphql-tools (v4). You may want to note that if you want to use a newer version of the schema directives code you can use @graphql-tools/schema yourself and call makeExecutableSchema. See #4810 (comment) for details (hopefully I can reduce the number of caveats)

Comment thread docs/source/schema/directives.md Outdated
```

This indicates that `@deprecated` can decorate either a schema field definition (as shown in the example above) or an enum value definition (as shown here):
This indicates that `@deprecated` can decorate either a `SCHEMA_FIELD` definition (as shown at the top of the article) or a schema `ENUM_VALUE` definition (as shown here):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mistyped "schema FIELD_DEFINITION"?

Each directive's definition specifies _where_ it can appear in a GraphQL document. For example, here's the GraphQL spec's definition of the `@deprecated` directive:
Each directive can only appear in _certain_ locations within a GraphQL schema or operation. These locations are listed in the directive's definition.

For example, here's the GraphQL spec's definition of the `@deprecated` directive:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may or may not be interesting that this is likely to change to add some more locations (graphql/graphql-spec#805) and that in fact graphql-js and others already support more locations...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, hmm. I'll keep like this for now to match 2018 and might revisit this with additional perspective after cleaning up the big article

Comment thread docs/source/schema/directives.md Outdated
| `@include(if: Boolean!)` | If `false`, the decorated field or fragment in an operation is _not_ resolved by the GraphQL server. |

GraphQL provides several default directives: [`@deprecated`](http://facebook.github.io/graphql/draft/#sec--deprecated), [`@skip`](http://facebook.github.io/graphql/draft/#sec--skip), and [`@include`](http://facebook.github.io/graphql/draft/#sec--include).
## Custom directives
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section should still have "schema" in the name since that's what it's about.


### Schema directives vs. operation directives

Usually, a given directive appears _exclusively_ in GraphQL schemas or _exclusively_ in GraphQL operations (rarely both, although the spec allows this).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the implementing directives page calls these query directives. I think moving towards operation rather than query is good, though, and I think you said you were heading to that page next.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll replicate this change over there.

Comment thread docs/source/schema/directives.md Outdated
### Creating

## Using custom schema directives
See [Implementing directives](/schema/creating-directives/).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that this subsection really helps much.

How about dropping the subsections and just starting the section with something like:

You can extend Apollo Server with custom schema directives created by you or a third party. To do so, add a definition for the directive to your schema. If you'd like your server to find all the usages of your directive during server startup and react to them in some way, you can implement a SchemaDirectiveVisitor and pass it to new ApolloServer. For full details on everything a SchemaDirectiveVisitor can do, see the section on implementing directives.

@StephenBarlow StephenBarlow merged commit 6378cfe into main Mar 9, 2021
@StephenBarlow StephenBarlow deleted the sb/directive-edits branch March 9, 2021 23:23
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

📝 documentation Focuses on changes to the documentation (docs)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants