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

[Enhancement] Rewrite grpc-web using TypeScript #405

Closed
pumano opened this issue Dec 5, 2018 · 4 comments
Closed

[Enhancement] Rewrite grpc-web using TypeScript #405

pumano opened this issue Dec 5, 2018 · 4 comments

Comments

@pumano
Copy link
Contributor

pumano commented Dec 5, 2018

I want to contribute to this project for implementing client-side interceptors, but stuck with .js code base from javascript/net/grpc/web.

That code base contains closure imports and some closure JSDoc, some features for implementing for example interceptors require some refactoring. Modern TypeScript provide better object oriented way for creating readable code, also it's possible to have better architecture. For example for implementing interceptors (Chain of Responsibility pattern) we can use better code base with chain of handlers and not to have callback hell.

It would be good to have original code base in TypeScript (where we can get all proper typings) and then translate it to understandable closure compiler code and use bazel too.
I know Google Angular team have tsickle project for translating typescript for using it with closure compiler and bazel.

@stanley-cheung what do you think about it?

Anyway I want to contribute, and can do it with or without typescript, but would be good to have proper typings from out-of-the-box and class based modern code base.

@stanley-cheung
Copy link
Collaborator

@pumano Thanks for raising this issue. We understand and receive a lot of feedback that developers prefer solid TypeScript support. One concern we have, from the project management perspective, is that this gRPC-Web client library is a shared codebase with our internal library. Unfortunately, our internal library will remain Closure-based for the foreseeable future so we have concerns about having a diverging set of source codes if we move completely to TypeScript here. It's already a challenge to try to keep these 2 codebases as in sync as possible.

Another concern I have is that, there is a lot of code we currently rely on from the Closure library, namely the goog.net.streams module, which does a lot of the heavy lifting of parsing the underlying HTTP requests / responses. We will have to re-implement a lot of those logic in TypeScript - unless I misunderstood your intention that there is some way to re-implement this library in TypeScript but still depends on the Closure library? If we have to re-implement these response parsing logic, then it goes back to my first point that we'd like to avoid having to maintain two effectively different language codebases but effectively doing the same thing.

We would love to welcome contributions to this project - we had already been seeing tremendous support from the community on fixing a lot of the bugs on the TypeScript code generation. We just have to be really careful and have a detail plan on how to move this forward without sacrificing code maintainability.

@pumano
Copy link
Contributor Author

pumano commented Dec 7, 2018

@stanley-cheung OK, no problem. I will create another issue about client side interceptors for detailed architecture discussion.

@pumano pumano closed this as completed Dec 7, 2018
@stanley-cheung
Copy link
Collaborator

We can continue the discussion on client-side interceptors on #283.

Prior art: gRPC Node interceptors: grpc/grpc-node#59

@johanbrandhorst
Copy link
Collaborator

@pumano just wanted to add that github.com/improbable-eng/grpc-web is another spec compliant typescript based client that you might want to check out if you're interested in contributing these kind of additions. It will remain as an alternative implementation of the spec for exactly this and other reasons.

loyalpartner pushed a commit to loyalpartner/grpc-web that referenced this issue Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants