nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript πŸš€
https://nestjs.com
MIT License
67.29k stars 7.59k forks source link

Better type inference of createMicroservice and connectMicroservice #11045

Closed jbl428 closed 1 year ago

jbl428 commented 1 year ago

Is there an existing issue that is already proposing this?

Is your feature request related to a problem? Please describe it

When using connectMicroservice and createMicroservice from @nest/microservice, we must explicitly provide the options type for type safety.

  await NestFactory.createMicroservice<TcpOptions>(AppModule, {
    transport: Transport.TCP,
    options: {
      host: 'localhost',
    },
  });

  app.connectMicroservice<GrpcOptions>({
    transport: Transport.GRPC,
  });

In contrast, the register method from ClientsModule has convenient type inference based on the specified Transport.

imports: [
  ClientsModule.register([ 
    {
      name: 'HERO_PACKAGE',
      transport: Transport.GRPC, // type is automatically infered as GrpcOptions
      options: {
        package: 'hero',
        protoPath: join(__dirname, 'hero/hero.proto'),
        foo: 'bar' // type error
      },
    },
  ]),
];

Describe the solution you'd like

It would be great if connectMicroservice could also provide convenient type inference using the type declaration of ClientsModule, if possible.

If this is not feasible, I suggest updating the documentation in each microservice section.

ex) Grpc docs

  const app = await NestFactory.createMicroservice<GrpcOptions>(AppModule, {
    transport: Transport.GRPC,
    options: {
      package: 'hero',
      protoPath: join(__dirname, 'hero/hero.proto'),
    },
  });

Teachability, documentation, adoption, migration strategy

What is the motivation / use case for changing the behavior?

In the Nest.js documentation, there is an example on how to create a gRPC server.

https://docs.nestjs.com/microservices/grpc

CleanShot 2023-02-05 at 11 09 44@2x

Although it uses MicroserviceOptions as the generic type, this does not allow for auto-completion or type inference for gRPC options.

kamilmysliwiec commented 1 year ago

Would you like to create a PR for this issue?

jbl428 commented 1 year ago

Yes, I would like to work on it. :)

jbl428 commented 1 year ago

@kamilmysliwiec

I would like to hear opinions before creating a pull request.

While reviewing the code, I discovered that resolving this issue requires moving the Transport.enum.ts, microservice-configuration.interface.ts, and other related interfaces from @nestjs/microservice to @nestjs/common.

In the Git history, these files were previously located in @nestjs/common but were removed in this commit e8da23cd Hence, moving them back is not a ideal option.

An alternative solution could be to create new interface files specifically for the server in the @nestjs/common package. However, this may result in duplicated code, as the Transport enum would be duplicated.

Another option is to simply update the documentation to guide users in using the correct type of interface. what do you think?

kamilmysliwiec commented 1 year ago

Another option is to simply update the documentation to guide users in using the correct type of interface.

Sounds good @jbl428!

jbl428 commented 1 year ago

I have discovered that type inference works correctly when providing the MicroserviceOptions type in the createMicroservice method. I suspect that the issue was due to a temporary problem with my IDE causing it not to function properly. Therefore, it seems appropriate to close this issue. πŸ˜„

Instead, there are some test codes where the generic type is not provided and incorrect options are used. so, I will create a pull request to address this issue.