Repository: apollographql/apollo-tooling (ts)

Issue:
Schema composition with directives that use complex argument types
Greetings, Apollo team!

Consider the following type definitions that will be used to build an Apollo Federation schema:

```graphql
directive @something(rules: [Rule!]!) on OBJECT | FIELD_DEFINITION

enum Number {
  one
  two
}

input Rule {
  numbers: [Number!]!
}

type Something {
  id: ID!
}

extend type Query {
  getSomething(id: ID!): Something!
    @something(rules: [{ numbers: [one, two] }])
}
```

A single directive called `@something` is declared, that takes in a non-nullable list of GraphQL input object types. This directive can then be used on objects and field definitions to perform some task. An example of a similar directive can be found here: https://docs.amplify.aws/cli/graphql-transformer/auth#auth.

The problem arises during composition of the schema, namely that input type `Rule` is being registered twice, leading to this error: https://github.com/graphql/graphql-js/blob/00eab30fb269b6e2a4e8e61d097d3e249319420e/src/type/schema.js#L223.

I originally posted the issue in `graphql-js` repository: https://github.com/graphql/graphql-js/issues/2845

Upon further investigation, it appears that the actual problem occurs during schema transformation: https://github.com/apollographql/apollo-tooling/blob/dc4fe35134f3108f5c2c0e442dad54a82969398e/packages/apollo-graphql/src/schema/transformSchema.ts#L46

The problem is that `recreateNamedType` creates a new instance of a named type and changes its internal JS object reference. However, `graphql` package insists on that JS references are kept intact during schema composition, since it uses a `Set.has` check to ensure types are not registered twice.

As far as I can see, there is no need to use `recreateNamedType` if type remains the same in https://github.com/apollographql/apollo-tooling/blob/dc4fe35134f3108f5c2c0e442dad54a82969398e/packages/apollo-graphql/src/schema/transformSchema.ts#L46

Perhaps replacing the following code:

```typescript
// Returning `undefined` keeps the old type.
const newType = result || oldType;
typeMap[newType.name] = recreateNamedType(newType);
```

with the following can fix the problem?

```typescript
// Returning `undefined` keeps the old type.
typeMap[oldType.name] = oldType || recreateNamedType(result);
```

At least it seems to work when I change it in my local `node_modules`.

I published an example repository to illustrate the problem: https://github.com/azaxarov/graphql-service-with-complex-directive-parameter

Hope to hear from you guys!

Pull request:
fix(apollo-graphql): support complex directive args
This change fixes `transformSchema`, which previously left directives as-in after transforming types. This meant that Input types used in directive arguments would appear duplicated in the schema. It now recreates schema directives and their arguments like it does with named schema types.

Fixes #2162

<!--
  Thanks for filing a pull request on Apollo Tooling!

  Please look at the following checklist to ensure that your PR
  can be accepted quickly:
-->

TODO:

- [x] Update CHANGELOG.md\* with your change (include reference to issue & this PR)
- [x] Make sure all of the significant new logic is covered by tests
- [x] Rebase your changes on master so that they can be merged easily
- [x] Make sure all tests and linter rules pass

\*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6373b547..3fca6811 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,7 +17,7 @@
 - `apollo-env`
   - <First `apollo-env` related entry goes here>
 - `apollo-graphql`
-  - <First `apollo-graphql` related entry goes here>
+  - Complex directive arguments don't break `transformSchema` (Fixes #2162)
 - `apollo-language-server`
   - <First `apollo-language-server` related entry goes here>
 - `apollo-tools`
diff --git a/packages/apollo-graphql/src/schema/transformSchema.ts b/packages/apollo-graphql/src/schema/transformSchema.ts
index 2132c683..7c83392e 100644
--- a/packages/apollo-graphql/src/schema/transformSchema.ts
+++ b/packages/apollo-graphql/src/schema/transformSchema.ts
@@ -19,7 +19,8 @@ import {
   GraphQLUnionType,
   isInputObjectType,
   GraphQLInputObjectType,
-  GraphQLInputFieldConfigMap
+  GraphQLInputFieldConfigMap,
+  GraphQLDirective
 } from "graphql";
 import { mapValues } from "../utilities/mapValues";
 
@@ -53,7 +54,8 @@ export function transformSchema(
     types: Object.values(typeMap),
     query: replaceMaybeType(schemaConfig.query),
     mutation: replaceMaybeType(schemaConfig.mutation),
-    subscription: replaceMaybeType(schemaConfig.subscription)
+    subscription: replaceMaybeType(schemaConfig.subscription),
+    directives: replaceDirectives(schemaConfig.directives)
   });
 
   function recreateNamedType(type: GraphQLNamedType): GraphQLNamedType {
@@ -145,4 +147,14 @@ export function transformSchema(
       type: replaceType(arg.type)
     }));
   }
+
+  function replaceDirectives(directives: GraphQLDirective[]) {
+    return directives.map(directive => {
+      const config = directive.toConfig();
+      return new GraphQLDirective({
+        ...config,
+        args: replaceArgs(config.args)
+      });
+    });
+  }
 }


Test patch:
diff --git a/packages/apollo-graphql/src/schema/__tests__/transformSchema.test.ts b/packages/apollo-graphql/src/schema/__tests__/transformSchema.test.ts
new file mode 100644
index 00000000..7326b63c
--- /dev/null
+++ b/packages/apollo-graphql/src/schema/__tests__/transformSchema.test.ts
@@ -0,0 +1,23 @@
+import { buildSchema, printSchema } from "graphql";
+import { transformSchema } from "../transformSchema";
+
+describe("transformSchema", () => {
+  test("no-op transform leaves types untouched", () => {
+    const schema = buildSchema(`#graphql
+    type Query {
+      foo: String @test(baz: { bar: "hello" })
+    }
+
+    input DirectiveArg {
+      bar: String
+    }
+
+    # https://github.com/apollographql/apollo-tooling/issues/2162
+    directive @test(baz: DirectiveArg) on FIELD_DEFINITION
+    `);
+
+    const newSchema = transformSchema(schema, namedType => namedType);
+
+    expect(printSchema(newSchema)).toEqual(printSchema(schema));
+  });
+});