nestjs / graphql

GraphQL (TypeScript) module for Nest framework (node.js) 🍷
https://docs.nestjs.com/graphql/quick-start
MIT License
1.45k stars 391 forks source link

GraphQl Multiple Endpoints not working #721

Open arslanmughal99 opened 4 years ago

arslanmughal99 commented 4 years ago

Bug Report

Current behavior

When adding multiple endpoints for graphql it gives following weird error

(node:16316) UnhandledPromiseRejectionWarning: Error: Schema must contain uniquely named types but contains multiple types named "User".
    at typeMapReducer (C:\Users\arsla\Documents\nest-graphql-practice\node_modules\graphql\type\schema.js:262:13)
    at Array.reduce (<anonymous>)
    at new GraphQLSchema (C:\Users\arsla\Documents\nest-graphql-practice\node_modules\graphql\type\schema.js:145:28)
    at GraphQLSchemaFactory.<anonymous> (C:\Users\arsla\Documents\nest-graphql-practice\node_modules\@nestjs\graphql\dist\schema-builder\graphql-schema.factory.js:28:28)
    at Generator.next (<anonymous>)
    at C:\Users\arsla\Documents\nest-graphql-practice\node_modules\tslib\tslib.js:113:75
    at new Promise (<anonymous>)
    at Object.__awaiter (C:\Users\arsla\Documents\nest-graphql-practice\node_modules\tslib\tslib.js:109:16)
    at GraphQLSchemaFactory.create (C:\Users\arsla\Documents\nest-graphql-practice\node_modules\@nestjs\graphql\dist\schema-builder\graphql-schema.factory.js:23:24)
    at GraphQLSchemaBuilder.<anonymous> (C:\Users\arsla\Documents\nest-graphql-practice\node_modules\@nestjs\graphql\dist\graphql-schema.builder.js:55:56)
(node:16316) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:16316) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Input Code

My appModule look the following

import { Module } from '@nestjs/common';
import { GraphQLModule } from '@nestjs/graphql';
import { MessagesModule } from './messages/messages.module';
import { UserModule } from './user/user.module';

@Module({
  imports: [
    MessagesModule,
    UserModule,

    GraphQLModule.forRoot({
      autoSchemaFile: 'userSchema.gql',
      playground: true,
      include: [UserModule],
      path: '/user',
    }),

    GraphQLModule.forRoot({
      autoSchemaFile: 'messageSchema.gql',
      playground: true,
      include: [MessagesModule],
      path: '/msg',
    })
  ],
})
export class AppModule { }

Expected behavior

The error above is totally unexpected , I have taken code first approach Also i have tried a lot but didn't find help any where.

Possible Solution

Environment


Nest version: X.Y.Z


For Tooling issues:
- Node version: v12.6.0
- Platform:  Windows

Others:
 "@nestjs/common": "^7.0.0",
    "@nestjs/core": "^7.0.0",
    "@nestjs/graphql": "^7.1.3",
    "@nestjs/platform-express": "^7.0.0",
    "apollo-server-express": "^2.11.0",
    "graphql": "^14.6.0",
    "graphql-tools": "^4.0.7",
kamilmysliwiec commented 4 years ago

Please, provide a minimal repository which reproduces your issue.

arslanmughal99 commented 4 years ago

@kamilmysliwiec here is the repository to reproduce nestjs-graphql-multiple-endpoint-issue if you disable any one of the end point it will work fine but raising error when both are activated at once

arslanmughal99 commented 4 years ago

