Nest: Make the Dependency Injection cleaner

[IonelLupu]

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

Hi. Love your work here. I'm coming with some suggestions for some features I think would be great to have and here i'm referring to the Dependency Injection feature in particular. Let me explain downside my proposals:

1. Better DI inside controller actions

__Current way:__

@Controller('user')
export class UserController {
    @Post('/search')
    search(@Req() request) {
        // do something with the request
    }
    @Get('/:id')
    search(@Param('id') id:number) {
        // do something with the param
    }
}

No type-hinting for the request variable. Of course, we can add the type like so @Req() request: Request,
but why not using the type to resolve the dependency instead of the decorator. The decorator seems redundant now.
The same for the @Param. We can use the name of the variable to resolve the dependency because the code on the back-end will almost never be uglified(i know it's kinda hackish to get the name of the a variable in JavaScript but is worth it).

__Proposed way:__

@Controller('user')
export class UserController {
    @Post('/search')
    search(request: Request) {
        // do something with the request
    }

    @Get('/:id')
    search(id: number) {
        // do something with the param
    }
}

Here request is resolved by the container to an instance of the Request class and it's passed to the controller action.

2. Support for Data Transfer Objects(DTOs), or Forms as I like to call them

We are inside typescript. Why don't we use reflection to inject our dependencies?
__Current way:__

class CreateUserDto{
    readonly name: string;
    readonly age: number;
}

@Controller('user')
export class UserController {
    @Post()
    async create(@Body() createUserDto: CreateUserDto) {
        return await this.usersService.create(createUserDto);
    }
}

This creates an instance of CreateUserDto and sets the data found in the request inside the instance. All fine and nice. But I think we can do better.

__Proposed way:__
I like to call DTOs Forms. We can add all sorts of things to our Form now, like validation using class-validator for example.

class CreateUserForm extends Form{
    readonly name: string;
    @IsInt()
    readonly age: number;
}

@Controller('user')
export class UserController {
    @Post()
    async create(createUserForm: CreateUserForm) {
        if(createUserForm.isNotValid()){
            throw new ValidationException(createUserForm.errors())
        }
        return await this.usersService.create(createUserForm.all());
    }
}

Actually, if we know that we are passing a Form instance we can move the entire validation logic behind the scenes and the controller could look like this:

@Controller('user')
export class UserController {
    @Post()
    async create(createUserForm: CreateUserForm) {
        return await this.usersService.create(createUserForm.all());
    }
}

If the data is not valid the framework can throw an exception on its own, no need for the developer to do that manually.

I am very inspired by this framework and I like the initiative. Would be nice to see these features added.

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

Cleaner code.

Waiting your warm answer! (I can contribute :D)
Thank you!

[shekohex]

Hi @JCWolf
Thank you for your proposals, let me explain my opinion about it:

First things first the nest documentations need a lot of work and improvements yeah it has a lot of things but it's yet have missed some important things too.
Lets take the first proposal

1. About Static Typing ? That's easy and not related to nest at all, just let me explain.

You know that nest is a good framework for organizing and structures your project and code and provide a lot of other tools too.
One of the most beautiful things that nest is not tight to a specific http layer
You can use Express or Fastify or anything works like that.

I know i know that nest is built on top of Express at first but sooner i think in the next major release 6.0.0 nest will get free of it `

So to get a static typing and type-hinting just install the types of your underlying framework like @types/express for example, so IMO it's not hard to implement a such thing, i like it personally but i don't know if there is some limition from typescript or not but BTW you will need at least one Decorator to make Typescript emit types.

1. And about DTO, actually Nest supports that did you see Pipes ?
   Nest has the built-in Validation Pipe.
   And supports class validator
   https://docs.nestjs.com/pipes

All validation logic is abstracted from you and Done by the Validation Pipe.

That's it 😀

[IonelLupu]

1. Typescript can emit types using the reflect-metadata library and later (after a request has been made for example) you can get the types of the parameter. Giving the below controller:

@Controller('users')
export class UserController{

    @Get('/')
    all(request: Request){
        // so something with request
    }
}

You can extract the parameter types like so:

let parameterTypes = Reflect.getMetadata("design:paramtypes", new UserController, 'all');

Using this we can pass the Request instance to the controller method. Now the controller will be cleaner and also have type-hinting.
Yes, you need at least one Decorator, but the routes already have one.
What I want is to make some methods slimmer by writing less code ( and concentrate more on the business logic ).

1. I saw the documentation but I think the validation library should already be in the framework. I know I know, you might say Nestjs let's you customize stuff but I think Nestjs can be one of the BIG framework that can provide you the basic support for every features needed(like Spring, Laravel, .ASP NET etc)

Let me show you an example. I was building a small back-end service API(nestjs + typeorm) for one of our applications and I had to build a decorator to get the entity from database based on the param value. Just like so:

@Controller('user')
export class UserController {
    @Get(':user')
    getUser(@Entity(User) user: User) {
        // some code
    }

This is equivalent to

@Controller('user')
export class UserController {
    @Get(':user')
    getUser(@Param('user') id: number) {
        let user = User.findOne(id)
        // some code
    }

I had this line let instance = Entity.find(id) way too many times so I decided to extract it in a decorator to make the controller slimmer. This thing is implemented inside the Laravel framework as 'route-model binding'. A little nice feature nice to have here too.

This can be replaced to simply getUser(user: User).

My point is: let's give the developers all the tools needed to build something BIG and make them worry less about the libraries they want to use(of course, these libraries can be replaced if needed).

As Elon Musk said: "The point of doing this is to give a hardcore smackdown to _other frameworks_" 😄

[BrunnerLivio]

    getUser(@Entity(User) user: User) {
        // some code
    }

@JCWolf I like the idea, and I think this could potentially be an additional package or directly integrated into @nest/typeorm. Otherwise I'd happily start a new project, maybe @nest-contrib/typeorm-extra?

Maybe create a new issue on nest/typeorm for the second feature?

[IonelLupu]

Hi @BrunnerLivio. Glad you like it but as I said, I would like NestJS to be a solid framework that has tons of built-in features.

I would create a new issue for this features @Entity feature(or perhaps we can add it without the decorator) but my problem is : How do we resolve the entity? In my example I used typeorm but NestJS doesn't have a default database package(would be nice if NestJS would have a database package by default. I would like to build a special one just for this framework).

@kamilmysliwiec , what do you think?

[BrunnerLivio]

NestJS doesn't have a default database package

It does have a default / supported database driver package: nest/typeorm. Or what do you actually mean by that?

I would like NestJS to be a solid framework that has tons of built-in features.

Nest is heading to that goal BUT with multiple packages.

Why multiple packages?

For example, as a developer I want to build a small console application built with Nest which does some HTTP requests to an API. Why do I need to have dependencies like typeorm, or express then? This would be just unnecessary and harder to maintain. So that is why Nest is trying to split these functionalities into multiple packages. More onto that in issue #531.

The disadvantage is off course, you have to install multiple packages for just a for example web application. But thats why the nest/cli was invented, so you still can easily bootstrap your ecosystem easily.

That is why decorators like @Entity should stay in nest/typeorm and not inside the core. The main functionality of nest/core should be the orchestration and the DI system in the end (or atleast thats planned for 6.x.x)

[IonelLupu]

I understand. You are right. Decorators like @Entity should stay inside nest/typeorm so i'll make an issue there.

I'm also thinking adding an issue for nest/cli to add a step for choosing the database with options like:

• No database
• Typeorm
• Mongoose
• Sequelize

But we deviated from my original issue: It is possible modify the DI to use the type of the parameter to inject the proper dependency without using the decorators? (using decorator AND parameter type is a little redundant: get(@Req request: Request)). Would be nice to write only get(request: Request)

[kamilmysliwiec]

In terms of getting rid of the @Req() decorator, please, keep in mind that:

1. You could use Nest with vanilla JavaScript too.
2. Nest goal is to stay portable across different HTTP libraries (we cannot explicitly tie framework to express typings, whilst we wanna support fastify as well).

[IonelLupu]

Well, there are two options then:

1. Use the nodejs's http.IncomingMessage as the base Request instance. It provides all the info we need and it is built in nodejs.
2. Make a Request wrapper around the nodejs's http.IncomingMessage interface that will have the same functionality as the @Req() decorator.

BUT:
What is the @Req returning anyways? An instance of the current request no matter of the HTTP library, right?. Using the DI we can return the same thing by binding the request instance in the container for each HTTP library.

Something Like

// express.provider.ts
this.container.bind('Request', theExpressRequest)

Or for fastify

// fastify.provider.ts
this.container.bind('Request', theFastifyRequest)

No matter the HTTP library, the Request instance will exist(the developer should know which one is returned by the HTTP library used. This is his concern).

[kamilmysliwiec]

Yes, but still:

You could use Nest with vanilla JavaScript too.

Which means = there are no typings.

[lock[bot]]

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.