buildFederatedSchema bug - errors out on non-scalar directive arguments

[lgandecki]

Apollo federation won't start if I try to use directives like so:

  directive @entity(
    embedded: Boolean
    additionalFields: [AdditionalEntityFields]
  ) on OBJECT

  input AdditionalEntityFields {
    path: String
    type: String
  }

Error: Schema must contain uniquely named types but contains multiple types named "AdditionalEntityFields".

This is a valid schema.

• A short, but descriptive title. The title doesn't need "Apollo" in it.

• The package name and version of Apollo showing the problem.
  "@apollo/federation": "0.19.1"

• If applicable, the last version of Apollo where the problem did not occur.
  No idea.

• The expected behavior.

It should accept a correct schema with non-scalar directive arguments.

• The actual behavior.

/sandbox/node_modules/graphql/type/schema.js:262
      throw new Error("Schema must contain uniquely named types but contains multiple types named \"".concat(namedType.name, "\"."));
      ^

Error: Schema must contain uniquely named types but contains multiple types named "AdditionalEntityFields".
    at typeMapReducer (/sandbox/node_modules/graphql/type/schema.js:262:13)
    at /sandbox/node_modules/graphql/type/schema.js:307:12
    at Array.reduce (<anonymous>)
    at typeMapDirectiveReducer (/sandbox/node_modules/graphql/type/schema.js:306:25)
    at Array.reduce (<anonymous>)
    at new GraphQLSchema (/sandbox/node_modules/graphql/type/schema.js:147:32)
    at Object.transformSchema (/sandbox/node_modules/apollo-graphql/lib/schema/transformSchema.js:17:12)
    at buildFederatedSchema (/sandbox/node_modules/@apollo/federation/dist/service/buildFederatedSchema.js:58:31)
    at Object.<anonymous> (/sandbox/index.js:31:11)
    at Module._compile (internal/modules/cjs/loader.js:778:30)

• A simple, runnable reproduction!
  https://codesandbox.io/s/elated-mirzakhani-wnwkk?file=/index.js:621-702

if you switch true into false on line 28

if (true) {
// ->
if (false) {

you will see that the app starts just fine with the same typeDefs. This has been broken for months now and I hoped it was some easy oversight that would get fixed shortly, but for some reason, this is still broken. I tried to fix it myself but to be honest that piece of code is a rabbit hole of weird transformations. It's really hard to understand WHY all that stuff is happening - and one downside of what happens there is the duplication of some types, like that one.

[lgandecki]

Since I was met with silence here I decided to spend a couple hours trying to debug this.

Please note that this bug prevents the usage of very popular https://graphql-code-generator.com/ with federation.
This is an example where they use an input type on a directive: https://graphql-code-generator.com/docs/plugins/typescript-mongodb#entityadditionalfields-additionalentityfields-on-object

Turns out the problem is indeed related to federation, as the commit that introduced the problem was titled "Build out federation utilities in apollo-graphql". Nonetheless the faulty code is actually in apollo-graphql package.

https://github.com/apollographql/apollo-tooling/blob/master/packages/apollo-graphql/src/schema/transformSchema.ts#L82-L89

You guys create new types, so later when graphql creates a set iterating over the types, it adds the same type twice (same shape, but not the same memory location/reference).

https://github.com/graphql/graphql-js/blob/main/src/type/schema.js#L401

So in the end it ends up having two types with the same name and throws an exception.

If I do a simple change in apollo-graphql

          if (type.toString() === 'AdditionalEntityFields') {
            return type;
          }

(take a look here if it's not clear where I add the code https://github.com/lgandecki/buildFederatedSchemaIssue/blob/main/patches/apollo-graphql%2B0.6.0.patch )

Everything starts working fine.

I'm not sure what was the point of recreating the Types as that's not explained as far as I see, but it is clearly against the idea of what GraphQL is doing when using a set for collecting them. I'm actually surprised this is not causing more issues.
It could be fixed by checking if a given type was already "recreated" and reuse it, instead of creating it twice messing up with GraphQL internal tooling. Or modyfing/extending the types instead of creating new objects based on the existing ones.

Please, help me out here. I could try to fix it but I would really not want to spend hours digging more into this just to be ignored again. Or to learn that my fix was wrong because of some concerns I'm not realizing yet.

you can use this link to reproduce:
https://codesandbox.io/s/elated-mirzakhani-wnwkk

and this repo to play around with it locally.

https://github.com/lgandecki/buildFederatedSchemaIssue/

if you run yarn apply-patch in it it will apply the patch from patches/apollo-graphql+0.6.0.patch and you will be able to run the code without an error.

@abernix is there something you could do for me here? :)

[spencerbull]

@lgandecki I ran into the same problem.

A fix was merged into the apollo-graphql package for the transformSchema function used in the buildFederatedSchema function

federation/federation-js/src/service/buildFederatedSchema.ts

Lines 10 to 17 in c3343b4

	import {
	buildSchemaFromSDL,
	transformSchema,
	GraphQLSchemaModule,
	modulesFromSDL,
	addResolversToSchema,
	GraphQLResolverMap,
	} from 'apollo-graphql';

There was another issue created here apollographql/apollo-tooling#2162 and the merged fix here apollographql/apollo-tooling#2335

Now we just need to wait until the @apollo/federation package is updated with the apollo-graphql package.

[spencerbull]

@lgandecki I can confirm that on @apollo/federation v0.25.1, complex input types on directives are working

[trevor-scheer]

Thank you for the report and apologies for the delay on this @lgandecki. And thanks for reporting back with confirmation of the fix @bullapse!

As @bullapse has pointed out, the fix landed in the apollo-graphql package, which we then updated to and released ~2 weeks ago in @apollo/federation@0.25.1:
#772
5ea2b6a

If you have any further issues with this please let me know, but I'll close this for now.

[lgandecki]

Thank you guys! Great to have this finally solved, I will verify shortly