Any updates ? i have tried "schema First approach" with different schemas but no luck so far :(

arslanmughal99 commented 4 years ago

So looks like something wrong with graphql dependency i have commented this line in node_modules\graphql\type\schema.js line 264

 if (seenType) {
    if (seenType !== namedType) {
      console.log(seenType, namedType);
      console.log(typeof seenType, typeof namedType);
      // throw new Error("Schema must contain uniquely named types but contains multiple types named \"".concat(namedType.name, "\"."));
    }

Now all working perfectly . but this not look like a permanent solution i have tried to console seenType and namedType both name user and typeof object. any perment solution ?

Ponjimon commented 4 years ago

The issue is that using code-first approach means using TS metadata because of all the decorators. TypeScript is unaware of the NestJS modules e.g. it cannot know that a @Resolver class belongs to a certain module thus the metadata storage stores ALL resolvers and makes them available to the global scope. Both GraphQLModules then load this metadata storage and both see the same resolvers and within the resolvers you're using the types which are shared again and so on thus resulting in duplicate types. I also do not know a solution for this yet, would appreciate it.

arslanmughal99 commented 4 years ago

@lookapanda Hi ...! I have just solve this issue by using schema first approach . Because i got no luck after in code-first approach .

JaLe29 commented 4 years ago

Hi @arslanmughal5566 ,

can you provide full code example of root module please.

I am trying to create one GraphQLModule with autoSchemaFile and second GraphQLModule without this flag.

arslanmughal99 commented 4 years ago

@JaLe29 Hello my solution was this app.module.ts

@Module({
  imports: [
  GraphQLModule.forRoot({
    debug: true,
    path: 'user',
    playground: true,
    include: [UserModule],
    typePaths: ['src/user/**/*.gql'],
    context: ({ req, res }) => ({ req, res })
  }),
  GraphQLModule.forRoot({
    debug: true,
    path: 'auth',
    playground: true,
    include: [AuthModule],
    typePaths: ['src/auth/**/*.gql'],
    context: ({ req, res }) => ({ req, res })
  })
],
 providers: [],
 exports: []
})

export class AppModule { }
kaufmo commented 4 years ago

Any updates ? i have tried "code First approach" with different schemas but no luck so far :(

I've created also an example, see #896

arslanmughal99 commented 4 years ago

@kaufmo Your issue is this https://github.com/kaufmo/nestjs-typeorm-multiple-graphql-bug/blob/684bab8815a001d4be8c9da00c98b7c36590d0d2/src/app.module.ts#L28

Dont use autoSchema . Remove code-first approch totally from the app and create schema.gql manually and then give it path to typePath like this


@Module({
  imports: [
  GraphQLModule.forRoot({
    debug: true,
    path: 'user',
    playground: true,
    include: [AdminModule],
    typePaths: ['your/path/to/*.gql'],
    context: ({ req, res }) => ({ req, res })
  }),
  GraphQLModule.forRoot({
    debug: true,
    path: 'auth',
    playground: true,
    include: [FrontendModule],
    typePaths: ['your/path/to/*.gql'],
    context: ({ req, res }) => ({ req, res })
  })
],
 providers: [],
 exports: []
})

export class AppModule { }

kamilmysliwiec commented 4 years ago

Will do my best to fix this issue as soon as possible

kaufmo commented 4 years ago

@kamilmysliwiec thx that would be really nice if its working. My existing api contains about 50 modules with code first approach, I don't want to update these to schema first.

kaufmo commented 4 years ago

@kamilmysliwiec do you have any information for me when this will be fixed?

TienHuong commented 4 years ago

@kamilmysliwiec Have any update for this issue?

bhuvan0616 commented 4 years ago

@kamilmysliwiec Looking for the fix. Any update?

jshearer commented 4 years ago

This is also not working for me -- same error as OP. Is the solution known here? I'd be happy to work on a PR if somebody can point me in the right direction. Something about the typescript metadata to graphql type generator is duplicating types when more than 1 endpoint is configured?

grinono commented 3 years ago

a solution to this problem would be fantastic!

j commented 3 years ago

I want to point out that this issue is related to #1171. I have a mono-repo with shared models. The models directly are decorated with @nextjs/graphql metadata. Simply requiring my shared models library automatically includes all types even though I have a resolver only using one type.

NormySan commented 3 years ago

I'm also having this issue and I really need to get this working. Is there anything that I could help with to get this resolved?

brycedeneen commented 3 years ago

@NormySan I found that when I changed my annotations from @ObjectType() to @ObjectType({ isAbstract: true }) it fixed the issue for me. Not sure if this helps you but thought I'd share.

NormySan commented 3 years ago

@brycedeneen Thanks for the info. I'll give it a try as a temporary solution.

NormySan commented 3 years ago

Has anybody investigated which part of the code that might be causing this issue? I'm going to try to solve it if I get some spare so if anybody has any input as to what could be causing it this would be a huge help to getting started!

francisfontoura commented 3 years ago

@NormySan I found that when I changed my annotations from @ObjectType() to @ObjectType({ isAbstract: true }) it fixed the issue for me. Not sure if this helps you but thought I'd share.

Use isAbstract: true in every ObjectType is the only approach that makes sense. I can't imagine why someone would want to include an unused (even registered) ObjectType in generated schema when using code first approach, because its main advantage is to keep consistency. I think isAbstract: true should be the default.

NormySan commented 3 years ago

@francisfontoura According to the documentation isAbstract is used to suppress SDL generation for the specific object type. If this solves the problem while it's still added to the schema then it's most likely a bug since thats not the expected behaviour from my understanding.

I'm not trying to include unused object types in my schema, I'm building an application with multiple GraphQL endpoints and both endpoints are getting the same object types, resolvers etc. added do them even though they are completely seperated and it's configured to only load object types, resolvers etc. from specific modules.

vnenkpet commented 3 years ago

I tried the isAbstract: true method, but this still doesn't work the moment I use any enum, where I can't set this property. Is this even in progress? Since this is opened since March and it's still not fixed, maybe it would be a good idea to remove it from the docs as it seems it's actually supported?

maximseshuk commented 3 years ago

Any solution? I followed the "isAbstract: true" solution, but my scalar types are throwing a multiple types error.

NormySan commented 3 years ago

I decided to switch my code from code first to schema first and now everything works as it should.

TBG-FR commented 3 years ago

Any chance someone has a solution to solve that while keeping code-first approach ?

altick commented 3 years ago

After some trials and errors I came up with working solution/workaround that allows to keep the code-first approach while having multiple GraphQL endpoints. The solution uses the NestJS workspaces.

The trick is in splitting the project into multiple apps with a shared library. Each of these apps has its own GraphQL configuration and can use whatever models and resolvers from the shared library. As both apps are built separately the schema is generated without any naming conflicts.

Here is the repo with the example

ahmedyounes commented 3 years ago

Even one endpoint

GraphQLModule.forRoot({
      typePaths: ['./src/**/*.graphql'],
      context: ({ req }) => ({ authToken: req.headers.authorization }),
      resolvers: {
        DateTime: GraphQLDateTime,
        Upload: GraphQLUpload,
      },

is not generating schema at launching tried many paths /src src ../src

smolinari commented 3 years ago

I think the multiple endpoint feature in the docs should have a note that it only works with the schema first system until this issue is fixed. I just spent a couple of hours wondering what I'm doing wrong to then decide to google for a possible answer or if someone else ran into the problem I have and this GH issue ended up the #1 result.

I believe this prohibits the ability to have a very secure authentication system with Nest using GraphQL (i.e. refreshing access tokens in a very "tight" cookie path for recalling the refresh token) because one can't have a separate endpoint for refreshing access tokens.

Also, with the above auth use case in mind, I think it would be cool to have an exclude prop, (as in opposite to the include prop) to tell the GraphqlModule to load every resolver except those noted in the exclude prop. If that sounds like a feasible idea, I'd even venture to try and PR it and at the same time see if I can dig into the code to find why the endpoint feature isn't working with the Code First approach.

For now and to move forward, I'm going to try creating the separate app suggested above. I'm using a monorepo setup already. Should be easy enough to try. It might not even be a bad idea seeing as a "refresh" endpoint could get a lot of hits with a lot of users and with it being its own app, it would be using a separate process (or even multiple processes) and so, it can scale separate to the core application or rather, not intrude on resources used by the core application.

Scott

wtho commented 3 years ago

As for my research, as my conflict appears with an ObjectType, it lies in the TypeDefinitionsGenerator#generateObjectTypeDefs - therefore somewhere in the ObjectTypeDefinitionFactory or one of the underlying registries, where probably the metadata/objects are not cleared and restored between the generation. So references to objects from the first run are reused, which breaks when creating a GraphQLSchema later on.

The GraphQLSchema from the official graphql-js package relies on object equality (using Set#has on GraphQLObjectType instances) and therefore adds referenced ObjectTypes twice - the one referenced in the metadata in the fields from a previous run as well as the object itself from the fresh generation.

Either the metadata/registries have to be cleared and constructed from scratch again or all objects have to be reused.

I have not identified the location where some store like a registry is reused from the previous module initialization though.

wtho commented 3 years ago

I dug a bit deeper and probably found the culprit for the discrepancy.

During the graphql module initialization and before constructing the schema, the GraphQLSchemaFactory resets all storages except of the TypeDefinitionsStorage, which is e. g. referenced by the TypeDefinitionsGenerator (which itself is used by GraphQLSchemaFactory).

The second execution of LazyMetadataStorage.load(resolvers); re-evaluates all object classes and their annotations and therefore cuts references to previous module initializations, but does not clear these references in the TypeDefinitionsStorage, which will be favoured to be re-used later when establishing e. g. connections between field-object-types with their parents.

A viable solution is to add a clear method on the TypeDefinitionsGenerator, which in return would call a clear method on the TypeDefinitionsStorage.

I will create a PR.

nick4fake commented 3 years ago

Hi all,

As a temporary solution until this feature is fixed I have published a forked package: https://www.npmjs.com/package/@nick4fake/nest-graphql

Please keep in mind that I obviously wouldn't support it in any way as it is simply used to unblock my work.

It is based on @wtho's pull request: https://github.com/nestjs/graphql/pull/1432

So far it works flawlessly (with code-first approach), so let's hope some solution is merged into the main codebase.

kamilmysliwiec commented 3 years ago

I'm gonna lock this PR for the time being as this is a well-known issue. I'll tackle this as soon as I have time.