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

Correct usage of the query bus to get a strongly-typed result? (Maybe feature request.) #490

Open
ldiego08 opened this issue Nov 6, 2020 · 13 comments

Comments

@ldiego08
Copy link

ldiego08 commented Nov 6, 2020

I'm submitting a...


[ ] Regression 
[ ] Bug report
[x] Feature request?
[x] Documentation issue or request?
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Getting a strongly-typed result from QueryBus.execute() involves specifying the TRes type parameter, but since it's the second one, you also have to specify the type of the query (inferred when no type params are specified).

According to this answer #167 (comment), considering the following query...

class UsersQuery {
   constructor(
      readonly pageSize: number
   ) {}
}

class UsersQueryHandler implements IQueryHandler<UsersQuery> {
    async execute(): Promise<User[]> {
        // ..
    }
}

... the intended use if you want a strongly-typed result would be:

const users = await queryBus.execute<UsersQuery, User[]>(new UsersQuery(10));

Expected behavior

If TRes was the first parameter, it would be less verbose:

const users = await queryBus.execute<User[]>(new UsersQuery(10));

Examining a bit deeper, I started wondering why is not the result type inferred along with the query type, i.e.:

class UsersQuery implements IQuery<User[]> {
   constructor(
      readonly pageSize: number
   ) {}
}

class UsersQueryHandler implements IQueryHandler<UsersQuery> {
    async execute(): Promise<User[]> {
        // ..
    }
}

execute could potentially infer the type if its definition was:

async execute<TResult = any, TQuery extends IQuery<TResult>>(query TQuery): Promise<TResult>;

I'm aware that QueryBase is already being used as the generic type for the entire QueryBus class. Does that mean I should inject as many query buses as query types I'm intending to execute?

constructor(
    readonly usersQueryBus: QueryBus<UsersQuery>,
    readonly rolesQueryBus: QueryBus<RolesQuery>,
) {}

While the implementation hints so, docs specify otherwise: the QueryBus is a generic class. With all the above said, I have some questions:

  • Is there a particular reason QueryBase is a class-level type parameter in QueryBus instead of a method-level one in execute?

  • Would it be okay to move the TRes type param first?

Environment


Nest version: 7.5.0

 
For Tooling issues:
- Node version: 14
- Platform:  MacOS
@underfisk
Copy link
Contributor

I have checked and also asked for Generics, i think what you are asking is a basic thing that everyone would benefit and shouldn't be hard to implement but there are some PR's in standby that i see no one from staff paying attention to

@kgajowy
Copy link

kgajowy commented Mar 24, 2021

// a bit of helpers
export class Command<T> implements ICommand {
  private $resultType!: T
}

export class Query<T> implements IQuery {
  private resultType!: T
}
// declarations.d.ts - extension to @nestjs/cqrs
declare module '@nestjs/cqrs/dist/query-bus' {
  export interface QueryBus {
    execute<X>(query: Query<X>): Promise<X>
  }

  export type IInferringQueryHandler<
    QueryType extends Query<unknown>
  > = QueryType extends Query<infer ResultType>
    ? IQueryHandler<QueryType, ResultType>
    : never
}

declare module '@nestjs/cqrs/dist/command-bus' {
  export interface CommandBus {
    execute<X>(command: Command<X>): Promise<X>
  }

  export type IInferringCommandHandler<
    CommandType extends Command<unknown>
  > = CommandType extends Command<infer ResultType>
    ? {
        execute(command: CommandType): Promise<ResultType>
      }
    : never
}

Then

class SomeQuery extends Query<Dto[]> {
  constructor() {
    super()
  }
}


@QueryHandler(SomeQuery)
export class SomeQueryHandler
  implements IInferringQueryHandler<SomeQuery> { ... }

We often use such enhancement/path to not add additional generics but just letting to infer the result type.

(code credits to @Dyostiq)

@kgajowy
Copy link

kgajowy commented Apr 27, 2021

@ldiego08 and others:

we have released the package that may be useful to achieve the above:

https://github.com/valueadd-poland/nestjs-packages/tree/master/packages/typed-cqrs

@Sikora00
Copy link
Contributor

@kamilmysliwiec the solution provided by @kgajowy can be easily added to the nestjs/cqrs package with backward compatibility using overloading.

PoC nestjs-architects/cqrs@053ebff#diff-077f9c78eb3526c090ed295ae929fd52c7a9348c6b858566daafab0dc2bc70e3

@mattheiler
Copy link

mattheiler commented Nov 24, 2021

MediatR (.NET) has this and it is sorely missed in my NodeJS projects.

@aeoncleanse
Copy link

This would be extremely handy for project I am now working on. I think this should be bumped to higher priority, it would be a very well-received feature for reducing verbosity.

@Sikora00
Copy link
Contributor

@aeoncleanse you can already use this addon https://github.com/valueadd-poland/nestjs-packages/tree/master/packages/typed-cqrs

@mohit-singh-pepper
Copy link

mohit-singh-pepper commented Feb 15, 2022

Out of curiosity, Now that nestJS 8 is here are you planning to release a new version of this or it's supported ? @Sikora00

@Sikora00
Copy link
Contributor

@mohit-singh-pepper I can release it this week. Basically it's just a change to the package.json. It works the same with --force install

@Sikora00
Copy link
Contributor

@mohit-singh-pepper v1.0.0 released :)
I removed dependency on any specific version of @nestjs/cqrs

@chrislambe
Copy link

chrislambe commented May 23, 2022

In the meantime, I've been accomplishing this with the following (admittedly verbose) typing:

queryBus.execute<
  FooQuery,
  Awaited<ReturnType<FooQueryHandler['execute']>>
>(new FooQuery(fooId))

Awaited type comes from TypeScript 4.5.

@lennartquerter
Copy link

Is there any update here? We are really missing this functionality in our applications and would rather not depend on an external library for this. Seems like a very good option to add. I can also create the PR if needed.

@Sikora00 Sikora00 mentioned this issue Nov 1, 2023
12 tasks
@alexandru-dodon
Copy link

So was this abandoned or can we expect an update any time soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants