Incorrect schema merging when using typeDefs rather than typePaths

[michaelbromley]

I'm submitting a...

[ x ] Bug report

Current behavior

(Me again! Thanks for the incredibly rapid response to my last two issues - hopefully this is the last for a while ... 😄 )

So in my app I am using the typeDefs config option and omitting typePaths because I do some of my own pre-processing of the schema files before handing them off to Nest.

There is an issue currently with this part of the GraphQLModule code:

graphql/lib/graphql.module.ts

Lines 100 to 106 in 304c1fe

	const typeDefs = this.graphQLFactory.mergeTypesByPaths(
	...(this.options.typePaths || []),
	);
	const apolloOptions = await this.graphQLFactory.mergeOptions({
	...this.options,
	typeDefs: extend(typeDefs, this.options.typeDefs),
	});

When this.options.typePaths is falsy (undefined in my case), then the call to this.graphQLFactory.mergeTypesByPath() will return the following string:

schema {
  query: Query
}

When this is later combined with the string I pass as the typeDefs value, then the resulting schema only contains my Queries, but none of my Mutations.

Expected behavior

Passing typeDefs and no typePaths should result in a schema exactly equivalent to that defined by the typeDefs string.

Minimal reproduction of the problem with instructions

1. git clone git@github.com:nestjs/nest.git
2. cd nest/sample/12-graphql-apollo
3. npm install
4. edit app.module.ts to look like:

   GraphQLModule.forRootAsync({
      useFactory() {
        return {
          installSubscriptionHandlers: true,
          typeDefs: `
          type Query {
            getCats: [Cat]
            cat(id: ID!): Cat
          }
          
          type Mutation {
            createCat(name: String): Cat
          }
          
          type Subscription {
            catCreated: Cat
          }
          
          type Cat {
            id: Int
            name: String
            age: Int
          }
          `
        };
      },
    }),

5. npm run start

When trying to execute the createCat mutation, you will not get the error: "Schema is not configured for mutations."

Additional note: I noticed when putting together the reproduction that when passing the above config to the .forRoot() method, the app does not even bootstrap, instead failing with the error:

UnhandledPromiseRejectionWarning: Error: Type "Query" was defined more than once.
    at Object.buildASTSchema (C:\Development\temp\nest\sample\12-graphql-apollo\node_modules\graphql\utilities

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

Sometimes you need to pre-process the typedefs before handing off to Nest. In my case, I use user config to create custom fields at run-time.

Suggested fix

I fixed the issue locally by changing line 100 to:

const typeDefs = this.options.typePaths ? this.graphQLFactory.mergeTypesByPaths(
      ...(this.options.typePaths || []),
    ) : '';

Environment

Nest version: 5.3.2, graphql v5.1.1

[kamilmysliwiec]

This issue comes from the underlying https://github.com/okgrow/merge-graphql-schemas library. I'll fix it asap

[kamilmysliwiec]

Fixed in 5.1.2

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.