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

Wrong type (as in #12264) #13383

Closed
4 of 15 tasks
AlexRMU opened this issue Apr 1, 2024 · 3 comments · May be fixed by #13388
Closed
4 of 15 tasks

Wrong type (as in #12264) #13383

AlexRMU opened this issue Apr 1, 2024 · 3 comments · May be fixed by #13388
Labels
needs triage This issue has not been looked into

Comments

@AlexRMU
Copy link

AlexRMU commented Apr 1, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

In issue #12264, we discussed problems with types related to ReflectableDecorator and CreateDecoratorOptions.transform.
However, this has not been fixed everywhere. There may be other functions, but I only found Reflector.getAllAndOverride, Reflector.getAll and Reflector.getAllAndMerge.
Here is an example of a fix:

function getAllAndOverride<TParam = any, TTransformed = TParam, T extends ReflectableDecorator<TParam, TTransformed>>(
    decorator: T,
    targets: (Type<any> | Function)[],
): T extends ReflectableDecorator<infer RParam, infer RTransformed> ? RTransformed : unknown;
// to
function getAllAndOverride<
    TParam = any,
    TTransformed = TParam,
    T extends ReflectableDecorator<TParam, TTransformed> = ReflectableDecorator<any>,
>(
    decorator: T,
    targets: (Type<any> | Function)[],
): T extends ReflectableDecorator<infer RParam, infer RTransformed> ? RTransformed : unknown;

Minimum reproduction code

Steps to reproduce

No response

Expected behavior

Ideally, the typing system would be able to detect the return value on the transform property, and use that as the return value of the Refector methods.

Package

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

Other package

No response

NestJS version

10.3.7

Packages versions

10.3.7

Node.js version

20.8.1

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@AlexRMU AlexRMU added the needs triage This issue has not been looked into label Apr 1, 2024
@micalevisk
Copy link
Member

if you believe you have a fix to that, can you please open a PR?

@AlexRMU
Copy link
Author

AlexRMU commented Apr 2, 2024

Ok

@kamilmysliwiec
Copy link
Member

Let's track this here #13388

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants