-
Notifications
You must be signed in to change notification settings - Fork 172
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
Return promises from service client methods #59
Comments
I'm on the fence. Supporting this change is the evidence that Promises replacing callbacks appears to be the future, see this relevant discussion on StackOverflow: https://stackoverflow.com/questions/30299613/why-does-nodejs-not-use-promise-for-the-readfile-api My concern with this approach is that we make a breaking change from the official nodejs gRPC API which returns a If we return a Promise from unary invocations then we will also need to provide an API for these use cases. We will also need to be comfortable with the fact that users who adopt grpc-web as their transport layer may face a difficult migration to the official nodejs gRPC API when it also supports Browser based environments (see grpc/grpc-web#91) and/if the grpc-web project is deprecated. Thoughts and suggestions welcome 😀 |
@bhollis How would you handle the case where a request returns a stream of messages but you want to react to them immediately when they arrive rather than when the whole stream finishes? This is one of the reasons promises are not particularly suitable for this job as we need to be able to return a value multiple times which Promises do not allow. |
That's fair - Observables would be better, but they're only a proposal right now. I was mostly thinking it'd be nice for unary calls, but I can always build my own Promise/Observable wrappers around the callback-driven code. |
Just bumping this as there has been no further response from the community. My current thinking is that we should mirror the node-grpc API for now to make migration pain free. If you would like to see a Promise/Observable API then I would suggest we either:
Let me know your thoughts |
We have been using https://github.com/bojand/grpc-caller to promisify the GRPC calls. Having ts-protoc-gen generates a typescript definition with Promises would help. At the moment, we have to hardcode it manually which requires more updates than we want to when modifying the proto files. |
@manu-st I'd be willing to consider a patch which generates promises, however it would need to be behind a protoc-gen feature flag |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Bump? |
Thanks for bumping this issue @clanstyles, let me try and bring some closure. As discussed in a handful of other issues, it's my current belief that ts-protoc-gen should be built around callbacks and event handlers. Other abstractions on top of these concepts (or: Promises and Observables) can be created by the community, presumably as a wrapper, ie:
My rationale is that I would like to keep ts-protoc-gen's API surface area as small as possible to make it easier to maintain. |
can this be reopend? |
Maybe we could re-open this discussion now that the official client supports promise clients? Obviously feature parity isn't a req but would be a shame to see a departure of new users over lower level usability stuff like this. |
Agree |
Big agree. Since the official client supports promises we should generate the types. |
2023, still no promise client :D |
How would folks feel about making the callback parameter on the generated service clients optional, and returning a native
Promise
when it's not provided? I'm happy to send a PR if there's interest.The text was updated successfully, but these errors were encountered: