stephenh / ts-proto

An idiomatic protobuf generator for TypeScript
Apache License 2.0
2.09k stars 340 forks source link

Update NestJS to also allow partial messages #486

Open mpx2m opened 2 years ago

mpx2m commented 2 years ago

If there is proto like this:

syntax = "proto3";
package proto.v1;

service TestService {
    rpc Login (LoginRequest) returns (LoginResponse);
}

message LoginRequest {
    string name = 1;
    string password = 2;
    string phone = 3;
    string email = 4;
    string verification_code = 5;
    string card_no = 6;
    string reader = 7;
    string credential = 8;
    string redirect = 9;
}

message LoginResponse {
    string auth_code = 1;
}

after

"protoc --plugin=./node_modules/.bin/protoc-gen-ts_proto --ts_proto_opt=outputPartialMethods=true --ts_proto_opt=nestJs=true --ts_proto_opt=env=node --ts_proto_opt=addGrpcMetadata=true --ts_proto_opt=useOptionals=messages --ts_proto_opt=forceLong=string --ts_proto_opt=unrecognizedEnum=false --ts_proto_out=. src/**/*.proto",

produce

/* eslint-disable */
import { GrpcMethod, GrpcStreamMethod } from "@nestjs/microservices";
import { util, configure } from "protobufjs/minimal";
import * as Long from "long";
import { Observable } from "rxjs";
import { Metadata } from "@grpc/grpc-js";

export const protobufPackage = "proto.v1";

export interface LoginRequest {
  name: string;
  password: string;
  phone: string;
  email: string;
  verificationCode: string;
  cardNo: string;
  reader: string;
  credential: string;
  redirect: string;
}

export interface LoginResponse {
  authCode: string;
}

export const PROTO_V1_PACKAGE_NAME = "proto.v1";

export interface TestServiceClient {
  login(request: LoginRequest, metadata?: Metadata): Observable<LoginResponse>;
}

export interface TestServiceController {
  login(
    request: LoginRequest,
    metadata?: Metadata
  ): Promise<LoginResponse> | Observable<LoginResponse> | LoginResponse;
}

export function TestServiceControllerMethods() {
  return function (constructor: Function) {
    const grpcMethods: string[] = ["login"];
    for (const method of grpcMethods) {
      const descriptor: any = Reflect.getOwnPropertyDescriptor(
        constructor.prototype,
        method
      );
      GrpcMethod("TestService", method)(
        constructor.prototype[method],
        method,
        descriptor
      );
    }
    const grpcStreamMethods: string[] = [];
    for (const method of grpcStreamMethods) {
      const descriptor: any = Reflect.getOwnPropertyDescriptor(
        constructor.prototype,
        method
      );
      GrpcStreamMethod("TestService", method)(
        constructor.prototype[method],
        method,
        descriptor
      );
    }
  };
}

export const TEST_SERVICE_NAME = "TestService";

// If you get a compile-error about 'Constructor<Long> and ... have no overlap',
// add '--ts_proto_opt=esModuleInterop=true' as a flag when calling 'protoc'.
if (util.Long !== Long) {
  util.Long = Long as any;
  configure();
}

In the NestJS code, if only name and password to be used in the login method, typescript code will be writed like blew to meet generated interfaces requirement, many fields have to be filled with undefined:

this.testServiceClient.login(
    name: "testName",
    password: "testPassword",
    phone: undefined,
    email: undefined,
    verificationCode: undefined,
    cardNo: undefined,
    reader: undefined,
    credential: undefined,
    redirect: undefined
)

So it would be nicer to allow request message fields to be optional:

export interface LoginRequest {
  name?: string;
  password?: string;
  phone?: string;
  email?: string;
  verificationCode?: string;
  cardNo?: string;
  reader?: string;
  credential?: string;
  redirect?: string;
}

but still keep response message as normal (not optional)

export interface LoginResponse {
  authCode: string;
}

above ts code will be simplified like:

this.testServiceClient.login(
    name: "testName",
    password: "testPassword"
)
stephenh commented 2 years ago

Yep, our grpc-js services already do this, they accept DeepPartial versions of requests. We just haven't had anyone get around to porting that over to the NestJS services.

...and I assume that would work, i.e. I'm pretty sure we'd still need to invoke FooMessage.fromPartial within the generated client method. I just haven't looked at the NestJS code in awhile.

A PR would be great.

mpx2m commented 2 years ago

@stephenh type-fest package would be great to implement DeepPartial

the generated code show above:

[...]
export interface TestServiceClient {
  login(request: LoginRequest, metadata?: Metadata): Observable<LoginResponse>;
}
[...]

After set the flag (new feature) for generating DeepPartial version requests

import { PartialDeep } from 'type-fest'

[...]
export interface TestServiceClient {
  login(request: PartialDeep<LoginRequest>, metadata?: Metadata): Observable<LoginResponse>;
}
[...]
stephenh commented 2 years ago

@mpx2m yeah, our grpc-js (or grpc-web...or both...I forget) services already output their own version of DeepPartial / PartialDeep, to avoid a dependency on type-fest.

It really is a matter of "we have two sets of code for generating service methods, one for grpc-js, one for NestJS, and only the grpc-js uses a DeepPartial".

I don't have time to fix it myself, but if you poke around at the grpc-js/grpc-web service generation, you should be able to update NestJS to do the same.