Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A Guard with dependencies can't be injected into another module. #3856

Closed
greenreign opened this issue Jan 17, 2020 · 5 comments
Closed

A Guard with dependencies can't be injected into another module. #3856

greenreign opened this issue Jan 17, 2020 · 5 comments
Labels
needs triage This issue has not been looked into

Comments

@greenreign
Copy link

Bug Report

A Guard with dependencies can't be injected into another module.

Current behavior

I have 3 modules UserModule, AuthModule and ProtectedModule
ProtectedModule imports AuthModule.
AuthModule imports UserModule
UserModule exports UserService.
AuthModule exports AuthGuard
ProtectedService uses AuthGuard

Nest is unable to create AuthGuard even though UserModule is imported into AuthModule and UserService is exported from UserModule.

Nest can't resolve dependencies of the AuthGuard (?). Please make sure that the argument UserService at index [0] is available in the ProtectedModule context.

Potential solutions:
- If UserService is a provider, is it part of the current ProtectedModule?
- If UserService is exported from a separate @Module, is that module imported within ProtectedModule?
  @Module({
    imports: [ /* the Module containing UserService */ ]
  })

This means I'd have to inject dependencies of dependencies wherever used. This negates the entire point of Modules and DI.

I've probably just missed something?

Input Code

https://github.com/greenreign/nest-dep-injection-issue

Expected behavior

Modules with imported providers should not have to also import the modules that those providers depend on when this is already defined in the provider's module.

Environment


Nest version: 6.10.14

 
For Tooling issues:
- Node version: 10.17
- Platform:  Windows
@greenreign greenreign added the needs triage This issue has not been looked into label Jan 17, 2020
@kamilmysliwiec
Copy link
Member

Guards are not providers and you shouldn't register them as providers. You can't "export" guards either. https://docs.nestjs.com/guards

Please, use our Discord channel (support) for such questions. We are using GitHub to track bugs, feature requests, and potential improvements.

@greenreign
Copy link
Author

Guards are not providers and you shouldn't register them as providers. You can't "export" guards either. https://docs.nestjs.com/guards

Ok, then this is very misleading eh?
"A provider is simply a class annotated with an @Injectable() decorator."
"A guard is a class annotated with the @Injectable() decorator."
https://docs.nestjs.com/providers
https://docs.nestjs.com/guards

I could have missed a note or something but nowhere do I see any indication that a guard would not be treated like any other provider and be part of the DI pattern. Why wouldn't I want to take advantage of DI and modularization with guards that will likely be dependent on reusable providers?

Please, use our Discord channel (support) for such questions. We are using GitHub to track bugs, feature requests, and potential improvements.

Sorry, I really like nestjs and the type of architecture it enables for typescript projects. I have been using it for 3 years and I had no idea there was a discord channel. I have never opened an issue before and I was certain that this was a bug. If this isn't a bug then it's at least a request for better documentation. Also, as a feature request I think it makes perfect sense.

@greenreign
Copy link
Author

greenreign commented Jan 18, 2020

In case anyone comes across this. Here is some info from the discord channel and how I decided to handle it:

Hello, I recently was educated that Guards are not providers. Therefore nest cannot construct them unless all of their dependencies exist in the module they are being used. That means if I have an AuthGuard that is going to be used in likely every single module. Any provider that AuthGuard depends on (ie AuthService, UserService, BusinessService etc) needs to be available in every module that needs protection from the AuthGuard. So I will have to import 3+ modules into any module that uses the AuthGuard (every module) even though those module do not directly require these other modules. Does this sound right? It sounds almost completely the opposite of the core spirit of nest. Therefore I hope I am missing something?

zakToday at 1:08 PM
Maybe I could create an AuthModule which imports and re-exports all the necessary providers? But that's just a hack since the AuthGuard actually is not a "provider" of the AuthModule.

jmcdo29Today at 1:19 PM
@zak I like to think of guards as "pseudo-providers" in that they are @Injectable() and can be constructed with DI, but aren't put into the providers array (just like interceptors, pipes, and filters). For my guards, I have everything the guards need in an AuthModule that exports any providers the guard needs. Then I just have to import the AuthModule and be done with it. All the dependencies of the guard are now available in the current module's context thanks to the AuthModule.

zakToday at 1:21 PM
Yeah, @jmcdo29 thanks. That works. It wasn't obvious to me. I can live with that as long as other modules can just import AuthModule it actually seems like it working how you'd expect. I originally forgot about module re-exporting. 👍

@kierans
Copy link

kierans commented Feb 18, 2020

Guards are not providers and you shouldn't register them as providers. You can't "export" guards either.

@kamilmysliwiec Are you able to please explain why? A Guard, Interceptor, Filter, etc are beans right? They're instantiated and managed by the DI container. Why are these beans special? Why can't I pass them between modules? (I had this issue with an Interceptor) I think there's really good use cases for making these beans regular providers, not pseudo providers (to borrow from @jmcdo29)

Ok, then this is very misleading eh?
"A provider is simply a class annotated with an @Injectable() decorator."
"A guard is a class annotated with the @Injectable() decorator."

@greenreign I had exactly the same thoughts/problems.

@kamilmysliwiec
Copy link
Member

@kamilmysliwiec Are you able to please explain why? A Guard, Interceptor, Filter, etc are beans right? They're instantiated and managed by the DI container. Why are these beans special?

Please, don't use Java Spring nomenclature. We don't have "beans".

They're instantiated and managed by the DI container.

Yes and no. Enhancers (interceptors/guards/pipes/filters) can be either resolved by the scope and managed by the DI container or instantiated manually (with new keyword)

I think there's really good use cases for making these beans regular providers, not pseudo providers (to borrow from @jmcdo29)

Enhancers can't be injected to other providers. Hence, they are not providers. Likewise, enhancers can be tied & injected to the method evaluation context, but you can't bind providers this way. There's no reason to change this, especially if you start thinking about enhancers scoped per host (module in which they exist). Adding them to the providers array explicitly creates an unnecessary redundancy and confusion (since enhancers don't act equally to providers).

Please, use our Discord channel (support) for such questions. We are using GitHub to track bugs, feature requests, and potential improvements.

@nestjs nestjs locked as resolved and limited conversation to collaborators Feb 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

3 participants