Federation Directive on Entity class means fields are missing when using recent graphql dependencies.

eddiesholl commented 2 years ago

Did you read the migration guide?

[X] I have read the whole migration guide

Is there an existing issue that is already proposing this?

[X] I have searched the existing issues

Potential Commit/PR that introduced the regression

No response

Versions

No response

Describe the regression

Starting point: I created a very vanilla nest js app, following the guides to set up the repo via the CLI, and then add dependencies needed for graphql federation. This includes getting graphql v16, for example.

When defining types via code-first, using an @Directive on a basic entity definition class means that the graphql parser can't find any fields on the type, and throws an error. Something has broken in the Apollo and graphql dependencies in a recent release, as this can be reproduced with the sample 31 federation example, if you upgrade to the latest dependencies.

A simple entity definition like this throws the error, and if you remove the Directive the fields are found correctly:

import { ObjectType, Field, ID, Directive } from '@nestjs/graphql';

@ObjectType()
@Directive('@key(fields: "id")')
export class OrgGroup {
  @Field((type) => ID)
  id: number;

  @Field()
  name: string;

  @Field((type) => [ID])
  members: number[];
}

GraphQLValidationFailed: The schema is not a valid GraphQL schema.. Caused by:
[<unnamed>] Type OrgGroup must define one or more fields.

    at new GraphQLErrorExt (/foo/node_modules/@apollo/core-schema/src/error.ts:23:5)
    at err (/foo/pathzero/pathzero.starter-kit/node_modules/@apollo/core-schema/src/error.ts:67:17)
    at ErrGraphQLValidationFailed (/foo/node_modules/@apollo/subgraph/node_modules/@apollo/federation-internals/src/definitions.ts:52:6)
    at addSubgraphToError (/foo/@apollo/subgraph/node_modules/@apollo/federation-internals/src/federation.ts:1506:84)
    at Subgraph.validate (/foo/node_modules/@apollo/subgraph/node_modules/@apollo/federation-internals/src/federation.ts:1389:15)

I've reproduced the issue using the nest sample 31, and upgrading the package.json dependencies to match what I've ended up with from running a nest initialisation today. See the branch here: https://github.com/eddiesholl/nest/tree/graphql-16-directive-bug

From my debugging, it sounds similar to this issue: https://github.com/apollographql/federation/pull/1539

Minimum reproduction code

Reproduced using the branch here: https://github.com/eddiesholl/nest/tree/graphql-16-directive-bug

Go to /sample/31-graphql-federation-code-first/posts-application
npm install
npm run start:dev

You'll see the error about no fields being found on the Post entity. Given that's the sample for this feature, that seems like a pretty good demonstration.

If you look at the commit on that branch, you'll see the package upgrades that trigger the problem. I've chosen these versions as that's what I've seen after following all the basic nest graphql setup instructions.

Expected behavior

The types generated from these basic types with Directives should be valid and accepted

Other

No response

greguintow commented 2 years ago

Hey @eddiesholl , I was facing the same issue, I fixed with this PR:

2251

kamilmysliwiec commented 2 years ago

Fixed in 10.0.19.

Example usage:

GraphQLModule.forRoot<ApolloDriverConfig>({
  driver: ApolloFederationDriver,
  autoSchemaFile: {
    federation: 2,
  },
})

nestjs / graphql