From b58ba99067951e31f3b1733175a156e831eb4969 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 2 Sep 2026 09:38:49 +0200 Subject: [PATCH 1/6] Allow disabling directives by name DirectiveLocator::disable() marks a directive as non-existent: its definition is left out of the schema and using it fails validation. This enables enforcing project conventions that a built-in directive would bypass. Explicitly resolved classes now also take precedence in classes(), so definitions() can no longer print a directive that differs from the one that is executed. --- .../custom-directives/getting-started.md | 28 ++++++++++++ src/Schema/DirectiveLocator.php | 44 +++++++++++++++---- tests/Unit/Schema/DirectiveLocatorTest.php | 25 +++++++++++ 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/docs/master/custom-directives/getting-started.md b/docs/master/custom-directives/getting-started.md index 42959b95c..38755da72 100644 --- a/docs/master/custom-directives/getting-started.md +++ b/docs/master/custom-directives/getting-started.md @@ -103,3 +103,31 @@ When Lighthouse encounters a directive within the schema, it starts looking for This means that our directive is already registered, just by matter of defining it in the default namespace. Will take precedence over potential other directives with the same name. + +## Override Or Disable Directives + +Namespace precedence is coarse: it applies to all directives of a namespace at once. +To control a single directive name, bind its class explicitly in a service provider: + +```php +namespace App\Providers; + +use Illuminate\Support\ServiceProvider; +use Nuwave\Lighthouse\Schema\DirectiveLocator; +use Nuwave\Lighthouse\Schema\Directives\PaginateDirective; + +class GraphQLServiceProvider extends ServiceProvider +{ + public function boot(DirectiveLocator $directiveLocator): void + { + // Use the built-in directive, even though a plugin defines its own @paginate + $directiveLocator->setResolved('paginate', PaginateDirective::class); + + // Forbid @field, e.g. to enforce that resolvers live in the configured namespaces + $directiveLocator->disable('field'); + } +} +``` + +A disabled directive behaves as if it did not exist: its definition is left out of the schema +and using it fails schema validation with an unknown directive error. diff --git a/src/Schema/DirectiveLocator.php b/src/Schema/DirectiveLocator.php index 3242c81a3..61a305579 100644 --- a/src/Schema/DirectiveLocator.php +++ b/src/Schema/DirectiveLocator.php @@ -33,13 +33,16 @@ class DirectiveLocator /** * A map from short directive names to full class names. * + * Takes precedence over the namespaces, `null` marks a directive as disabled. + * * E.g. * [ * 'create' => 'Nuwave\Lighthouse\Schema\Directives\CreateDirective', * 'custom' => 'App\GraphQL\Directives\CustomDirective', + * 'field' => null, * ] * - * @var array> + * @var array|null> */ protected array $resolvedClassnames = []; @@ -77,7 +80,7 @@ public function namespaces(): array */ public function classes(): array { - $directives = []; + $directives = $this->resolvedClassnames; foreach ($this->namespaces() as $directiveNamespace) { /** @var array $classesInNamespace */ @@ -93,12 +96,15 @@ public function classes(): array continue; } - // Only add the first directive that was found - $directives[self::directiveName($class)] ??= $class; + // Only add the first directive that was found, keeping disabled ones out + $directiveName = self::directiveName($class); + if (! array_key_exists($directiveName, $directives)) { + $directives[$directiveName] = $class; + } } } - return $directives; + return array_filter($directives); } /** @@ -133,7 +139,12 @@ public function create(string $directiveName): Directive public function resolve(string $directiveName): string { if (array_key_exists($directiveName, $this->resolvedClassnames)) { - return $this->resolvedClassnames[$directiveName]; + $resolvedClassname = $this->resolvedClassnames[$directiveName]; + if ($resolvedClassname === null) { + throw self::noDirectiveFound($directiveName); + } + + return $resolvedClassname; } foreach ($this->namespaces() as $directiveNamespace) { @@ -152,7 +163,12 @@ public function resolve(string $directiveName): string } } - throw new DirectiveException("No directive found for `{$directiveName}`"); + throw self::noDirectiveFound($directiveName); + } + + protected static function noDirectiveFound(string $directiveName): DirectiveException + { + return new DirectiveException("No directive found for `{$directiveName}`"); } /** Returns the expected class name for a directive name. */ @@ -171,14 +187,24 @@ public static function directiveName(string $className): string ); } - /** @param class-string<\Nuwave\Lighthouse\Support\Contracts\Directive> $directiveClass */ - public function setResolved(string $directiveName, string $directiveClass): self + /** @param class-string<\Nuwave\Lighthouse\Support\Contracts\Directive>|null $directiveClass */ + public function setResolved(string $directiveName, ?string $directiveClass): self { $this->resolvedClassnames[$directiveName] = $directiveClass; return $this; } + /** + * Make a directive unavailable, as if it did not exist. + * + * Its definition is left out of the schema and using it fails validation. + */ + public function disable(string $directiveName): self + { + return $this->setResolved($directiveName, null); + } + /** * Get all directives that are associated with an AST node. * diff --git a/tests/Unit/Schema/DirectiveLocatorTest.php b/tests/Unit/Schema/DirectiveLocatorTest.php index e62bb3a06..1fc714623 100644 --- a/tests/Unit/Schema/DirectiveLocatorTest.php +++ b/tests/Unit/Schema/DirectiveLocatorTest.php @@ -6,6 +6,7 @@ use Nuwave\Lighthouse\Exceptions\DirectiveException; use Nuwave\Lighthouse\Schema\DirectiveLocator; use Nuwave\Lighthouse\Schema\Directives\BaseDirective; +use Nuwave\Lighthouse\Schema\Directives\ComplexityDirective; use Nuwave\Lighthouse\Schema\Directives\FieldDirective; use Nuwave\Lighthouse\Schema\Values\FieldValue; use Nuwave\Lighthouse\Support\Contracts\FieldMiddleware; @@ -77,6 +78,30 @@ public function handleField(FieldValue $fieldValue): void {} $this->assertNotInstanceOf(BaseDirective::class, $directive); } + public function testResolvesExplicitlySetClassInsteadOfScannedNamespaces(): void + { + $this->directiveLocator->setResolved('field', ComplexityDirective::class); + + $this->assertSame(ComplexityDirective::class, $this->directiveLocator->classes()['field']); + } + + public function testThrowsIfDirectiveIsDisabled(): void + { + $this->directiveLocator->disable('field'); + + $this->expectException(DirectiveException::class); + $this->expectExceptionMessage('No directive found for `field`'); + + $this->directiveLocator->create('field'); + } + + public function testOmitsDefinitionOfDisabledDirective(): void + { + $this->directiveLocator->disable('field'); + + $this->assertArrayNotHasKey('field', $this->directiveLocator->classes()); + } + public function testThrowsIfDirectiveNameCanNotBeResolved(): void { $this->expectException(DirectiveException::class); From 5c3696ab3c5fe307b498464ea11c2571181e4684 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 2 Sep 2026 09:39:32 +0200 Subject: [PATCH 2/6] Add changelog entry --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 058eab2b4..0a4067720 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Added + +- Add `DirectiveLocator::disable()` to make a directive unavailable, leaving its definition out of the schema https://github.com/nuwave/lighthouse/pull/2787 + +### Changed + +- Let classes passed to `DirectiveLocator::setResolved()` take precedence in `DirectiveLocator::classes()` and thus `DirectiveLocator::definitions()` https://github.com/nuwave/lighthouse/pull/2787 + ## v6.69.2 ### Fixed From 76e049f32745509ea30005e79a594f407435f1e9 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 2 Sep 2026 10:37:10 +0200 Subject: [PATCH 3/6] Drop disable(), keeping only the classes() precedence fix Disabling a directive at resolution time is unsafe: Lighthouse resolves the fields generated by @paginate, @node and federation through @field, so disabling @field broke those schemas. Whether a built-in directive is used internally is not something users can know, so the API cannot be offered as it was. Restricting what may appear in a hand-written schema belongs to whoever parses that schema. --- CHANGELOG.md | 4 -- .../custom-directives/getting-started.md | 8 +--- src/Schema/DirectiveLocator.php | 42 ++++--------------- tests/Unit/Schema/DirectiveLocatorTest.php | 17 -------- 4 files changed, 10 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a4067720..a52cc06d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,6 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased -### Added - -- Add `DirectiveLocator::disable()` to make a directive unavailable, leaving its definition out of the schema https://github.com/nuwave/lighthouse/pull/2787 - ### Changed - Let classes passed to `DirectiveLocator::setResolved()` take precedence in `DirectiveLocator::classes()` and thus `DirectiveLocator::definitions()` https://github.com/nuwave/lighthouse/pull/2787 diff --git a/docs/master/custom-directives/getting-started.md b/docs/master/custom-directives/getting-started.md index 38755da72..eeb65afad 100644 --- a/docs/master/custom-directives/getting-started.md +++ b/docs/master/custom-directives/getting-started.md @@ -104,7 +104,7 @@ When Lighthouse encounters a directive within the schema, it starts looking for This means that our directive is already registered, just by matter of defining it in the default namespace. Will take precedence over potential other directives with the same name. -## Override Or Disable Directives +## Override A Single Directive Namespace precedence is coarse: it applies to all directives of a namespace at once. To control a single directive name, bind its class explicitly in a service provider: @@ -122,12 +122,6 @@ class GraphQLServiceProvider extends ServiceProvider { // Use the built-in directive, even though a plugin defines its own @paginate $directiveLocator->setResolved('paginate', PaginateDirective::class); - - // Forbid @field, e.g. to enforce that resolvers live in the configured namespaces - $directiveLocator->disable('field'); } } ``` - -A disabled directive behaves as if it did not exist: its definition is left out of the schema -and using it fails schema validation with an unknown directive error. diff --git a/src/Schema/DirectiveLocator.php b/src/Schema/DirectiveLocator.php index 61a305579..dac0c085d 100644 --- a/src/Schema/DirectiveLocator.php +++ b/src/Schema/DirectiveLocator.php @@ -33,16 +33,15 @@ class DirectiveLocator /** * A map from short directive names to full class names. * - * Takes precedence over the namespaces, `null` marks a directive as disabled. + * Takes precedence over the namespaces. * * E.g. * [ * 'create' => 'Nuwave\Lighthouse\Schema\Directives\CreateDirective', * 'custom' => 'App\GraphQL\Directives\CustomDirective', - * 'field' => null, * ] * - * @var array|null> + * @var array> */ protected array $resolvedClassnames = []; @@ -96,15 +95,12 @@ public function classes(): array continue; } - // Only add the first directive that was found, keeping disabled ones out - $directiveName = self::directiveName($class); - if (! array_key_exists($directiveName, $directives)) { - $directives[$directiveName] = $class; - } + // Only add the first directive that was found + $directives[self::directiveName($class)] ??= $class; } } - return array_filter($directives); + return $directives; } /** @@ -139,12 +135,7 @@ public function create(string $directiveName): Directive public function resolve(string $directiveName): string { if (array_key_exists($directiveName, $this->resolvedClassnames)) { - $resolvedClassname = $this->resolvedClassnames[$directiveName]; - if ($resolvedClassname === null) { - throw self::noDirectiveFound($directiveName); - } - - return $resolvedClassname; + return $this->resolvedClassnames[$directiveName]; } foreach ($this->namespaces() as $directiveNamespace) { @@ -163,12 +154,7 @@ public function resolve(string $directiveName): string } } - throw self::noDirectiveFound($directiveName); - } - - protected static function noDirectiveFound(string $directiveName): DirectiveException - { - return new DirectiveException("No directive found for `{$directiveName}`"); + throw new DirectiveException("No directive found for `{$directiveName}`"); } /** Returns the expected class name for a directive name. */ @@ -187,24 +173,14 @@ public static function directiveName(string $className): string ); } - /** @param class-string<\Nuwave\Lighthouse\Support\Contracts\Directive>|null $directiveClass */ - public function setResolved(string $directiveName, ?string $directiveClass): self + /** @param class-string<\Nuwave\Lighthouse\Support\Contracts\Directive> $directiveClass */ + public function setResolved(string $directiveName, string $directiveClass): self { $this->resolvedClassnames[$directiveName] = $directiveClass; return $this; } - /** - * Make a directive unavailable, as if it did not exist. - * - * Its definition is left out of the schema and using it fails validation. - */ - public function disable(string $directiveName): self - { - return $this->setResolved($directiveName, null); - } - /** * Get all directives that are associated with an AST node. * diff --git a/tests/Unit/Schema/DirectiveLocatorTest.php b/tests/Unit/Schema/DirectiveLocatorTest.php index 1fc714623..e7f124600 100644 --- a/tests/Unit/Schema/DirectiveLocatorTest.php +++ b/tests/Unit/Schema/DirectiveLocatorTest.php @@ -85,23 +85,6 @@ public function testResolvesExplicitlySetClassInsteadOfScannedNamespaces(): void $this->assertSame(ComplexityDirective::class, $this->directiveLocator->classes()['field']); } - public function testThrowsIfDirectiveIsDisabled(): void - { - $this->directiveLocator->disable('field'); - - $this->expectException(DirectiveException::class); - $this->expectExceptionMessage('No directive found for `field`'); - - $this->directiveLocator->create('field'); - } - - public function testOmitsDefinitionOfDisabledDirective(): void - { - $this->directiveLocator->disable('field'); - - $this->assertArrayNotHasKey('field', $this->directiveLocator->classes()); - } - public function testThrowsIfDirectiveNameCanNotBeResolved(): void { $this->expectException(DirectiveException::class); From d7c128382d9c59f441709d65da76afe4273e526a Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 2 Sep 2026 11:13:09 +0200 Subject: [PATCH 4/6] Address review Mutate and return the resolved classnames directly, caching the scan. --- CHANGELOG.md | 2 +- src/Schema/DirectiveLocator.php | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a52cc06d0..4001e0868 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ You can find and compare releases at the [GitHub release page](https://github.co ### Changed -- Let classes passed to `DirectiveLocator::setResolved()` take precedence in `DirectiveLocator::classes()` and thus `DirectiveLocator::definitions()` https://github.com/nuwave/lighthouse/pull/2787 +- Classes passed to `DirectiveLocator::setResolved()` take precedence in `DirectiveLocator::classes()` and `DirectiveLocator::definitions()` https://github.com/nuwave/lighthouse/pull/2787 ## v6.69.2 diff --git a/src/Schema/DirectiveLocator.php b/src/Schema/DirectiveLocator.php index dac0c085d..5b66df01a 100644 --- a/src/Schema/DirectiveLocator.php +++ b/src/Schema/DirectiveLocator.php @@ -79,8 +79,6 @@ public function namespaces(): array */ public function classes(): array { - $directives = $this->resolvedClassnames; - foreach ($this->namespaces() as $directiveNamespace) { /** @var array $classesInNamespace */ $classesInNamespace = ClassFinder::getClassesInNamespace($directiveNamespace); @@ -96,11 +94,11 @@ public function classes(): array } // Only add the first directive that was found - $directives[self::directiveName($class)] ??= $class; + $this->resolvedClassnames[self::directiveName($class)] ??= $class; } } - return $directives; + return $this->resolvedClassnames; } /** From ceb6b6f01ba69421422a4f04ebf20e30809e3c75 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 2 Sep 2026 12:15:34 +0200 Subject: [PATCH 5/6] Pin a directive class that declares the name it is pinned to --- tests/Unit/Schema/DirectiveLocatorTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Unit/Schema/DirectiveLocatorTest.php b/tests/Unit/Schema/DirectiveLocatorTest.php index e7f124600..e1b3f83ac 100644 --- a/tests/Unit/Schema/DirectiveLocatorTest.php +++ b/tests/Unit/Schema/DirectiveLocatorTest.php @@ -6,12 +6,12 @@ use Nuwave\Lighthouse\Exceptions\DirectiveException; use Nuwave\Lighthouse\Schema\DirectiveLocator; use Nuwave\Lighthouse\Schema\Directives\BaseDirective; -use Nuwave\Lighthouse\Schema\Directives\ComplexityDirective; use Nuwave\Lighthouse\Schema\Directives\FieldDirective; use Nuwave\Lighthouse\Schema\Values\FieldValue; use Nuwave\Lighthouse\Support\Contracts\FieldMiddleware; use Nuwave\Lighthouse\Support\Contracts\FieldResolver; use Nuwave\Lighthouse\Support\Utils; +use Tests\Integration\Events\FieldDirective as AlternateFieldDirective; use Tests\TestCase; final class DirectiveLocatorTest extends TestCase @@ -80,9 +80,9 @@ public function handleField(FieldValue $fieldValue): void {} public function testResolvesExplicitlySetClassInsteadOfScannedNamespaces(): void { - $this->directiveLocator->setResolved('field', ComplexityDirective::class); + $this->directiveLocator->setResolved('field', AlternateFieldDirective::class); - $this->assertSame(ComplexityDirective::class, $this->directiveLocator->classes()['field']); + $this->assertSame(AlternateFieldDirective::class, $this->directiveLocator->classes()['field']); } public function testThrowsIfDirectiveNameCanNotBeResolved(): void From dafc6060d2a27016aed74cfdec94dca34cbc6256 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 2 Sep 2026 12:26:09 +0200 Subject: [PATCH 6/6] Drop the docs section setResolved() is not marked as public API, no need to document it. --- .../custom-directives/getting-started.md | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/docs/master/custom-directives/getting-started.md b/docs/master/custom-directives/getting-started.md index eeb65afad..42959b95c 100644 --- a/docs/master/custom-directives/getting-started.md +++ b/docs/master/custom-directives/getting-started.md @@ -103,25 +103,3 @@ When Lighthouse encounters a directive within the schema, it starts looking for This means that our directive is already registered, just by matter of defining it in the default namespace. Will take precedence over potential other directives with the same name. - -## Override A Single Directive - -Namespace precedence is coarse: it applies to all directives of a namespace at once. -To control a single directive name, bind its class explicitly in a service provider: - -```php -namespace App\Providers; - -use Illuminate\Support\ServiceProvider; -use Nuwave\Lighthouse\Schema\DirectiveLocator; -use Nuwave\Lighthouse\Schema\Directives\PaginateDirective; - -class GraphQLServiceProvider extends ServiceProvider -{ - public function boot(DirectiveLocator $directiveLocator): void - { - // Use the built-in directive, even though a plugin defines its own @paginate - $directiveLocator->setResolved('paginate', PaginateDirective::class); - } -} -```