@nestjs/graphql redundancy required for types

[radarsu]

I'm submitting a...

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

Current behavior

Currently I have to define both: cats.graphql file and cats.resolver.ts along with cats.dto.ts. I clearly see quite a lot of redundancy in those files, it's huge space for endless mistakes from developers synchronizing those. Decorators handle resolver mapping, but .graphql file or files could be generated from typescript code too (https://github.com/19majkel94/type-graphql does it).

Expected behavior

I would love type-graphql library to be integrated in @nest/graphql. I would do this on my own and submit pull request, but graphql package is quite a lot of code to analyze.

Minimal reproduction of the problem with instructions

Currently to remove redundancy in files I have to use type-graphql library as a workaround, doubling decorators (or merging them both into one). For example:
app.module.ts

// graphql
GraphQLModule.forRootAsync({
    useFactory: async () => {
        const config: GqlModuleOptions = {
            typePaths: [
                path.join(process.cwd(), `src/modules/**/**/*.graphql`),
            ],
            installSubscriptionHandlers: true,
        };

        config.graphql.schema = await buildSchema({ // buildSchema is method from type-graphql library
            resolvers: [AuthResolver],
            emitSchemaFile: path.resolve(process.cwd(), `src/modules/auth/auth.graphql`),
        });

        return config.graphql;
    },
}),

auth.resolver.ts

import {
    Args, Mutation, ParseIntPipe, Query, Resolver, Subscription, UseGuards, UserEntity, UserGuard, UserService, radarsuLogger, config, AuthService, Root, Context, Info,
} from '../../exporter';

import {
    Mutation as TypeMutation, Resolver as TypeResolver, Args as TypeArgs, Arg as TypeArg, Query as TypeQuery, InputType, Field as TypeField, ArgsType, ObjectType,
} from 'type-graphql';

@InputType()
export class LoginUserInput {
    @TypeField()
    login: string;

    @TypeField()
    password: string;
}

@InputType()
export class CreateUserInput {
    @TypeField()
    login: string;

    @TypeField()
    password: string;
}

@TypeResolver()
@Resolver('Auth')
export class AuthResolver {
    public constructor(private readonly authService: AuthService) { }

    @TypeMutation(returns => Boolean, { nullable: true })
    @Mutation()
    public async loginUser(@TypeArg('loginUserInput') @Args('loginUserInput') loginUserInput: LoginUserInput): Promise<void> {
        log.debug(`loginUserInput`, loginUserInput);
        const userLogged = await this.authService.login(loginUserInput);
        config.pubSub.publish('userLogged', { userLogged });
    }
}

In above example everything works perfectly and I get automatically-generated on application lift schema.graphql file. No need to leave world-of-typescript for graphql schemes because they get generated:

# -----------------------------------------------
# !!! THIS FILE WAS GENERATED BY TYPE-GRAPHQL !!!
# !!!   DO NOT MODIFY THIS FILE BY YOURSELF   !!!
# -----------------------------------------------

input LoginUserInput {
  login: String!
  password: String!
}

type Mutation {
  loginUser(loginUserInput: LoginUserInput!): Boolean
}

type Query {
  recipes: Recipe!
}

type Recipe {
  id: String!
}

Also, graphql.schema.ts file becomes unnecessary in such setup. The only bad thing is that these setup is not part of @nestjs/graphql package! Is there any good reason for that?

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

To improve @nestjs/graphql to the next level!

[soanvig]

I'm looking exactly into the same issue. Though https://docs.nestjs.com/graphql/resolvers-map describes TS being generated out of GraphQL schema, but when one wants to add decorators to those typings, they become unmaintanable anymore. This is how I perceive that, at least.

GraphQL files being less advanced than TS files should be generated out of these, not vice versa.

I try Nest.js, wanted to check out GraphQL, but I came across that issue, and I'm not moving forward with GraphQL until it is resolved in some way. That kind of redundancy is one of the worst.

[biels]

@radarsu What would this .dto.ts file exactly be? Wouldn't the graphql input type for the entity replace this supposed dto? At least that's how I'm doing it.
We should really come with a standarized way to do it.

@soanvig

GraphQL files being less advanced than TS files should be generated out of these, not vice versa.

