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

Directives missing in the autogenerated GraphQL Schema if using code-first approach (annotations inside the model are ignored) #1597

Closed
keadex opened this issue Jun 23, 2021 · 5 comments

Comments

@keadex
Copy link

keadex commented Jun 23, 2021

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

I'm trying to build multiple GraphQL Microservices (Experience Service and Company Service) by using Apollo Federation and code-first approach.
I've followed this documentation: https://docs.nestjs.com/graphql/federation#federation

Basically:

  • inside the Experience Service you can find model, resolver and module related to the Experience entity.
  • inside the Company Service you can find model, resolver and module related to the Company entity. Moreover, in order to separate by concern, in the Company Service you can find an extension of the Experience model in order to add the company field (as described here: https://docs.nestjs.com/graphql/federation#code-first-1):

https://github.com/keadex/keadex-einaudi/blob/0.0.1/apps/company/src/models/experience.model.ts

@Schema()
@ObjectType()
@Directive('@extends')
@Directive('@key(fields: "_id")')
export class Experience {
  @Field(() => String)
  @Directive('@external')
  _id: MongooseSchema.Types.ObjectId;

  @Field(() => [Company])
  @Prop({ type: [MongooseSchema.Types.ObjectId], ref: Company.name })
  companies: MongooseSchema.Types.ObjectId[] | Company[];
}

The problem is that, the autogenerated GraphQL Schema ignores all the Directives, included the @extends one:

# ------------------------------------------------------
# THIS FILE WAS AUTOMATICALLY GENERATED (DO NOT MODIFY)
# ------------------------------------------------------

directive @extends on INTERFACE | OBJECT

directive @external on FIELD_DEFINITION | OBJECT

directive @key(fields: String!) on INTERFACE | OBJECT

directive @provides(fields: String!) on FIELD_DEFINITION

directive @requires(fields: String!) on FIELD_DEFINITION
...
type Experience {
  _id: String!
  ciao: String!
  companies: [Company!]!
}
...

Expected behavior

With the code-first approach I expect that NestJS, during the generation of the GraphQL Schema, takes into account all the directives added inside the model, autogenerating so a schema like the following one:

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

Minimal reproduction of the problem with instructions

The repo with the full code (Experience Service, Company Service, Gateway) is the following one: https://github.com/keadex/keadex-einaudi/tree/0.0.1

This is the Experience model (which extends the one inside the Experience Service - https://github.com/keadex/keadex-einaudi/blob/0.0.1/apps/experience/src/models/experience.model.ts) inside the Company Service: https://github.com/keadex/keadex-einaudi/blob/0.0.1/apps/company/src/models/experience.model.ts

What is the motivation / use case for changing the behavior?

Is not possible to extend GraphQL types and create GraphQL microservices separated by concern leveraging on Apollo Federation.

Environment


Nest version: 7.6.0

 
For Tooling issues:
- Node version: 12.13.0  
- Platform: Windows 
@jmcdo29
Copy link
Member

jmcdo29 commented Jun 23, 2021

Can you make a minimum reproduction that doesn't have the database dependency? Is this only for federation or does it happen for regular GQL too?

@keadex
Copy link
Author

keadex commented Jun 23, 2021

@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented Jun 24, 2021

#1278 (comment)
and the original issue graphql/graphql-js#1343

@keadex
Copy link
Author

keadex commented Jul 4, 2021

After some investigation and reverse engineering, I think that the issue reported by @kamilmysliwiec (graphql/graphql-js#1343) has been fixed by an own implementation of Apollo.

In fact by inspecting nestjs code, the schema is autogenerated by using the printSchema utility from the graphql package (https://graphql.org/graphql-js/utilities/#printschema):

By logging the fileContent variable, I can see exactly the (not correct - without directives) autogenerated file schema.

As stated above, Apollo has fixed the issue graphql/graphql-js#1343 in its own implementation, as also stated at the head of the code: https://github.com/apollographql/federation/blob/main/federation-js/src/service/printFederatedSchema.ts

In fact, nestjs uses that at runtime and by logging the generated schema by Apollo, I can see correctly the directives:

So in order to autogenerate from nestjs the correct GraphQL Schema (with the directives), in the following file I've changed the import of the printSchema function from graphql to @apollo/federation package:
https://github.com/nestjs/graphql/blob/3a7468e865472bad7d9ddedc8cc7370c3b9e91a4/lib/graphql-schema.builder.ts

By doing this, now the autogenerated GraphQL Schema file is correct (with the directives):

# ------------------------------------------------------
# THIS FILE WAS AUTOMATICALLY GENERATED (DO NOT MODIFY)
# ------------------------------------------------------
...
type Experience @extends @key(fields: "_id") {
  _id: String! @external
  companies: [Company!]!
}
...

@Kalmac
Copy link

Kalmac commented Jul 28, 2021

@keadex , your solution is perfect !

Thanks a lot.

@nestjs nestjs locked and limited conversation to collaborators Jul 29, 2021
keadex added a commit to keadex/graphql that referenced this issue Oct 7, 2021
…pproach

Federated schema generated with the code first approach does not contain
the directives.

Closes nestjs#1597
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants