Strip all Type System Directives during composition

packages/apollo-federation/CHANGELOG.md

@@ -4,6 +4,8 @@
 
 > The changes noted within this `vNEXT` section have not been released yet.  New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development.  When a release is being prepared, a new header will be (manually) created below and the the appropriate changes within that release will be moved into the new section.
 
+* Strip all Type System Directives during composition [#3736](https://github.com/apollographql/apollo-server/pull/3736)
+
 # v0.11.1
 
 > [See complete versioning details.](https://github.com/apollographql/apollo-server/commit/2a4c654986a158aaccf947ee56a4bfc48a3173c7)

packages/apollo-federation/src/composition/__tests__/composeAndValidate.test.ts

@@ -329,11 +329,11 @@ describe('composition of value types', () => {
         `union CatalogItem = Couch | Mattress`,
       );
       expect(schema.getType('Couch')).toMatchInlineSnapshot(`
-      type Couch {
-        sku: ID!
-        material: String!
-      }
-    `);
+              type Couch {
+                sku: ID!
+                material: String!
+              }
+          `);
     });
 
     it('input types', () => {
@@ -345,11 +345,11 @@ describe('composition of value types', () => {
       `);
       expect(errors).toHaveLength(0);
       expect(schema.getType('NewProductInput')).toMatchInlineSnapshot(`
-      input NewProductInput {
-        sku: ID!
-        type: String
-      }
-    `);
+              input NewProductInput {
+                sku: ID!
+                type: String
+              }
+          `);
     });
 
     it('interfaces', () => {
@@ -360,10 +360,10 @@ describe('composition of value types', () => {
       `);
       expect(errors).toHaveLength(0);
       expect(schema.getType('Product')).toMatchInlineSnapshot(`
-      interface Product {
-        sku: ID!
-      }
-    `);
+              interface Product {
+                sku: ID!
+              }
+          `);
     });
 
     it('enums', () => {
@@ -375,11 +375,11 @@ describe('composition of value types', () => {
       `);
       expect(errors).toHaveLength(0);
       expect(schema.getType('CatalogItemEnum')).toMatchInlineSnapshot(`
-      enum CatalogItemEnum {
-        COUCH
-        MATTRESS
-      }
-    `);
+              enum CatalogItemEnum {
+                COUCH
+                MATTRESS
+              }
+          `);
     });
   });
 
@@ -551,3 +551,76 @@ describe('composition of value types', () => {
     });
   });
 });
+
+describe('composition of schemas with directives', () => {
+  /**
+   * To see which usage sites indicate whether a directive is "executable" or
+   * merely for use by the type-system ("type-system"), see the GraphQL spec:
+   * https://graphql.github.io/graphql-spec/June2018/#sec-Type-System.Directives
+   */
+  it('preserves executable and purges type-system directives', () => {
+    const serviceA = {
+      typeDefs: gql`
+        "directives at FIELDs are executable"
+        directive @audit(risk: Int!) on FIELD
+
+        "directives at FIELD_DEFINITIONs are for the type-system"
+        directive @transparency(concealment: Int!) on FIELD_DEFINITION
+
+        type EarthConcern {
+          environmental: String! @transparency(concealment: 5)
+        }
+
+        extend type Query {
+          importantDirectives: [EarthConcern!]!
+        }
+      `,
+      name: 'serviceA',
+    };
+
+    const serviceB = {
+      typeDefs: gql`
+        "directives at FIELDs are executable"
+        directive @audit(risk: Int!) on FIELD
+
+        "directives at FIELD_DEFINITIONs are for the type-system"
+        directive @transparency(concealment: Int!) on FIELD_DEFINITION
+
+        "directives at OBJECTs are for the type-system"
+        directive @experimental on OBJECT
+
+        extend type EarthConcern @experimental {
+          societal: String! @transparency(concealment: 6)
+        }
+      `,
+      name: 'serviceB',
+    };
+
+    const { schema, errors } = composeAndValidate([serviceA, serviceB]);
+    expect(errors).toHaveLength(0);
+
+    const audit = schema.getDirective('audit');
+    expect(audit).toMatchInlineSnapshot(`"@audit"`);
+
+    const transparency = schema.getDirective('transparency');
+    expect(transparency).toBeUndefined();
+
+    const type = schema.getType('EarthConcern') as GraphQLObjectType;
+
+    expect(type.astNode).toMatchInlineSnapshot(`
+      type EarthConcern {
+        environmental: String!
+      }
+    `);
+
+    const fields = type.getFields();
+
+    expect(fields['environmental'].astNode).toMatchInlineSnapshot(
+      `environmental: String!`,
+    );
+
+    expect(fields['societal'].astNode).toMatchInlineSnapshot(
+      `societal: String!`,
+    );
+  });
+});

packages/apollo-federation/src/composition/compose.ts

@@ -29,6 +29,7 @@ import {
   mapValues,
   isFederationDirective,
   executableDirectiveLocations,
+  stripTypeSystemDirectivesFromTypeDefs,
 } from './utils';
 import {
   ServiceDefinition,
@@ -135,7 +136,14 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
 
     externalFields.push(...strippedFields);
 
-    for (let definition of typeDefsWithoutExternalFields.definitions) {
+    // Type system directives from downstream services are not a concern of the
+    // gateway, but rather the services on which the fields live which serve
+    // those types.  In other words, its up to an implementing service to
+    // act on such directives, not the gateway.
+    const typeDefsWithoutTypeSystemDirectives =
+      stripTypeSystemDirectivesFromTypeDefs(typeDefsWithoutExternalFields);
+
+    for (const definition of typeDefsWithoutTypeSystemDirectives.definitions) {
       if (
         definition.kind === Kind.OBJECT_TYPE_DEFINITION ||
         definition.kind === Kind.OBJECT_TYPE_EXTENSION

packages/apollo-federation/src/composition/utils.ts

@@ -83,6 +83,20 @@ export function stripExternalFieldsFromTypeDefs(
   return { typeDefsWithoutExternalFields, strippedFields };
 }
 
+export function stripTypeSystemDirectivesFromTypeDefs(typeDefs: DocumentNode) {
+  const typeDefsWithoutTypeSystemDirectives = visit(typeDefs, {
+    Directive(node) {
+      const isFederationDirective = federationDirectives.some(
+        ({ name }) => name === node.name.value,
+      );
+      // Returning `null` to a visit will cause it to be removed from the tree.
+      return isFederationDirective ? undefined : null;
+    },
+  }) as DocumentNode;
+
+  return typeDefsWithoutTypeSystemDirectives;
+}
+
 /**
  * Returns a closure that strips fields marked with `@external` and adds them
  * to an array.