From 16ad075d7809200796c612753a3c060fe6bfb71e Mon Sep 17 00:00:00 2001 From: Zak Henry Date: Wed, 16 Mar 2022 09:39:14 +1300 Subject: [PATCH] fix(Grpc-Web): Fix compilation failure when a service definition contains a client streaming call. Note this does not implement client streaming, instead the function will throw due to the feature not being implemented. This is a stop-gap until true client streaming support is added. --- integration/grpc-web/example-test.ts | 12 ++++++++++- integration/grpc-web/example.bin | Bin 3050 -> 3278 bytes integration/grpc-web/example.proto | 2 ++ integration/grpc-web/example.ts | 13 ++++++++++++ src/generate-grpc-web.ts | 29 +++++++++++++++++++-------- src/generate-services.ts | 6 ++---- src/main.ts | 4 +++- src/types.ts | 7 ++++++- 8 files changed, 58 insertions(+), 15 deletions(-) diff --git a/integration/grpc-web/example-test.ts b/integration/grpc-web/example-test.ts index ae6cff407..c2c00bae6 100644 --- a/integration/grpc-web/example-test.ts +++ b/integration/grpc-web/example-test.ts @@ -1,4 +1,5 @@ import { DashStateClientImpl } from './example'; +import { EMPTY } from 'rxjs'; describe('grpc-web', () => { it('at least compiles', () => { @@ -19,5 +20,14 @@ describe('grpc-web', () => { const client = new DashStateClientImpl(rpc); const userSettings = client.UserSettings; userSettings({}); - }) + }); + it('throws on client streaming call', () => { + const rpc = { + unary: jest.fn(), + invoke: jest.fn(), + }; + const client = new DashStateClientImpl(rpc); + const call = () => client.ChangeUserSettingsStream(EMPTY) + expect(call).toThrowError("ts-node does not yet support client streaming!") + }); }); diff --git a/integration/grpc-web/example.bin b/integration/grpc-web/example.bin index ace3893cc1b3f18219323162dea103d7fad2ebc6..72f331cf3e1bc492cb01c75424a58d5c31f7426c 100644 GIT binary patch literal 3278 zcmbtXYjfK~6qQzMSzbRPuM;YJ@(>0ZhpC;A#BK9nnzSh$p2IlJz_*TLtBwc9HkJY{ zU+5p=3qJ||2RwJqL2EjjM3b81SZ>8L>&7p? zSH;bkv}V&+^@MYJkvKDxeh(bb-|<8)4*eRRobq!Kxygx)^mh9 zaYFrp;DlsRJP1#tZWPExkqIul!;z{d0dhz1tJx^@Y*durUegu*jo&)!d=+-$WZ4ke zm(*bpdRgBBucUc#=bbcMdoDQf)Y$L0|eJ-w(pxY&eaEld;O5eQ^<>bZ_Sy#7YbD%CxkqQ+Ra zDx4lpdJ&?GL^MnIbohV!O*+fy&K}?F>?9Zk|5`z!kvK{2WU+mh)9Cf~pJuZC$-dne zh503tBHocWvsTmlGM|$0PK0;di-tc1H$(9J^1o_qneuz#J-O(P2e+xbC|wA>|`V4wQw5Lt5PfKmhrAnq-5N;kn|qhLP{od3rUaeEz%cnTwzOY z7$>f9gjtZ5VMuyn(qcwI2*GSaa&F{IJX^M5N{c&Xkr&)Hm?5nU+ZP3qA%IZEmMMZj zrsAc@0{Y;gq9M^{U#pNw$X-nmX!hENDYAh!+bM#Geg8}GI6fPR4ja_k{&*7k;nj3H znMFb0ACCROY}#x79Gu_t&#w@=m|Tteei#i$BfmGfoDN5UP>Kv^JNj&v0rKyrF*;!V zFh!`ud6w49P>J(RkKEyKCvPmp?r^dopSZ(gcM7ab0t6slU`~pVSjZJp1i8Y3OYR^O zp(1lt$i%rtW~Yc^*?fvXE0z|3K%+Vq*rJuh;z|~fSrj(KXi_ehq9n2jgpkXN*BH&Z zZds=zT;{G0(~QbIe~pn|#>uHVpqx{q79@9cI-_{Lpwm%zg_m^LLVAVgt})Upg<|Zk zDfu14Pu!&{?l>#4O(u2Zu}a)70NnMIyFf_va-}qeRi6{h-T7EH&L8P{{oFa(S)_fp{SQybVTT4a+gv@0$O(p_S zLo0kWb|HpnX@!exjP#X!^%_HZWqC!@5ni36o1m}G(W$xBIXbns3i?xAaF~48_}HxN zHE;bBKE78zeV0OUYeEyF!nc8rxa%yEV|F~Bb@qUKAb`-i9q&6KxVc`er!iEr-fSxx z5*wOEiVaPpZ&E|ksDXy2A+(`sw0|3#Mzh<{G`hMQpwZQ3lHW5vNtTN^_nhPoGMYt` zRpTK7z{;jtE+8Z}bIG`YAlJO(#aqHiqs87+G@3w5)2NkJF8LZD5wzCglA@u&O|{C5 z(N}0w(@1httukVuZK_p{!IoNO0MNEHjjq&IF8NR(2HF-@nR>>>+%fJaJyX_>x={Gq zOKVs43;?-ZB?pAWUDY!Xw5t$Icbv^ILCD!AkijZc#*IIXTo^qfJ~Bh_<;dOlKB0HBH^B?pAW zBUJ?uZ>OtT|v$5N7 ze*R-oT#rd_GJezeMTE!Q^!4ZG_>qzDF4D8uU3Kq;9eR`-f@g2XNzCx7+MaEJ(z5xj z=LmIFLSt8OLb4*l(_ue(H5exS$dM&cJh|-kk~EE683_J;cQTBa3`ODDWHOpWUgJ+! z%u~hFG?|}!n27Rpof!Qq>!PD773pW{%^m!vx4pH>GjTzlk?=g3Um$aB)L<>_Fas#x9#6tz5ROM zc0_r4iKI|FqB84syD!rzns+SXC%tU&Q*u27&oBOKjgQ>+@2)7*j^y5A(S1EPd3AUm zv;(bItT4Rm6G;!{29llD4J7U34J2#uJEX7vstR9n$9WowK)7Y;IgX?!COzhqg%Hel zBH@D9pw=@_;_vGc+Xn>{}CZ3Hi5k z1e$;6ZjOAQ2aj@uBL2?Tq=`dQ=*%ijoDL4PCayFIRwD|5g7gFna)d+?mUD!1VkRO@ z$V8~bA`^0PZi)FhqFAw%BhX6K86eO|M+K5~Oev`#vm$(okv~-~MO73vM4>9LTwyS) zQO%|!Ji{X!CTC`N=?Wu#1}Ep$0i&sE)I!7qo6aa+D%*7EuJNi3dq}VG@CqZnR<0;_ zUCHO2+uB`HF(2GgHo4T%0-IIs0>Iq`<1P>q7s6^D!*L5WX=n&78h2er?k*a4fhcC& z1p;jmJL6wxn2L-O5;;PVxHOj|6p80oa)cuBomC(d@i3w~yQN12gv>gcCKG|E zp=Ewsxu76gTIS*kBYnBFbcIoRdF7U+BfK(2cR^p7qEmA#Q*>%?1@vdQRxtUlbI+~s zH*fqCtN2Q+($^prHzqVOD%?mq;%+cUj`?anYiyT%Ab`-CulAh~++3?P@)(q?H6Ixo z64xz_Dy~}^eF@erjT%_DG=w%RjrMQD(r9)Ymc~Ha0FACLll;E(Kra_@?g#n~GMdFE zTT(*=fR&qOxqy(k8R~HZq1@(TthR(vjV-odXf%N>OQTk{Lj5&BB4}H4^M(e2O|#03 zG0>WpMkSkOl@SB2X;xW*mRV&0&{~#8SE?224+UbNwXn+6vnTmO=VRS7Wj!<(3g1}i zwN1|eDAzXSfRNZWJp-X!+w|;_z-`kr0BGBmMm=wvo)H6W8$FW>F8Pl0iB`dpwPRFp zQn72w0ifKjDF=kaT~iJS<#vsQoK)-?@c^LhSsJO>GvW~gZO@43v>N;NG;)65(x~LV zJ&l~-x2FLd80Wci{=m}6(*xr?Vw7o|FG&8_`Be8@K-Oc^bAft3G%5g~;?R@>LgJxO y0fcggMn!=H9vKw?pdDEn^?YPhAO_kIRD^IIAFHQMME4b!6wg8BrLX?8-~JoJNTa|2 diff --git a/integration/grpc-web/example.proto b/integration/grpc-web/example.proto index 62ba9421d..f5ffd02b2 100644 --- a/integration/grpc-web/example.proto +++ b/integration/grpc-web/example.proto @@ -5,6 +5,8 @@ package rpx; service DashState { rpc UserSettings(Empty) returns (DashUserSettingsState); rpc ActiveUserSettingsStream(Empty) returns (stream DashUserSettingsState); + // not supported in grpc-web, but should still compile + rpc ChangeUserSettingsStream (stream DashUserSettingsState) returns (stream DashUserSettingsState) {} } message DashFlash { diff --git a/integration/grpc-web/example.ts b/integration/grpc-web/example.ts index f37b0f1e9..270472a04 100644 --- a/integration/grpc-web/example.ts +++ b/integration/grpc-web/example.ts @@ -595,6 +595,11 @@ export const Empty = { export interface DashState { UserSettings(request: DeepPartial, metadata?: grpc.Metadata): Promise; ActiveUserSettingsStream(request: DeepPartial, metadata?: grpc.Metadata): Observable; + /** not supported in grpc-web, but should still compile */ + ChangeUserSettingsStream( + request: Observable>, + metadata?: grpc.Metadata + ): Observable; } export class DashStateClientImpl implements DashState { @@ -604,6 +609,7 @@ export class DashStateClientImpl implements DashState { this.rpc = rpc; this.UserSettings = this.UserSettings.bind(this); this.ActiveUserSettingsStream = this.ActiveUserSettingsStream.bind(this); + this.ChangeUserSettingsStream = this.ChangeUserSettingsStream.bind(this); } UserSettings(request: DeepPartial, metadata?: grpc.Metadata): Promise { @@ -613,6 +619,13 @@ export class DashStateClientImpl implements DashState { ActiveUserSettingsStream(request: DeepPartial, metadata?: grpc.Metadata): Observable { return this.rpc.invoke(DashStateActiveUserSettingsStreamDesc, Empty.fromPartial(request), metadata); } + + ChangeUserSettingsStream( + request: Observable>, + metadata?: grpc.Metadata + ): Observable { + throw new Error('ts-node does not yet support client streaming!'); + } } export const DashStateDesc = { diff --git a/src/generate-grpc-web.ts b/src/generate-grpc-web.ts index 0ffbc2f96..6e74f47e9 100644 --- a/src/generate-grpc-web.ts +++ b/src/generate-grpc-web.ts @@ -1,5 +1,5 @@ import { MethodDescriptorProto, FileDescriptorProto, ServiceDescriptorProto } from 'ts-proto-descriptors'; -import { requestType, responsePromiseOrObservable, responseType } from './types'; +import { rawRequestType, requestType, responsePromiseOrObservable, responseType } from './types'; import { Code, code, imp, joinCode } from 'ts-poet'; import { Context } from './context'; import { assertInstanceOf, FormattedMethodDescriptor, maybePrefixPackage } from './utils'; @@ -35,7 +35,7 @@ export function generateGrpcClientImpl( assertInstanceOf(methodDesc, FormattedMethodDescriptor); chunks.push(code`this.${methodDesc.formattedName} = this.${methodDesc.formattedName}.bind(this);`); } - chunks.push(code`}\n`); + chunks.push(code`}`); // Create a method for each FooService method for (const methodDesc of serviceDesc.method) { @@ -43,25 +43,38 @@ export function generateGrpcClientImpl( } chunks.push(code`}`); - return joinCode(chunks, { trim: false }); + return joinCode(chunks, { trim: false, on: '\n' }); } /** Creates the RPC methods that client code actually calls. */ function generateRpcMethod(ctx: Context, serviceDesc: ServiceDescriptorProto, methodDesc: MethodDescriptorProto) { assertInstanceOf(methodDesc, FormattedMethodDescriptor); - const { options, utils } = ctx; - const inputType = requestType(ctx, methodDesc); - const partialInputType = code`${utils.DeepPartial}<${inputType}>`; + const requestMessage = rawRequestType(ctx, methodDesc) + const inputType = requestType(ctx, methodDesc, true); const returns = responsePromiseOrObservable(ctx, methodDesc); + + if (methodDesc.clientStreaming) { + + return code` + ${methodDesc.formattedName}( + request: ${inputType}, + metadata?: grpc.Metadata, + ): ${returns} { + throw new Error('ts-node does not yet support client streaming!'); + } + `; + + } + const method = methodDesc.serverStreaming ? 'invoke' : 'unary'; return code` ${methodDesc.formattedName}( - request: ${partialInputType}, + request: ${inputType}, metadata?: grpc.Metadata, ): ${returns} { return this.rpc.${method}( ${methodDescName(serviceDesc, methodDesc)}, - ${inputType}.fromPartial(request), + ${requestMessage}.fromPartial(request), metadata, ); } diff --git a/src/generate-services.ts b/src/generate-services.ts index b7001695f..e54c379ed 100644 --- a/src/generate-services.ts +++ b/src/generate-services.ts @@ -51,12 +51,10 @@ export function generateService( params.push(code`ctx: Context`); } - let inputType = requestType(ctx, methodDesc); // 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 - if (options.outputClientImpl === 'grpc-web') { - inputType = code`${utils.DeepPartial}<${inputType}>`; - } + const partialInput = options.outputClientImpl === 'grpc-web' + const inputType = requestType(ctx, methodDesc, partialInput); params.push(code`request: ${inputType}`); // Use metadata as last argument for interface only configuration diff --git a/src/main.ts b/src/main.ts index 9d1abdf63..a12c03764 100644 --- a/src/main.ts +++ b/src/main.ts @@ -231,7 +231,9 @@ export function generateFile(ctx: Context, fileDesc: FileDescriptorProto): [stri chunks.push(generateGrpcClientImpl(ctx, fileDesc, serviceDesc)); chunks.push(generateGrpcServiceDesc(fileDesc, serviceDesc)); serviceDesc.method.forEach((method) => { - chunks.push(generateGrpcMethodDesc(ctx, serviceDesc, method)); + if (!method.clientStreaming) { + chunks.push(generateGrpcMethodDesc(ctx, serviceDesc, method)); + } if (method.serverStreaming) { hasServerStreamingMethods = true; } diff --git a/src/types.ts b/src/types.ts index 00d00ccad..f5197478c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -652,8 +652,13 @@ export function rawRequestType(ctx: Context, methodDesc: MethodDescriptorProto): return messageToTypeName(ctx, methodDesc.inputType); } -export function requestType(ctx: Context, methodDesc: MethodDescriptorProto): Code { +export function requestType(ctx: Context, methodDesc: MethodDescriptorProto, partial: boolean = false): Code { let typeName = rawRequestType(ctx, methodDesc); + + if (partial) { + typeName = code`${ctx.utils.DeepPartial}<${typeName}>` + } + if (methodDesc.clientStreaming) { return code`${imp('Observable@rxjs')}<${typeName}>`; }