I think that the problem is that graphql and entity represent different logical things. One thing is how you store internal data and the other is which queries and actions (mutations) you offer as an api.
For example consider the following very useful practice: having a user type like the following:

type User {
   name: String
   permissionLevel: PermissionLevel
   permissionLevelOptions: [PermissionLevel]
}
enum PermissionLevel{
   admin
   moderator
   collaborator
   viewer
}

The permissionLevelOptions would show which options you have if you want to set the permissionLevel field through an input that would contain only name and permissionLevel in this case. The options displayed in permissionLevelOptions would change depending on the context (for example, your own permission level) so you can render a dropdown in a frontend using permissionLevel as the field value and permissionLevelOptions as the field options. In this example permissionLevel is of type enum but it could be of any other type, such as another entity for example.
So in this case it makes sense to specify the graphql schema independently of the internal representation of the data.
However, I also think that automating part of the generation of the graphql schema is possible and necessary. Maybe by having some fields always be there. Or maybe doing it the other way around: Providing default implementations for typical queries like allUsers() or user(id: ID) like spring does with repositories in spring-data-jpa (I know we are into node here but it can be useful to look at how others have solved similar problems).

[soanvig]

This is hard topic, because for now if you are building an API you do layers:

1. database
2. ORM
3. DTO (built out of ORM objects)
4. server/routing/controllers

adding fifth layer (GraphQL schemas) makes it even harder to develop.

[radarsu]

My current solution are custom decorators. I mixed decorators from Nest and Type-graphql.

export const combineDecorators = (decorator1: any, decorator2: any) => {
    return (options1: any[] = [], options2: any[] = []): any => {
        let fn1;
        let fn2;

        try {
            fn1 = decorator1(...options1);
        } catch (e) {

        }

        try {
            fn2 = decorator2(...options2);
        } catch (e) {

        }

        return (target: any, key: string, index: any) => {
            if (typeof fn1 === 'function') {
                fn1(target, key, index);
            } else if (typeof decorator1 === 'function') {
                decorator1(target, key, index);
            }

            if (typeof fn2 === 'function') {
                fn2(target, key, index);
            } else if (typeof decorator2 === 'function') {
                decorator2(target, key, index);
            }
        };
    };
};

// type-grapqhl + @nest/graphql - type-graphql uses options more often so its first

// class decorators
export const RadarsuResolver = combineDecorators(TypeResolver, NestResolver);

// method decorators
export const RadarsuMutation = combineDecorators(TypeMutation, NestMutation);
export const RadarsuQuery = combineDecorators(TypeQuery, NestQuery);
export const RadarsuSubscription = combineDecorators(TypeSubscription, NestSubscription);

// args decorators
// export const RadarsuArgs = combineDecorators(TypeArg, NestArgs);
export const RadarsuArgs = (name?: string, options1: any[] = [], options2: any[] = []): any => {
    const fn1 = TypeArg(name, ...options1);
    const fn2 = NestArgs(name, ...options2);

    return (target: any, key: string, index: any) => {
        if (typeof fn1 === 'function') {
            fn1(target, key, index);
        }

        if (typeof fn2 === 'function') {
            fn2(target, key, index);
        }
    };
};

Then in resolvers it looks like that:

// requests allowed with token
@UseGuards(TokenGuard)
@RadarsuResolver()
export class AuthResolver {
    public constructor(
        private readonly authService: AuthService,
    ) { }

    // queries

    // sending confirmation response
    @RadarsuQuery([() => Boolean, { nullable: true }])
    public async response(@RadarsuArgs('id') id: number, @NestContext() ctx): Promise<void> {
        if (_.isFunction(config.idAwaiter[id])) {
            config.idAwaiter[id]();
            delete config.idAwaiter[id];
        }
    }

    // mutations

    // login
    @RadarsuMutation([() => Boolean, { nullable: true }])
    public async loginUser(@RadarsuArgs('input') input: LoginUserInput): Promise<void> {
        const userLogged = await this.authService.login(input);
        config.pubSub.publish('userLogged', { userLogged });
    }

    // sign up
    @RadarsuMutation([() => Boolean, { nullable: true }])
    public async createUser(@RadarsuArgs('input') input: CreateUserInput): Promise<void> {
        const userCreated = await this.authService.create(input);
        config.pubSub.publish('userCreated', { userCreated });
    }
}

