mercurius: Using fastify-gql in place of Apollo Gateway does not work

I’ve spent more time debugging this, and I have come to the conclusion that my understanding of the issue as expressed in the previous message was incomplete.

Here’s my new understanding:

1. Apollo-server is indeed stripping directives, but only the directive definitions from the implementing service schemas, not the directive usages in extensions. I.e.

    extend type User @key(fields: "id") {
      id: ID! @external
      comments: [Comment]
    }

gets added to the schema as is, still retaining the @key part 2. Apollo-server overcomes the error we are getting by calling extendSchema with the option assumeValidSDL as @flux627 suggested at one point. See it here: https://github.com/apollographql/apollo-server/blob/00887a1e57a26075483ba397098f6d088a706499/packages/apollo-federation/src/composition/compose.ts#L373 3. Apollo-server however add its own validations before extending the schema: https://github.com/apollographql/apollo-server/blob/00887a1e57a26075483ba397098f6d088a706499/packages/apollo-federation/src/composition/compose.ts#L371. It uses a private function from graphql-js: validateSDL. It’s the same function function used internally by validateSchema, but apollo-server replaces the rules it uses

I’m working on a PR to replicate the behaviour by calling extendSchema with assumeValidSDL set to true but also adding the validations.

I don’t have time to look into this, but just wanted to comment on

It is actually expected that service B extending a type from service A also includes the sane @key directive, according to the apollo website: https://www.apollographql.com/docs/apollo-server/federation/entities/#referencing

While this is true that this is expected, that doesn’t mean that it’s valid to attach the same directives to extended types. The way Federation defines how separate services need to attach directives to their own schemas does not dictate what is valid within the same schema.

Let me rephrase that. The Apollo docs clearly state that when extending entities, the extend must include the same @key directive as the original type definition. This is the case for stubs and actual extensions.

The errors occurs in https://github.com/mcollina/fastify-gql/blob/master/lib/federation.js#L253. Apparently graphql’s extendSchema raises an error when it is passed two definitions that extend the same type at the same type.

Based on what I remember when testing, this is not the case. If you have a type with a directive, and extend the type with the same directive, then it will give the error. It makes sense that you’d be getting the error with multiple extensions of the same type with the same directive, for this reason. It’s possible I’m remembering wrong, however.

As far as I can see, this is not the case - or at least it’s not enforced at the graphql-js level. Fastify-gql builds the schema in buildFederationSchema (https://github.com/mcollina/fastify-gql/blob/master/lib/federation.js#L230). In there the concatenation of the schemas from the federated services is parsed and then split in type stubs, extensions and definitions. It then proceed to make a call to extendSchema with each of those three groups.

The error we get in the demo repository (and also in fastify-gql own example in https://github.com/mcollina/fastify-gql/blob/master/examples/gateway-subscription.js) is thrown by the call to extendSchema that tries to add all extensions at the same time. This happens regardless of how many type extensions with the same @key directive have been added beforehand: the problem is just with calling extendSchema with the extend for the same type with the same directive.

To prove that I made this small change https://github.com/mcollina/fastify-gql/pull/221/files#diff-d9185daf57698ed90d572d1966e61b16R253-R258 where extensions are added one at the time and that fixed the issue: a sign that extendSchema is accepting multiple extend with the same key directive if each one happens in their own call to extendSchema

In any case, in order to abide by Apollo’s documentation we still need to strip the directives from the final schema. Since this is necessary, then removing them before extending kills two birds with one stone. It just means we need to capture the configuration as defined by these directives beforehand.

I agree with this. Regardless of what is actually causing the exception we are seeing, the correct implementation would be to replicate the behaviour of Apollo, which is to strip the directives in the federated schema.

I took a look at Apollo’s code and that appears to be happening in https://github.com/apollographql/apollo-server/blob/5bab3aedeeb1e70c636f3aa3ac7a147679f450f7/packages/apollo-federation/src/composition/normalize.ts#L20 (more specifically in https://github.com/apollographql/apollo-server/blob/5bab3aedeeb1e70c636f3aa3ac7a147679f450f7/packages/apollo-federation/src/composition/normalize.ts#L270).

So, to sum up my point:

1. I find the behaviour of parse and extendSchema from graphql-js weird: a schema parsed without errors by parse throws and error in extendSchema, and that also depends on the fact that you make only one call or multiple calls to it. May be worth further investigating into this and report to graphql-js
2. Regardless of that, fastify-gql should replicate Apollo’s behaviour and strip the directives