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

feat: support unwrapped value type rpc method arguments #979

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
6 changes: 3 additions & 3 deletions integration/grpc-web/example.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable */

Check failure on line 1 in integration/grpc-web/example.ts

View workflow job for this annotation

GitHub Actions / build (20.x)

Argument of type '{ value: string | undefined; }' is not assignable to parameter of type '{ value?: string; } & { value?: string; } & {}' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
import { grpc } from "@improbable-eng/grpc-web";
import { BrowserHeaders } from "browser-headers";
import * as _m0 from "protobufjs/minimal";
Expand Down Expand Up @@ -827,7 +827,7 @@
Create(request: DeepPartial<DashAPICredsCreateReq>, metadata?: grpc.Metadata): Promise<DashCred>;
Update(request: DeepPartial<DashAPICredsUpdateReq>, metadata?: grpc.Metadata): Promise<DashCred>;
Delete(request: DeepPartial<DashAPICredsDeleteReq>, metadata?: grpc.Metadata): Promise<DashCred>;
Uppercase(request: DeepPartial<StringValue>, metadata?: grpc.Metadata): Promise<StringValue>;
Uppercase(request: string | undefined, metadata?: grpc.Metadata): Promise<StringValue>;
}

export class DashAPICredsClientImpl implements DashAPICreds {
Expand All @@ -853,8 +853,8 @@
return this.rpc.unary(DashAPICredsDeleteDesc, DashAPICredsDeleteReq.fromPartial(request), metadata);
}

