feature(microservices): Return correct client type in ClientProxyFactory

[BrunnerLivio]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

At the moment ClientProxyFactory.create will always return the type ClientProxy & Closeable. Therefore it will not take the given transport option into consideration, even though we would know what type should be returned.

const client = ClientProxyFactory.create({ transport: Transport.GRPC, options: { ... }); // The type is `ClientProxy & Closeable`

// To get the correct type you have to use this workaround
const grpcClient = (client as any) as ClientGrpc;

On top of that ClientProxy & Closeable is straight up wrong with some clients, such as the GrpcClient.
Calling .connect() from ClientProxy & Closeable on ClientGrpc would result in this error: Error: The "connect()" method is not supported in gRPC mode.

What is the new behavior?

Adds overloads to the ClientProxyFactory for better type security.
This allows the user to not cast the client to any after creating it
using the factory.

const client = ClientProxyFactory.create({ transport: Transport.GRPC, options: { .. }) // The type is `ClientGrpcProxy`

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

The ClientProxyFactory is used in @nestjs/terminus to dynamically create a microservice client on the fly. Better type security would lead to a better code :)

[BrunnerLivio]

This line has to be "duplicated" so a parameter of type ClientOptions is still valid (used in microservices/module/ClientsModule)

[coveralls]

Pull Request Test Coverage Report for Build 2930

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 95.265%

---

Totals
Change from base Build 2927:	0.0%
Covered Lines:	3380
Relevant Lines:	3548

---

💛 - Coveralls

[kamilmysliwiec]

So actually the main idea behind ClientProxyFactory was to isolate, hide these types and set ClientProxy as a sort of contract. Hence, if you switch between Redis or NATS, you still interact with ClientProxy, regardless of what sits behind.

However, ClientGrpc is a different story because their interfaces don't overlap. As you can see in the docs https://docs.nestjs.com/microservices/grpc, we don't recommend using ClientProxy anymore. Instead, the type is explicitly set to ClientGrpc.

Could you change this PR so this method still returns ClientProxy when transport isn't gRPC, otherwise, return ClientGrpc? What do you think?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.