Add Node interface as soon as a type uses it (#2387)

nuwave · Apr 17, 2023 · 94e1e9a · 94e1e9a
1 parent d4ea07d
commit 94e1e9a
Show file tree

Hide file tree

Showing 13 changed files with 122 additions and 144 deletions.
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
@@ -1,9 +1,9 @@
+<!-- Link to related issues this PR resolves, e.g. "Resolves #236"-->
+
 - [ ] Added or updated tests
 - [ ] Documented user facing changes
 - [ ] Updated CHANGELOG.md
 
-<!-- Link to related issues this PR resolves, e.g. "Resolves #236"-->
-
 **Changes**
 
 <!-- Detail the changes in behaviour this PR introduces. -->

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,12 @@ You can find and compare releases at the [GitHub release page](https://github.co
 
 ## Unreleased
 
+## v6.6.1
+
+### Fixed
+
+- Add `Node` interface as soon as a type uses it with `@node` https://github.com/nuwave/lighthouse/pull/2387
+
 ## v6.6.0
 
 ### Fixed

diff --git a/docs/4/subscriptions/client-implementations.md b/docs/4/subscriptions/client-implementations.md
@@ -25,9 +25,8 @@ class PusherLink extends ApolloLink {
     });
 
     // Capture the super method
-    const prevSubscribe = subscribeObservable.subscribe.bind(
-      subscribeObservable
-    );
+    const prevSubscribe =
+      subscribeObservable.subscribe.bind(subscribeObservable);
 
     // Override subscribe to return an `unsubscribe` object, see
     // https://github.com/apollographql/subscriptions-transport-ws/blob/master/src/client.ts#L182-L212

diff --git a/docs/5/subscriptions/client-implementations.md b/docs/5/subscriptions/client-implementations.md
@@ -25,9 +25,8 @@ class PusherLink extends ApolloLink {
     });
 
     // Capture the super method
-    const prevSubscribe = subscribeObservable.subscribe.bind(
-      subscribeObservable
-    );
+    const prevSubscribe =
+      subscribeObservable.subscribe.bind(subscribeObservable);
 
     // Override subscribe to return an `unsubscribe` object, see
     // https://github.com/apollographql/subscriptions-transport-ws/blob/master/src/client.ts#L182-L212

diff --git a/docs/6/subscriptions/client-implementations.md b/docs/6/subscriptions/client-implementations.md
@@ -25,9 +25,8 @@ class PusherLink extends ApolloLink {
     });
 
     // Capture the super method
-    const prevSubscribe = subscribeObservable.subscribe.bind(
-      subscribeObservable
-    );
+    const prevSubscribe =
+      subscribeObservable.subscribe.bind(subscribeObservable);
 
     // Override subscribe to return an `unsubscribe` object, see
     // https://github.com/apollographql/subscriptions-transport-ws/blob/master/src/client.ts#L182-L212

diff --git a/docs/master/subscriptions/client-implementations.md b/docs/master/subscriptions/client-implementations.md
@@ -25,9 +25,8 @@ class PusherLink extends ApolloLink {
     });
 
     // Capture the super method
-    const prevSubscribe = subscribeObservable.subscribe.bind(
-      subscribeObservable
-    );
+    const prevSubscribe =
+      subscribeObservable.subscribe.bind(subscribeObservable);
 
     // Override subscribe to return an `unsubscribe` object, see
     // https://github.com/apollographql/subscriptions-transport-ws/blob/master/src/client.ts#L182-L212

diff --git a/src/Federation/ASTManipulator.php b/src/Federation/ASTManipulator.php
@@ -5,6 +5,10 @@
 use GraphQL\Language\AST\ObjectTypeDefinitionNode;
 use GraphQL\Language\Parser;
 use Nuwave\Lighthouse\Events\ManipulateAST;
