Commit f8ffad3
authored
feat(validation): reject directive definition cycles (#4726)
Adds a new `NoDirectiveDefinitionCyclesRule` SDL validation rule that
detects cycles in the graph of directives used within directive
definitions.
### What is a directive definition cycle?
A cycle arises when following the references within directive
definitions leads back to the same directive. Two kinds of references
are tracked:
1. **Directive argument types** — if `@foo` takes an argument of type
`InputObject`, and `InputObject` has a field with directive `@foo`, that
forms a cycle: `@foo → InputObject → @foo`.
2. **Directives on directive definitions** — when the experimental
`experimentalDirectivesOnDirectiveDefinitions` parser option is enabled,
a directive applied directly to a directive definition or extension is
also a reference. If `@a` is annotated with `@b` and `@b` is annotated
with `@a`, that is a cycle: `@a → @b → @a`.
Cycles make it impossible to build a well-defined schema; the spec
already [forbids
them](https://spec.graphql.org/draft/#sec-Type-System.Directives.Type-Validation),
and experimental directive extensions [do the
same](https://github.com/graphql/graphql-spec/pull/1206/changes#diff-30a69c5a5eded8e1aea52e53dad1181e6ec8f549ca2c50570b035153e2de1c43R2342).
### Key design question: should we consider existing schema applied
directives?
In this initial attempt, when validating an SDL extension document
against an existing schema, only references that appear **in the
document being validated** are considered to close a cycle. Applied
directives already persisted in the base schema are not counted as
cycle-forming edges.
This means you can have a pre-existing schema like:
```graphql
directive @A @b on DIRECTIVE_DEFINITION
directive @b on DIRECTIVE_DEFINITION
```
…where `@a` already carries `@b` in the stored schema, and then in your
extension document write:
```graphql
extend directive @b @A
```
This will **not** be reported as a cycle (`@a → @b → @a`), because the
`@b` edge on `@a` comes from the original schema, not from the document
under validation. The rule only errors when at least one edge in the
detected cycle is introduced by the new document. This is because
applied directives, while accessible via the stored
definitions/extensions, do not actually form part of the existing schema
object => at least, per the specification.
Is this the desired behavior? @BoD @graphql/tsc1 parent 6618357 commit f8ffad3
6 files changed
Lines changed: 757 additions & 1 deletion
File tree
- src
- utilities/__tests__
- validation
- __tests__
- rules
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
476 | 476 | | |
477 | 477 | | |
478 | 478 | | |
| 479 | + | |
479 | 480 | | |
480 | 481 | | |
481 | 482 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
316 | 316 | | |
317 | 317 | | |
318 | 318 | | |
319 | | - | |
| 319 | + | |
320 | 320 | | |
321 | 321 | | |
322 | 322 | | |
| |||
0 commit comments