And in app.module:

GraphQLModule.forRootAsync({
    async useFactory() {
        config.graphql.schema = await buildSchema({
            resolvers: [AuthResolver, PublicResolver],
            emitSchemaFile: path.resolve(process.cwd(), `data/graphql/schema.graphql`),
        });

        return config.graphql;
    },
}),

[kamilmysliwiec]

Perfect @radarsu. This is exactly what I was thinking about in terms of Nest and TypeGraphQL integration cc @19majkel94

[MichalLytek]

Combined decorators are an obvious solution for the basic integration.

The trickiest part for me is making guards, interceptors and pipes working (see #135 (comment)), as I haven't dug into the nest execution pipeline yet.
It has to support features from both frameworks, like middlewares and class-validator integration (TypeGraphQL) and AuthGuards and pipes (Nest).

For now I have to fix the core before 1.0 release, so after that we can think about deeper integration @kamilmysliwiec 😉

[kamilmysliwiec]

@19majkel94 solution presented by @radarsu is using powerful TypeGraphQL capabilities of generating schema directly from metadata provided by generators, and at the same time, is leaving the rest of responsibilities to Nest itself (execution pipeline + framework's specific tooling [guards, interceptors, etc]). Basically, Nest already provides an integration with class-validator (through ClassSerializerInterceptor + ValidationPipe), however, we don't support GQL middleware (like TGQL does, and integration at this point might be very tricky, almost impossible I believe).

So generally, I could integrate these two at @nestjs/graphql lib level, albeit, in this case, we would probably stick with our execution model + take advantage of tremendous abilities of TGQL type reflection (to generate the whole schema and typings from metadata). If you like it, I think that it shouldn't take too much time to integrate them together :) @19majkel94

[MichalLytek]

TypeGraphQL is not only about typeDefs from classes but also a set of useful features for creating the API. Some of them have equivalents in Nest (dependency injection, @Authorized vs. AuthGuard, class-validator integration vs. ValidationPipe) but some of the don't (middlewares, inline field resolvers, different subscriptions signature and probably resolvers inheritance and args classes).

Without controlling the execution of resolvers, a lot of features that are coming up next won't be usable in Nest (MichalLytek/type-graphql#193, MichalLytek/type-graphql#142, MichalLytek/type-graphql#51, MichalLytek/type-graphql#44), so supporting only subset of features shouldn't be the final goal.

integration at this point might be very tricky, almost impossible I believe

Everything is possible 😄 It's just a matter of time and effort.
But I agree it's not so easy and will require a lot of work and even compromises (support for some mutually exclusive features only from one side).

So, for now, as a temporary solution, I would propose two approaches:

• @nestjs/type-graphql - using TypeGraphQL only as a tool for "generating the whole schema and typings from metadata" (by providing combined decorators) and using the Nest execution model with it's specific tooling (guards, interceptors, etc.)
• @typegraphql/nestjs - using TypeGraphQL as the main schema execution model with a bunch of Nest helpers (module/factory for plugging this all in single line of code, like a DI container) for those who need middlewares and advanced resolvers things of TypeGraphQL

Maybe during the integration implementation we will hit upon an idea how to mix them together without compromises 😉

[kamilmysliwiec]

Thanks for quick response @19majkel94, I fully agree.

Recapitulating, let's start with temporary solutions. During the integration process, I'll dive a little bit into TypeGraphQL public API and perhaps, I'll try to adjust Nest API as possible (for example, subscriptions API). Due to that, it should be easier to provide better integration at some point :) I'll let you know if I've come across some potential idea how to connect them in an even more solid way, please, let me know too!

[mohaalak]

@kamilmysliwiec @19majkel94 I made a module for nestjs, manually combining decorators, type-graphql will create the schema and nestjs will provide the resolvers
https://github.com/mohaalak/nestjs_typegraphql

[IgorSzymanski]

@kamilmysliwiec @19majkel94 I made a module for nestjs, manually combining decorators, type-graphql will create the schema and nestjs will provide the resolvers
https://github.com/mohaalak/nestjs_typegraphql

Would be much better, if type-graphql was a peerDependency.

[kamilmysliwiec]

type-graphql integration is provided in 6.0.0

[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.