search

grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.46k stars 762 forks source link

Fix TS callback error and response to be nullable #1310

Closed sampajano closed 1 year ago

sampajano commented 1 year ago

IMPORTANT: This will be a BREAKING change for some existing users, but hopefully in a good way because now the return type is more accurately reflecting the runtime behavior.

In reality, both error and response can be null.

JS Defintion: https://github.com/grpc/grpc-web/blob/e49389873d887d15ab2870288f620aa2f15b3b85/javascript/net/grpc/web/abstractclientbase.js#L50-L51

Callsites: https://github.com/grpc/grpc-web/blob/e49389873d887d15ab2870288f620aa2f15b3b85/javascript/net/grpc/web/grpcwebclientbase.js#L249

https://github.com/grpc/grpc-web/blob/e49389873d887d15ab2870288f620aa2f15b3b85/javascript/net/grpc/web/grpcwebclientbase.js#L284

Fixes #1309

sampajano commented 1 year ago

@stanley-cheung Hey Stanley, i think maybe this is a TS bug from day 1. I wonder if there's any concern if we make this fix (probably be breaking for some existing clients but should be easy to fix) and just bump the "minor" version (to 1.5)? Thanks!

sampajano commented 1 year ago

Making this a draft for now.. See discussion here: https://github.com/grpc/grpc-web/issues/1309#issuecomment-1373045693

sampajano commented 1 year ago

Please see https://github.com/grpc/grpc-web/issues/1309#issuecomment-1373045693 — I realized that this fix will create additional unnecessary troubles for callers so it's not idea. Dropping it for now.

Better ideas that does not involve a big breaking change would be welcome!