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

Add missing types definitions #886

Merged
merged 6 commits into from
Jul 13, 2020

Conversation

stanley-cheung
Copy link
Collaborator

@stanley-cheung stanley-cheung commented Jul 8, 2020

Fixes #848: Fixed a bug where we can't pass the interceptors into the client constructor because of a type mismatch.

Fixes #868: Added MethodDescriptor to the exported types. Replace the deprecated MethodInfo with MethodDescriptor.

Fixes #877: Fixed issue where UnaryResponse symbols are being optimized away.

And in general added some missing classes to the exported types as well.

Added tests for both callback-based StreamInterceptor and promise-based UnaryInterceptor. And now execute tsc with --strict.

invoker: (request: Request<Req, Resp>) =>
ClientReadableStream<Resp>): ClientReadableStream<Resp>;
}

Copy link

@willsalz willsalz Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this up! I was wondering if you could also add UnaryInterceptor while you're at it?

From: https://github.com/grpc/grpc-web/blob/master/javascript/net/grpc/web/interceptor.js#L47

Suggested change
export interface UnaryInterceptor<Req, Resp> {
intercept(request: Request<Req, Resp>,
invoker: (request: Request<Req, Resp>) =>
Promise<UnaryResponse<Req, Resp>>): Promise<UnaryResponse<Req, Resp>>;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes literally working on that part as we speak :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willsalz Added the UnaryInterceptor interface typings to the PR. Still WIP. Hopefully should land soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants