Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot compose schema that's been through buildSubgraphSchema + printSubgraphSchema #1824

Closed
1 of 2 tasks
benweatherman opened this issue May 3, 2022 · 4 comments
Closed
1 of 2 tasks
Assignees
Labels
Milestone

Comments

@benweatherman
Copy link
Contributor

benweatherman commented May 3, 2022

The following script generates the supergraph schema and errors mentioned below. I'd expect this to compose without errors.

import gql from 'graphql-tag';
import { composeServices } from '@apollo/composition';
import { buildSubgraphSchema, printSubgraphSchema } from '@apollo/subgraph';

const typeDefs = gql`
  extend schema
    @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key", "@shareable"])

  type Greeting @key(fields: "name") {
    message: String! @shareable
    name: String!
  }

  type Query {
    hello(name: String!): Greeting!
  }
`;

const schema = buildSubgraphSchema({ typeDefs });

const subgraphSchemaText = printSubgraphSchema(schema);
console.log(subgraphSchemaText);

const service = {
  name: 'service',
  typeDefs: gql`
    ${subgraphSchemaText}
  `,
};
const { supergraphSdl, errors, hints } = composeServices([service]);
console.warn(hints);
for (const error of errors ?? []) {
  console.error(error.message);
}

Output of buildSubgraphSchema + printSubgraphSchema

Slightly modified for brevity

extend schema @link(url: "https://specs.apollo.dev/link/v1.0") @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key", "@shareable"])

directive @federation__requires(fields: federation__FieldSet!) on FIELD_DEFINITION
directive @federation__provides(fields: federation__FieldSet!) on FIELD_DEFINITION
directive @federation__external(reason: String) on OBJECT | FIELD_DEFINITION
directive @federation__tag(name: String!) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
directive @federation__extends on OBJECT | INTERFACE
directive @federation__inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
directive @federation__override(from: String!) on FIELD_DEFINITION

type Greeting @key(fields: "name") {
  message: String! @shareable
  name: String!
}

type Query {
  hello(name: String!): Greeting!
  _entities(representations: [_Any!]!): [_Entity]!
  _service: _Service!
}

enum link__Purpose {
  SECURITY
  EXECUTION
}

scalar federation__FieldSet

Errors

[service] Invalid definition of type Purpose: expected values [EXECUTION,SECURITY] but found [].
[service] Invalid definition of type Purpose: expected values [EXECUTION,SECURITY] but found [].
[service] Unknown directive "@shareable". If you meant the "@shareable" federation 2 directive, note that this schema is a federation 1 schema. To be a federation 2 schema, it needs to @link to the federation specifcation v2.

Acceptance criteria

  • printSubgraphSchema should output something very similar to the input schema
  • printSubgraphSchema is deprecated (this could be a new ticket)

Implementation notes

  • Should add a Schema.from(schema: GraphQLJSSchema)
  • printSubgraphSchema should utilize the above function to output Schema.from(schema).toString()
  • Add type definitions earlier in the process of buildSchema (this would fix the enum errors)
  • Don't output _Entities, _entities, _Service, or _service by default

Reported via apollographql/rover#1115

@split
Copy link

split commented May 4, 2022

Thank you @benweatherman for adding issue about this! 🎉 I were planning to report this also here, but had not yet collected information what exactly is the issue.

One thing that I noticed when not outputting _Entities and _Services types is that corresponding _entities and _service Query fields are kept. When those keep referencing to those filtered out types, it prevents using output schema in tools like GraphQL codegen. Those would create types for resolvers that point to nothing, causing resulting provider types to break. Using schema directly from buildSubgraphSchema has similar issues.

Couple options came to my mind:

  1. If _Entities and _Services are not in the output, then also _entities and _service fields (and input types of those) could be removed from it. All the needed information should already be in the federation directives and those fields only reference undefined input and output types. Hard to see any benefit of having those defined
  2. If _service and _entities fields are kept in the output then maybe _Entities, _Services and input types those are referencing should also stay to keep schema complete. This would be more aligned with output that rover introspection provides

One option would be to define options to pick behaviour, but if that is not option I would prefer option 1.

Thanks again for looking into this 🙌 😄

@benweatherman
Copy link
Contributor Author

Thanks for all that info and appreciate the thoughtful consideration of those options. We plan on implementing the first option you suggested (don't output _entities or _service), but I left that detail out. I've updated the comment now to include all those fields. Does that line up with what you were thinking?

@split
Copy link

split commented May 4, 2022

Awesome. Yes. That sounds perfect

@pcmanus
Copy link
Contributor

pcmanus commented May 5, 2022

I've push fixes for this on #1831.

But I'll note that imo the higher level issue here is that the output of printSubgraphSchema is currently (for fed 2) kind of a mess. The fact that the output of it cannot be fed back to composition (or back into buildSubgraphSchema) is a consequence of that, and it's definitively important to fix it, but it begs the question of "what does printSubgraphSchema is even supposed to do?".

And to clarify what I mean by "kind of a mess", as can be seen in the description, the output adds a bunch of definitions (directives and types) that aren't even used, but it's not a full schema either because the directives that are used (@link, @key and @shareable) don't have their definition. And yes, the fact it shows _entities and _service but not the types they use is also odd and hard to justify consistency wise.

So the attached PR makes it so that printSubgraphSchema essentially output the input schema, but without the federation "cruft" that both buildSubgraphSchema and compose knows how to add. That is, given a schema like the one in the description, that is:

const typeDefs = gql`
  extend schema
    @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key", "@shareable"])

  type Greeting @key(fields: "name") {
    message: String! @shareable
    name: String!
  }

  type Query {
    hello(name: String!): Greeting!
  }
`;

then printSubgraphSchema(buildSubgraphSchema({typeDefs})) simply output back the user input schema (modulo a few formatting difference in that specific case).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants