cannot get design-time type annotations with `Reflector`

Thore1954 commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

I need to access the handler's return type in my interceptor. As I understand it the reflect-metadata package provides design-time type annotations automatically when the compiler's emitDecoratorMetadata is turned on:

import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { Observable } from 'rxjs';

@Injectable()
export class MyInterceptor implements NestInterceptor {
  constructor(private reflector: Reflector) {}

  intercept(context: ExecutionContext, next: CallHandler): Observable<any> {
    const type = this.reflector.get('design:returntype', context.getHandler());
    console.log(type); // -> undefined
    return next.handle();
  }
}

But it always returns undefined.

Minimum reproduction code

https://stackblitz.com/edit/nestjs-typescript-starter-y6xc98

Steps to reproduce

No response

Expected behavior

It should return the design-time return type of the handler.

Package

[ ] I don't know. Or some 3rd-party package
[X] code>@nestjs/common</code
[X] code>@nestjs/core</code
[ ] code>@nestjs/microservices</code
[ ] code>@nestjs/platform-express</code
[ ] code>@nestjs/platform-fastify</code
[ ] code>@nestjs/platform-socket.io</code
[ ] code>@nestjs/platform-ws</code
[ ] code>@nestjs/testing</code
[ ] code>@nestjs/websockets</code
[X] Other (see below)

Other package

reflect-metadata

NestJS version

No response

Packages versions

{
    "@nestjs/common": "^9.0.0",
    "@nestjs/core": "^9.0.0",
    "@nestjs/platform-express": "^9.0.0",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^7.5.5"
  }

Node.js version

No response

In which operating systems have you tested?

[ ] macOS
[ ] Windows
[ ] Linux

Other

No response

Thore1954 commented 2 years ago

I found out it is possible to get the return type directly from Reflect:

const clazz = context.getClass();
const inst = new clazz();
const type = Reflect.getMetadata('design:returntype', inst, context.getHandler().name);
console.log(type); // -> String

Could this get added to the Reflector service? Is there a better way to do this?

jmcdo29 commented 2 years ago

If you use that same Reflect in your reproduction, you also get back undefined

Thore1954 commented 2 years ago

As a matter of fact my second approach is working:

I created a seperate link for it: https://stackblitz.com/edit/nestjs-typescript-starter-8lcshr I just want to know if there's a better way of doing it since it seems hacky and I expected it to work with Reflector.

micalevisk commented 2 years ago

I expected it to work with Reflector

I believe that we have a misunderstanding on what's the role of that utility class. Reflector#get just calls Reflect.getMetadata with few arguments, ie., it doesn't covers all use cases of reflect-metadata

https://github.com/nestjs/nest/blob/f4e9ac6208f3e7ee7ad44c3de713c9086f657977/packages/core/services/reflector.service.ts#L26

Since reflect-metadata is a hard dependency of your project, using Reflect.getMetadata directly like you did is pretty fine. Thus, I don't think we should bring that to Reflector otherwise we would end up creating a 'weak facade' to Reflect.getMetadata

So this is not a bug but a feature request.

jmcdo29 commented 2 years ago

As a matter of fact my second approach is working:

You're correct, I was under the assumption that context.getClass() would return what was necessary, not needing to actually instantiate the class again

Thore1954 commented 2 years ago

Since reflect-metadata is a hard dependency of your project, using Reflect.getMetadata directly like you did is pretty fine.

That's fair. closing this.

Just for the record, is there a better way of getting type annotations with what ExecutionContext provides? I saw that you're supplying the controller instance to get type annotations for pipes: https://github.com/nestjs/nest/blob/f4e9ac6208f3e7ee7ad44c3de713c9086f657977/packages/core/helpers/context-utils.ts#L29-L34 Is there a reason it's not supplied for us? Reflect.getMetadata is supposed to work with instances.

Also, this might not be related to nest, but why does this.reflector.get('roles', context.getHandler()); work but this.reflector.get('design:returntype', context.getHandler()); doesnt?

micalevisk commented 2 years ago

Is there a reason it's not supplied for us?

I don't have a clue. You might want to open another Issue requesting that instance inside ExecutionContextHost

but this.reflector.get('design:returntype', context.getHandler()); doesnt?

that's due to how reflect-metadata works. See https://rbuckton.github.io/reflect-metadata/#ordinarygetmetadata Also, this.reflector.get('roles', context.getHandler()) works because you have called Reflect.defineMetadata('roles', methodFn) beforehand

nestjs / nest