+use Nuwave\Lighthouse\Federation\Resolvers\Entities;
+use Nuwave\Lighthouse\Federation\Resolvers\Service;
+use Nuwave\Lighthouse\Federation\Types\Any;
+use Nuwave\Lighthouse\Federation\Types\FieldSet;
 use Nuwave\Lighthouse\Schema\AST\DocumentAST;
 use Nuwave\Lighthouse\Schema\RootType;
 
@@ -22,17 +26,15 @@ public function handle(ManipulateAST $manipulateAST): void
 
     protected function addScalars(DocumentAST &$documentAST): void
     {
-        $documentAST->setTypeDefinition(
-            Parser::scalarTypeDefinition(/** @lang GraphQL */ '
-            scalar _Any @scalar(class: "Nuwave\\\Lighthouse\\\Federation\\\Types\\\Any")
-            '),
-        );
-
-        $documentAST->setTypeDefinition(
-            Parser::scalarTypeDefinition(/** @lang GraphQL */ '
-            scalar _FieldSet @scalar(class: "Nuwave\\\Lighthouse\\\Federation\\\Types\\\FieldSet")
-            '),
-        );
+        $anyClass = addslashes(Any::class);
+        $documentAST->setTypeDefinition(Parser::scalarTypeDefinition(/** @lang GraphQL */ <<<GRAPHQL
+            scalar _Any @scalar(class: "{$anyClass}")
+        GRAPHQL));
+
+        $fieldSetClass = addslashes(FieldSet::class);
+        $documentAST->setTypeDefinition(Parser::scalarTypeDefinition(/** @lang GraphQL */ <<<GRAPHQL
+            scalar _FieldSet @scalar(class: "{$fieldSetClass}")
+        GRAPHQL));
     }
 
     /** Combine object types with @key into the _Entity union. */
@@ -68,24 +70,24 @@ protected function addEntityUnion(DocumentAST &$documentAST): void
 
     protected function addRootFields(DocumentAST &$documentAST): void
     {
-        // In federation it is fine for a schema to not have a user-defined root query type,
+        // In federation, it is fine for a schema to not have a user-defined root query type,
         // since we add two federation related fields to it here.
-        if (! isset($documentAST->types[RootType::QUERY])) {
-            $documentAST->types[RootType::QUERY] = Parser::objectTypeDefinition(/** @lang GraphQL */ 'type Query');
-        }
+        $documentAST->types[RootType::QUERY] ??= Parser::objectTypeDefinition(/** @lang GraphQL */ 'type Query');
 
         $queryType = $documentAST->types[RootType::QUERY];
         assert($queryType instanceof ObjectTypeDefinitionNode);
 
-        $queryType->fields[] = Parser::fieldDefinition(/** @lang GraphQL */ '
-        _entities(
-            representations: [_Any!]!
-        ): [_Entity]! @field(resolver: "Nuwave\\\Lighthouse\\\Federation\\\Resolvers\\\Entities")
-        ');
-
-        $queryType->fields[] = Parser::fieldDefinition(/** @lang GraphQL */ '
-        _service: _Service! @field(resolver: "Nuwave\\\Lighthouse\\\Federation\\\Resolvers\\\Service")
-        ');
+        $entitiesClass = addslashes(Entities::class);
+        $queryType->fields[] = Parser::fieldDefinition(/** @lang GraphQL */ <<<GRAPHQL
+            _entities(
+                representations: [_Any!]!
+            ): [_Entity]! @field(resolver: "{$entitiesClass}")
+        GRAPHQL);
+
+        $serviceClass = addslashes(Service::class);
+        $queryType->fields[] = Parser::fieldDefinition(/** @lang GraphQL */ <<<GRAPHQL
+           _service: _Service! @field(resolver: "{$serviceClass}")
+        GRAPHQL);
     }
 
     protected function addServiceType(DocumentAST &$documentAST): void

diff --git a/src/GlobalId/GlobalIdServiceProvider.php b/src/GlobalId/GlobalIdServiceProvider.php
@@ -2,20 +2,12 @@
 
 namespace Nuwave\Lighthouse\GlobalId;
 
-use GraphQL\Language\AST\ObjectTypeDefinitionNode;
-use GraphQL\Language\Parser;
 use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Support\ServiceProvider;
-use Nuwave\Lighthouse\Events\ManipulateAST;
 use Nuwave\Lighthouse\Events\RegisterDirectiveNamespaces;
-use Nuwave\Lighthouse\Schema\AST\ASTHelper;
-use Nuwave\Lighthouse\Schema\AST\DocumentAST;
-use Nuwave\Lighthouse\Schema\RootType;
 
 class GlobalIdServiceProvider extends ServiceProvider
 {
-    public const NODE = 'Node';
-
     public function register(): void
     {
         $this->app->bind(GlobalId::class, Base64GlobalId::class);
@@ -25,55 +17,5 @@ public function register(): void
     public function boot(Dispatcher $dispatcher): void
     {
         $dispatcher->listen(RegisterDirectiveNamespaces::class, static fn (): string => __NAMESPACE__);
-        $dispatcher->listen(ManipulateAST::class, function (ManipulateAST $manipulateAST): void {
-            $documentAST = $manipulateAST->documentAST;
-
-            // Only add the node type and node field if a type actually implements them.
-            // If we were to add it regardless, a validation error is thrown because an
-            // interface without implementations is pointless to have in the schema.
-            if ($this->hasTypeImplementingNodeInterface($documentAST)) {
-                $this->addNodeSupport($documentAST);
-            }
-        });
-    }
-
-    protected function addNodeSupport(DocumentAST $documentAST): void
-    {
-        $node = self::NODE;
-        $globalId = config('lighthouse.global_id_field');
-
-        // Double slashes to escape the slashes in the namespace.
-        $documentAST->setTypeDefinition(
-            Parser::interfaceTypeDefinition(/** @lang GraphQL */ <<<GRAPHQL
-"Any object implementing this type can be found by ID through `Query.node`."
-interface {$node} @interface(resolveType: "Nuwave\\\Lighthouse\\\GlobalId\\\NodeRegistry@resolveType") {
-  "Global identifier that can be used to resolve any Node implementation."
-  {$globalId}: ID!
-}
-GRAPHQL
-            ),
-        );
-
-        $queryType = $documentAST->types[RootType::QUERY];
-        assert($queryType instanceof ObjectTypeDefinitionNode);
-
-        $queryType->fields[] = Parser::fieldDefinition(/** @lang GraphQL */ <<<'GRAPHQL'
-  node(id: ID! @globalId): Node @field(resolver: "Nuwave\\Lighthouse\\GlobalId\\NodeRegistry@resolve")
-GRAPHQL
-        );
-    }
-
-    protected function hasTypeImplementingNodeInterface(DocumentAST $documentAST): bool
-    {
-        foreach ($documentAST->types as $typeDefinition) {
-            if (
-                $typeDefinition instanceof ObjectTypeDefinitionNode
-                && ASTHelper::typeImplementsInterface($typeDefinition, self::NODE)
-            ) {
-                return true;
-            }
-        }
-
-        return false;
     }
 }
diff --git a/src/GlobalId/NodeDirective.php b/src/GlobalId/NodeDirective.php
@@ -6,18 +6,23 @@
 use GraphQL\Language\AST\ObjectTypeDefinitionNode;
 use GraphQL\Language\AST\TypeDefinitionNode;
 use GraphQL\Language\Parser;
+use Illuminate\Contracts\Config\Repository as ConfigRepository;
 use Illuminate\Database\Eloquent\Model;
 use Nuwave\Lighthouse\Exceptions\DefinitionException;
 use Nuwave\Lighthouse\Schema\AST\DocumentAST;
 use Nuwave\Lighthouse\Schema\Directives\BaseDirective;
+use Nuwave\Lighthouse\Schema\RootType;
 use Nuwave\Lighthouse\Schema\Values\TypeValue;
 use Nuwave\Lighthouse\Support\Contracts\TypeManipulator;
 use Nuwave\Lighthouse\Support\Contracts\TypeMiddleware;
 
 class NodeDirective extends BaseDirective implements TypeMiddleware, TypeManipulator
 {
+    public const NODE_INTERFACE_NAME = 'Node';
+
     public function __construct(
         protected NodeRegistry $nodeRegistry,
+        protected ConfigRepository $config,
     ) {}
 
     public static function definition(): string
@@ -74,11 +79,35 @@ public function manipulateTypeDefinition(DocumentAST &$documentAST, TypeDefiniti
             throw new DefinitionException("The {$this->name()} directive must only be used on object type definitions, not on {$typeDefinition->kind} {$typeDefinition->getName()->value}.");
         }
 
-        $namedTypeNode = Parser::parseType(GlobalIdServiceProvider::NODE, ['noLocation' => true]);
+        $namedTypeNode = Parser::parseType(self::NODE_INTERFACE_NAME, ['noLocation' => true]);
         assert($namedTypeNode instanceof NamedTypeNode);
         $typeDefinition->interfaces[] = $namedTypeNode;
 
-        $globalIdFieldName = config('lighthouse.global_id_field');
+        $globalIdFieldName = $this->config->get('lighthouse.global_id_field');
         $typeDefinition->fields[] = Parser::fieldDefinition(/** @lang GraphQL */ "{$globalIdFieldName}: ID! @globalId");
+
+        if (! isset($documentAST->types[self::NODE_INTERFACE_NAME])) {
+            $nodeInterfaceName = self::NODE_INTERFACE_NAME;
+            $nodeRegistryClass = addslashes(NodeRegistry::class);
+
+            $documentAST->setTypeDefinition(
+                Parser::interfaceTypeDefinition(/** @lang GraphQL */ <<<GRAPHQL
+                "Any object implementing this type can be found by ID through `Query.node`."
+                interface {$nodeInterfaceName} @interface(resolveType: "{$nodeRegistryClass}@resolveType") {
+                  "Global identifier that can be used to resolve any Node implementation."
+                  {$globalIdFieldName}: ID!
+                }
+                GRAPHQL
+                ),
+            );
+
+            $queryType = $documentAST->types[RootType::QUERY];
+            assert($queryType instanceof ObjectTypeDefinitionNode);
+
+            $queryType->fields[] = Parser::fieldDefinition(/** @lang GraphQL */ <<<GRAPHQL
+              node(id: ID! @globalId): Node @field(resolver: "{$nodeRegistryClass}@resolve")
+            GRAPHQL
+            );
+        }
     }
 }
diff --git a/src/Pagination/PaginationManipulator.php b/src/Pagination/PaginationManipulator.php
@@ -82,16 +82,15 @@ protected function registerConnection(
             $connectionTypeName = "{$fieldTypeName}Connection";
         }
 
-        $connectionFieldName = addslashes(ConnectionField::class);
-
+        $connectionFieldClass = addslashes(ConnectionField::class);
         $connectionType = Parser::objectTypeDefinition(/** @lang GraphQL */ <<<GRAPHQL
             "A paginated list of {$fieldTypeName} edges."
             type {$connectionTypeName} {
                 "Pagination information about the list of edges."
-                {$paginationType->infoFieldName()}: PageInfo! @field(resolver: "{$connectionFieldName}@pageInfoResolver")
+                {$paginationType->infoFieldName()}: PageInfo! @field(resolver: "{$connectionFieldClass}@pageInfoResolver")
 
                 "A list of {$fieldTypeName} edges."
-                edges: [{$connectionEdgeName}!]! @field(resolver: "{$connectionFieldName}@edgeResolver")
+                edges: [{$connectionEdgeName}!]! @field(resolver: "{$connectionFieldClass}@edgeResolver")
             }
 GRAPHQL
         );
@@ -133,9 +132,7 @@ protected function addPaginationWrapperType(ObjectTypeDefinitionNode $objectType
         $existingType = $this->documentAST->types[$typeName] ?? null;
         if ($existingType !== null) {
             if (! $existingType instanceof ObjectTypeDefinitionNode) {
-                throw new DefinitionException(
-                    "Expected object type for pagination wrapper {$typeName}, found {$objectType->kind} instead.",
-                );
+                throw new DefinitionException("Expected object type for pagination wrapper {$typeName}, found {$objectType->kind} instead.");
             }
 
             $objectType = $existingType;
@@ -145,7 +142,10 @@ protected function addPaginationWrapperType(ObjectTypeDefinitionNode $objectType
             isset($this->modelClass)
             && ! ASTHelper::hasDirective($objectType, ModelDirective::NAME)
         ) {
-            $objectType->directives[] = Parser::constDirective(/** @lang GraphQL */ '@model(class: "' . addslashes($this->modelClass) . '")');
+            $modelClassEscaped = addslashes($this->modelClass);
+            $objectType->directives[] = Parser::constDirective(/** @lang GraphQL */ <<<GRAPHQL
+                @model(class: "{$modelClassEscaped}")
+            GRAPHQL);
         }
 
         $this->documentAST->setTypeDefinition($objectType);
@@ -171,18 +171,16 @@ protected function registerPaginator(
                 "A list of {$fieldTypeName} items."
                 data: [{$fieldTypeName}!]! @field(resolver: "{$paginatorFieldClassName}@dataResolver")
             }
-GRAPHQL
-        );
+        GRAPHQL);
         $this->addPaginationWrapperType($paginatorType);
 
         $fieldDefinition->arguments[] = Parser::inputValueDefinition(
             self::countArgument($defaultCount, $maxCount),
         );
         $fieldDefinition->arguments[] = Parser::inputValueDefinition(/** @lang GraphQL */ <<<'GRAPHQL'
-"The offset from which items are returned."
-page: Int
-GRAPHQL
-        );
+            "The offset from which items are returned."
+            page: Int
+        GRAPHQL);
 
         $fieldDefinition->type = $this->paginationResultType($paginatorTypeName);
         $parentType->fields = ASTHelper::mergeUniqueNodeList($parentType->fields, [$fieldDefinition], true);
@@ -208,18 +206,16 @@ protected function registerSimplePaginator(
                 "A list of {$fieldTypeName} items."
                 data: [{$fieldTypeName}!]! @field(resolver: "{$paginatorFieldClassName}@dataResolver")
             }
-GRAPHQL
-        );
+        GRAPHQL);
         $this->addPaginationWrapperType($paginatorType);
 
         $fieldDefinition->arguments[] = Parser::inputValueDefinition(
             self::countArgument($defaultCount, $maxCount),
         );
         $fieldDefinition->arguments[] = Parser::inputValueDefinition(/** @lang GraphQL */ <<<'GRAPHQL'
-"The offset from which items are returned."
-page: Int
-GRAPHQL
-        );
+            "The offset from which items are returned."
+            page: Int
+        GRAPHQL);
 
         $fieldDefinition->type = $this->paginationResultType($paginatorTypeName);
         $parentType->fields = ASTHelper::mergeUniqueNodeList($parentType->fields, [$fieldDefinition], true);

diff --git a/src/Validation/ValidatorDirective.php b/src/Validation/ValidatorDirective.php
@@ -120,9 +120,12 @@ protected function setFullClassnameOnDirective(Node &$definition, string $classC
         // @phpstan-ignore-next-line The passed in Node types all have the property $directives
         foreach ($definition->directives as $directive) {
             if ($directive->name->value === $this->name()) {
+                $validatorClassEscaped = addslashes($validatorClass);
                 $directive->arguments = ASTHelper::mergeUniqueNodeList(
                     $directive->arguments,
-                    [Parser::argument('class: "' . addslashes($validatorClass) . '"')],
+                    [Parser::argument(/** @lang GraphQL */ <<<GRAPHQL
+                        class: "{$validatorClassEscaped}"
+                    GRAPHQL)],
                     true,
                 );
             }