-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Guards and Pipes Execution order #767
Comments
@amirsaber totally agree! |
@kamilmysliwiec any thought on this? |
I totally understand your point here. Nevertheless, giving an ability to switch execution hierarchy may bring a lot of mess to the framework and make the codebases less consistent since the order might by totally inverted. Any ideas how the API could hypothetically look like? |
I am wondering what is the initial thought about executing pipes after guards. That might help me to understand what is the best solution.
If the order is not defined or equal we can follow a static rule of which one to execute first. We can also stick to the idea of pipes being used for transforming data to desired output and provide a ValidationGuard that takes care of throwing error if body is not in correct format. |
how abount this solution? just like interceptor. @kamilmysliwiec @Injectable()
export class Guard implements CanActivate {
canActivate(
context: ExecutionContext,
pipe$: Observable<any>,
): boolean | Promise<boolean> | Observable<boolean> {
/* guard before pipe */
if (false) return of(false);
return pipe$.pipe(map(() => {
/* guard after pipe */
return true;
}));
}
} |
@amirasaber EDIT: nest/packages/common/decorators/core/use-guards.decorator.ts Lines 17 to 37 in ce498e8
|
@kamilmysliwiec great point, although some features make it easier to work, in some cases, they can break the default structure of the framework and cause problems in the of use. |
@kamilmysliwiec any future research? |
Even though this change could sometimes, potentially make life easier, we cannot break the default request pipeline and the natural behavior of the framework. As I've mentioned, giving an ability to switch execution hierarchy may bring a lot of mess to the framework and make the codebases less consistent since the order might by totally inverted. Every time when someone would face an issue and ask a question on either StackOverflow or Discord, we would first have to ask if the default order has been changed. Additionally, some packages/solutions/tutorials might stop working correctly if one decides to switch an execution order leading to some strange side-effect. So sadly, because I love the flexibility, this change would very likely bring us more problems than benefits. |
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. |
I'm submitting a...
Current behavior
Guards are executed after each middleware, but before any pipe.
Expected behavior
Being able to order them or use pipes before guards.
What is the motivation / use case for changing the behavior?
I am using nest in different applications and I am noticing in some cases guards are dependent of what is inside body. In those cases I would want to have the validationpipe before any guard so that if the request is not fully compliant with what we expect, block it from there.
I believe this will have some performance impacts too since guards are using services and talking to databases before making decisions so by running validationpipe before guards we can avoid unnecessary calls.
Thanks for this amazing framework.
The text was updated successfully, but these errors were encountered: