nestjs / cqrs

A lightweight CQRS module for Nest framework (node.js) :balloon:
https://nestjs.com
MIT License
849 stars 150 forks source link

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

Open ldiego08 opened 4 years ago

ldiego08 commented 4 years ago

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 https://github.com/nestjs/cqrs/issues/167#issuecomment-628462104, 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:

Environment


Nest version: 7.5.0


For Tooling issues:
- Node version: 14
- Platform:  MacOS            
kgajowy commented 3 years ago
// 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 commented 3 years ago

@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 commented 3 years ago

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

PoC https://github.com/nestjs-architects/cqrs/commit/053ebff8e6a3f95be9514d0d219d989a3585051b#diff-077f9c78eb3526c090ed295ae929fd52c7a9348c6b858566daafab0dc2bc70e3

mattheiler commented 2 years ago

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

aeoncleanse commented 2 years ago

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 commented 2 years ago
mohit-singh-pepper commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

chrislambe commented 2 years ago

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 commented 1 year ago

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.

alexandru-dodon commented 9 months ago

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

ajfranzoia commented 4 months ago

@kamilmysliwiec if a PR was created for this, would you consider it? Thanks