feat(composition): Relax @tag definition validation

federation-js/src/composition/utils.ts

@@ -32,6 +32,8 @@ import {
   isDirective,
   isNamedType,
   stripIgnoredCharacters,
+  NonNullTypeNode,
+  NamedTypeNode,
 } from 'graphql';
 import {
   ExternalFieldDefinition,
@@ -56,6 +58,14 @@ export function isDirectiveDefinitionNode(node: any): node is DirectiveDefinitio
   return node.kind === Kind.DIRECTIVE_DEFINITION;
 }
 
+export function isNonNullTypeNode(node: any): node is NonNullTypeNode {
+  return node.kind === Kind.NON_NULL_TYPE;
+}
+
+export function isNamedTypeNode(node: any): node is NamedTypeNode {
+  return node.kind === Kind.NAMED_TYPE;
+}
+
 // Create a map of { fieldName: serviceName } for each field.
 export function mapFieldNamesToServiceName<Node extends { name: NameNode }>(
   fields: ReadonlyArray<Node>,

federation-js/src/composition/validate/preNormalization/__tests__/tagDirective.test.ts

@@ -65,6 +65,21 @@ describe('tagDirective', () => {
       const errors = tagDirective(serviceA);
       expect(errors).toHaveLength(0);
     });
+
+    it('permits alternative, compatible @tag definitions', () => {
+      const serviceA = {
+        typeDefs: gql`
+          directive @tag(name: String!) on FIELD_DEFINITION | INTERFACE
+          type Query {
+            hello: String @tag(name: "hello")
+          }
+        `,
+        name: 'serviceA',
+      };
+
+      const errors = tagDirective(serviceA);
+      expect(errors).toHaveLength(0);
+    });
   });
 
   describe('reports errors', () => {
@@ -99,35 +114,130 @@ describe('tagDirective', () => {
       `);
     });
 
-    it('when @tag usage and definition exist, but definition is incorrect', () => {
-      const serviceA = {
-        typeDefs: gql`
-          directive @tag(name: String!) on FIELD_DEFINITION
+    describe('incompatible definition', () => {
+      it('locations incompatible', () => {
+        const serviceA = {
+          typeDefs: gql`
+            directive @tag(name: String!) on FIELD_DEFINITION | SCHEMA
 
-          type Query {
-            hello: String @tag(name: "hello")
-          }
-        `,
-        name: 'serviceA',
-      };
+            type Query {
+              hello: String @tag(name: "hello")
+            }
+          `,
+          name: 'serviceA',
+        };
 
-      const errors = tagDirective(serviceA);
+        const errors = tagDirective(serviceA);
 
-      expect(errors).toMatchInlineSnapshot(`
-        Array [
-          Object {
-            "code": "TAG_DIRECTIVE_DEFINITION_INVALID",
-            "locations": Array [
-              Object {
-                "column": 1,
-                "line": 2,
-              },
-            ],
-            "message": "[@tag] -> Found @tag definition in service serviceA, but the @tag directive definition was invalid. Please ensure the directive definition in your schema's type definitions matches the following:
-        	directive @tag(name: String!) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION",
-          },
-        ]
-      `);
+        expect(errors).toMatchInlineSnapshot(`
+          Array [
+            Object {
+              "code": "TAG_DIRECTIVE_DEFINITION_INVALID",
+              "locations": Array [
+                Object {
+                  "column": 1,
+                  "line": 2,
+                },
+              ],
+              "message": "[@tag] -> Found @tag definition in service serviceA, but the @tag directive definition was invalid. Please ensure the directive definition in your schema's type definitions is compatible with the following:
+          	directive @tag(name: String!) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION",
+            },
+          ]
+        `);
+      });
+
+      it('name arg missing', () => {
+        const serviceA = {
+          typeDefs: gql`
+            directive @tag on FIELD_DEFINITION
+
+            type Query {
+              hello: String @tag
+            }
+          `,
+          name: 'serviceA',
+        };
+
+        const errors = tagDirective(serviceA);
+
+        expect(errors).toMatchInlineSnapshot(`
+          Array [
+            Object {
+              "code": "TAG_DIRECTIVE_DEFINITION_INVALID",
+              "locations": Array [
+                Object {
+                  "column": 1,
+                  "line": 2,
+                },
+              ],
+              "message": "[@tag] -> Found @tag definition in service serviceA, but the @tag directive definition was invalid. Please ensure the directive definition in your schema's type definitions is compatible with the following:
+          	directive @tag(name: String!) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION",
+            },
+          ]
+        `);
+      });
+
+      it('name arg incompatible', () => {
+        const serviceA = {
+          typeDefs: gql`
+            directive @tag(name: String) on FIELD_DEFINITION
+
+            type Query {
+              hello: String @tag(name: "hello")
+            }
+          `,
+          name: 'serviceA',
+        };
+
+        const errors = tagDirective(serviceA);
+
+        expect(errors).toMatchInlineSnapshot(`
+          Array [
+            Object {
+              "code": "TAG_DIRECTIVE_DEFINITION_INVALID",
+              "locations": Array [
+                Object {
+                  "column": 1,
+                  "line": 2,
+                },
+              ],
+              "message": "[@tag] -> Found @tag definition in service serviceA, but the @tag directive definition was invalid. Please ensure the directive definition in your schema's type definitions is compatible with the following:
+          	directive @tag(name: String!) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION",
+            },
+          ]
+        `);
+      });
+
+      it('additional args', () => {
+        const serviceA = {
+          typeDefs: gql`
+            directive @tag(name: String!, additional: String) on FIELD_DEFINITION
+
+            type Query {
+              hello: String @tag(name: "hello")
+            }
+          `,
+          name: 'serviceA',
+        };
+
+        const errors = tagDirective(serviceA);
+
+        expect(errors).toMatchInlineSnapshot(`
+          Array [
+            Object {
+              "code": "TAG_DIRECTIVE_DEFINITION_INVALID",
+              "locations": Array [
+                Object {
+                  "column": 1,
+                  "line": 2,
+                },
+              ],
+              "message": "[@tag] -> Found @tag definition in service serviceA, but the @tag directive definition was invalid. Please ensure the directive definition in your schema's type definitions is compatible with the following:
+          	directive @tag(name: String!) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION",
+            },
+          ]
+        `);
+      });
     });
 
     it('when @tag usage is missing args', () => {

federation-js/src/composition/validate/preNormalization/tagDirective.ts

@@ -1,17 +1,19 @@
-import { federationDirectives } from '../../../directives';
+import {
+  directiveDefinitionsAreCompatible,
+  federationDirectives,
+} from '../../../directives';
 import {
   DirectiveDefinitionNode,
   KnownDirectivesRule,
   visit,
   BREAK,
-  print,
-  NameNode,
+  parse,
 } from 'graphql';
 import { KnownArgumentNamesOnDirectivesRule } from 'graphql/validation/rules/KnownArgumentNamesRule';
 import { ProvidedRequiredArgumentsOnDirectivesRule } from 'graphql/validation/rules/ProvidedRequiredArgumentsRule';
 import { validateSDL } from 'graphql/validation/validate';
 import { ServiceDefinition } from '../../types';
-import { errorWithCode, logDirective, stripDescriptions } from '../../utils';
+import { errorWithCode, logDirective } from '../../utils';
 
 // Likely brittle but also will be very obvious if this breaks. Based on the
 // content of the error message itself to remove expected errors related to
@@ -47,36 +49,30 @@ export const tagDirective = ({
     },
   });
 
-  // Ensure the tag directive definition is correct
-  if (tagDirectiveDefinition) {
-    const printedTagDefinition =
-      'directive @tag(name: String!) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION';
+  const printedTagDefinition =
+    'directive @tag(name: String!) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION';
+  const parsedTagDefinition = parse(printedTagDefinition)
+    .definitions[0] as DirectiveDefinitionNode;
 
-    if (
-      print(normalizeDirectiveDefinitionNode(tagDirectiveDefinition)) !==
-      printedTagDefinition
-    ) {
-      errors.push(
-        errorWithCode(
-          'TAG_DIRECTIVE_DEFINITION_INVALID',
-          logDirective('tag') +
-            `Found @tag definition in service ${serviceName}, but the @tag directive definition was invalid. Please ensure the directive definition in your schema's type definitions matches the following:\n\t${printedTagDefinition}`,
-          tagDirectiveDefinition,
-        ),
-      );
-    }
+  if (
+    tagDirectiveDefinition &&
+    !directiveDefinitionsAreCompatible(
+      parsedTagDefinition,
+      tagDirectiveDefinition,
+    )
+  ) {
+    errors.push(
+      errorWithCode(
+        'TAG_DIRECTIVE_DEFINITION_INVALID',
+        logDirective('tag') +
+          `Found @tag definition in service ${serviceName}, but the @tag directive definition was invalid. Please ensure the directive definition in your schema's type definitions is compatible with the following:\n\t${printedTagDefinition}`,
+        tagDirectiveDefinition,
+      ),
+    );
   }
 
   return errors.filter(
     ({ message }) =>
       !errorsMessagesToFilter.some((keyWord) => message === keyWord),
   );
 };
-
-function normalizeDirectiveDefinitionNode(node: DirectiveDefinitionNode) {
-  // Remove descriptions from the AST
-  node = stripDescriptions(node);
-  // Sort locations alphabetically
-  (node.locations as NameNode[]).sort((a, b) => a.value.localeCompare(b.value));
-  return node;
-}

federation-js/src/directives.ts

@@ -14,7 +14,10 @@ import {
   TypeSystemExtensionNode,
   TypeDefinitionNode,
   ExecutableDefinitionNode,
+  DirectiveDefinitionNode,
+  print,
 } from 'graphql';
+import { stripDescriptions } from './composition/utils';
 
 export const KeyDirective = new GraphQLDirective({
   name: 'key',
@@ -135,5 +138,40 @@ export function typeIncludesDirective(
 ): boolean {
   if (isInputObjectType(type)) return false;
   const directives = gatherDirectives(type as GraphQLNamedTypeWithDirectives);
-  return directives.some(directive => directive.name.value === directiveName);
+  return directives.some((directive) => directive.name.value === directiveName);
+}
+
+export function directiveDefinitionsAreCompatible(
+  baseDefinition: DirectiveDefinitionNode,
+  toCompare: DirectiveDefinitionNode,
+) {
+  if (baseDefinition.name.value !== toCompare.name.value) return false;
+  // arguments must be equal in length
+  if (baseDefinition.arguments?.length !== toCompare.arguments?.length) {
+    return false;
+  }
+  // arguments must be equal in type
+  for (const arg of baseDefinition.arguments ?? []) {
+    const toCompareArg = toCompare.arguments?.find(
+      (a) => a.name.value === arg.name.value,
+    );
+    if (!toCompareArg) return false;
+    if (
+      print(stripDescriptions(arg)) !== print(stripDescriptions(toCompareArg))
+    ) {
+      return false;
+    }
+  }
+  // toCompare's locations must exist in baseDefinition's locations
+  if (
+    toCompare.locations.some(
+      (location) =>
+        !baseDefinition.locations.find(
+          (baseLocation) => baseLocation.value === location.value,
+        ),
+    )
+  ) {
+    return false;
+  }
+  return true;
 }