fix(Grpc-Web): Fix compilation failure when a service definition contains a client streaming call. #535
Conversation
zakhenry
force-pushed
the
fix/grpc-web-streaming-compilation-failure
branch
from
3d961fc
to
16ad075
Compare
There was a problem hiding this comment.
@zakhenry thanks for the PR! I agree this is a good baby step / mitigation until we have full support. I just had one comment about ts-node/ts-proto typo, but otherwise looks great! Ping when that's updated and I'll hit merge.
Thanks!
| inputType = code`${utils.DeepPartial}<${inputType}>`; | ||
| } | ||
| const partialInput = options.outputClientImpl === 'grpc-web' | ||
| const inputType = requestType(ctx, methodDesc, partialInput); |
| const inputType = requestType(ctx, methodDesc); | ||
| const partialInputType = code`${utils.DeepPartial}<${inputType}>`; | ||
| const requestMessage = rawRequestType(ctx, methodDesc) | ||
| const inputType = requestType(ctx, methodDesc, true); |
There was a problem hiding this comment.
Instead of invoking both rawRequestType + requestType, I wonder if requestType would return a tuple or something...
Just thinking out loud, b/c we also have an open issue where someone's input type is not actually a message, so we need to make the fromPartial call optional, and maybe that'd be nice to handle inside of requestType itself... 🤷
There was a problem hiding this comment.
Yea I think there is a bit of a code smell that the requestType function manages the wrapping of the type into partial, but it was a minimal change. I suspect it might need a bit of a rethink for that other issue
integration/grpc-web/example.ts
Outdated
| request: Observable<DeepPartial<DashUserSettingsState>>, | ||
| metadata?: grpc.Metadata | ||
| ): Observable<DashUserSettingsState> { | ||
| throw new Error('ts-node does not yet support client streaming!'); |
zakhenry
force-pushed
the
fix/grpc-web-streaming-compilation-failure
branch
from
16ad075
to
9313f19
Compare
…ains 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.
zakhenry
force-pushed
the
fix/grpc-web-streaming-compilation-failure
branch
from
9313f19
to
7e6bf69
Compare
|
Thanks @zakhenry ! |
## [1.110.2](v1.110.1...v1.110.2) (2022-03-27) ### Bug Fixes * **Grpc-Web:** Fix compilation failure when a service definition contains a client streaming call. ([#535](#535)) ([0c83892](0c83892))
|
🎉 This PR is included in version 1.110.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
This PR relates to #329 (the original issue), and partially overlaps with #335 (a WIP PR that actually implements client streaming, but has stalled)