Uppercase(request: DeepPartial<StringValue>, metadata?: grpc.Metadata): Promise<StringValue> {
return this.rpc.unary(DashAPICredsUppercaseDesc, StringValue.fromPartial(request), metadata);
Uppercase(request: string | undefined, metadata?: grpc.Metadata): Promise<StringValue> {
return this.rpc.unary(DashAPICredsUppercaseDesc, StringValue.fromPartial({ value: request }), metadata);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('wrappers in service methods', () => {
it('generates a services that compiles', () => {
let c: Clock = {
Now: () => Promise.resolve(Timestamp.fromPartial({ seconds: 0, nanos: 0 })),
NowString: (inp: StringValue) => Promise.resolve(inp),
NowString: (inp: string | undefined) => Promise.resolve(StringValue.fromPartial({ value: inp })),
NowStringStream: (inp: Observable<StringValue>) => inp,
NowBool: () => Promise.resolve(BoolValue.fromPartial({ value: true }))
};
Expand Down
6 changes: 3 additions & 3 deletions integration/wrappers-regression/wrappers-regression.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable */

Check failure on line 1 in integration/wrappers-regression/wrappers-regression.ts

View workflow job for this annotation

GitHub Actions / build (20.x)

Argument of type '{ value: string | undefined; }' is not assignable to parameter of type '{ value?: string; } & { value?: string; } & {}' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
import * as _m0 from "protobufjs/minimal";
import { Observable } from "rxjs";
import { map } from "rxjs/operators";
Expand All @@ -10,7 +10,7 @@

export interface Clock {
Now(request: Empty): Promise<Timestamp>;
NowString(request: StringValue): Promise<StringValue>;
NowString(request: string | undefined): Promise<StringValue>;
NowStringStream(request: Observable<StringValue>): Observable<StringValue>;
NowBool(request: Empty): Promise<BoolValue>;
}
Expand All @@ -33,8 +33,8 @@
return promise.then((data) => Timestamp.decode(_m0.Reader.create(data)));
}

NowString(request: StringValue): Promise<StringValue> {
const data = StringValue.encode(request).finish();
NowString(request: string | undefined): Promise<StringValue> {
const data = StringValue.encode(StringValue.fromPartial({ value: request })).finish();
const promise = this.rpc.request(this.service, "NowString", data);
return promise.then((data) => StringValue.decode(_m0.Reader.create(data)));
}
Expand Down
10 changes: 6 additions & 4 deletions src/generate-grpc-web.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MethodDescriptorProto, FileDescriptorProto, ServiceDescriptorProto } from "ts-proto-descriptors";
import { rawRequestType, requestType, responsePromiseOrObservable, responseType, observableType } from "./types";
import { rawRequestType, requestType, responsePromiseOrObservable, responseType, observableType, valueTypeName} from "./types";
import { Code, code, imp, joinCode } from "ts-poet";
import { Context } from "./context";
import { assertInstanceOf, FormattedMethodDescriptor, maybePrefixPackage } from "./utils";
Expand Down Expand Up @@ -50,8 +50,10 @@ function generateRpcMethod(ctx: Context, serviceDesc: ServiceDescriptorProto, me
assertInstanceOf(methodDesc, FormattedMethodDescriptor);
const { options } = ctx;
const { useAbortSignal } = options;
const requestMessage = requestType(ctx, methodDesc, false);
const inputType = requestType(ctx, methodDesc, true);
const isValueType = valueTypeName(ctx, methodDesc.inputType) !== undefined;
const inputType = requestType(ctx, methodDesc, true && !isValueType, false);
const inputValue = isValueType ? '{ value: request }' : 'request';
const requestMessage = requestType(ctx, methodDesc, false, true);
const returns = responsePromiseOrObservable(ctx, methodDesc);

if (methodDesc.clientStreaming) {
Expand All @@ -75,7 +77,7 @@ function generateRpcMethod(ctx: Context, serviceDesc: ServiceDescriptorProto, me
): ${returns} {
return this.rpc.${method}(
${methodDescName(serviceDesc, methodDesc)},
${requestMessage}.fromPartial(request),
${requestMessage}.fromPartial(${inputValue}),
metadata,
${useAbortSignal ? "abortSignal," : ""}
);
Expand Down
11 changes: 8 additions & 3 deletions src/generate-services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
responsePromiseOrObservable,
responseType,
observableType,
valueTypeName,
} from "./types";
import {
assertInstanceOf,
Expand Down Expand Up @@ -57,8 +58,9 @@ export function generateService(

// the grpc-web clients auto-`fromPartial` the input before handing off to grpc-web's
// serde runtime, so it's okay to accept partial results from the client
const isValueType = valueTypeName(ctx, methodDesc.inputType) !== undefined;
const partialInput = options.outputClientImpl === "grpc-web";
const inputType = requestType(ctx, methodDesc, partialInput);
const inputType = requestType(ctx, methodDesc, partialInput && !isValueType, !isValueType || methodDesc.clientStreaming);
params.push(code`request: ${inputType}`);

// Use metadata as last argument for interface only configuration
Expand Down Expand Up @@ -109,7 +111,8 @@ function generateRegularRpcMethod(ctx: Context, methodDesc: MethodDescriptorProt
const { options } = ctx;
const Reader = impFile(ctx.options, "Reader@protobufjs/minimal");
const rawInputType = rawRequestType(ctx, methodDesc, { keepValueType: true });
const inputType = requestType(ctx, methodDesc);
const isValueType = valueTypeName(ctx, methodDesc.inputType) !== undefined;
const inputType = requestType(ctx, methodDesc, false, !isValueType || methodDesc.clientStreaming);
const rawOutputType = responseType(ctx, methodDesc, { keepValueType: true });

const params = [
Expand All @@ -130,7 +133,9 @@ function generateRegularRpcMethod(ctx: Context, methodDesc: MethodDescriptorProt
`;
}

let encode = code`${rawInputType}.encode(request).finish()`;
let encode = isValueType && !methodDesc.clientStreaming
? code`${rawInputType}.encode(${rawInputType}.fromPartial({ value: request })).finish()`
: code`${rawInputType}.encode(request).finish()`;
let beforeRequest;
if (options.rpcBeforeRequest) {
beforeRequest = code`
Expand Down
9 changes: 7 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -734,8 +734,13 @@ export function observableType(ctx: Context, asType: boolean = false): Code {
}
}

export function requestType(ctx: Context, methodDesc: MethodDescriptorProto, partial: boolean = false): Code {
let typeName = rawRequestType(ctx, methodDesc, { keepValueType: true });
export function requestType(
ctx: Context,
methodDesc: MethodDescriptorProto,
partial: boolean = false,
keepValueType: boolean = true
): Code {
let typeName = rawRequestType(ctx, methodDesc, { keepValueType: keepValueType});

if (partial) {
typeName = code`${ctx.utils.DeepPartial}<${typeName}>`;
Expand Down
Loading