Skip to content

Conversation

@jamaljsr
Copy link
Contributor

The grpc npm package is currently receiving maintenance updates only and plans to be deprecated in April. This PR updates lnrpc to use the recommended grpc-js package instead.

I ran into issues getting the latest version of Electron to function properly using the native grpc package, so I decided to give it's successor grpc-js a try. I've tested with with both unary and streaming RPCs with no issues. The diff is pretty small since the API is almost identical.

There was only one issue with this migration which was caused by the Proxy on the grpc.Client objects. The getter would be called, and therefore promisify, internal method invocations such as client. checkOptionalUnaryResponseArguments. This function was returning a Promise instead of an object, causing a bunch of errors. To resolve this, I updated the Proxy not to promisify calls to base class functions.

I am opening this PR to save you all the effort that I spent today resolving my own grpc issues. If you prefer not to use the approach I took, feel free to close this PR. No hard feelings from my end 😁

@cavanmflynn
Copy link
Contributor

Thanks so much @jamaljsr! I tried to swap grpc for @grpc/grpc-js back in 2019, but ran into a number of issues. Looks like it has progressed a lot since then. I'll have a deeper look at this shortly!

Copy link
Contributor

@cavanmflynn cavanmflynn left a comment

Choose a reason for hiding this comment

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

🎉

@cavanmflynn cavanmflynn merged commit 4e72ad5 into RadarTech:master Jan 28, 2021
@jamaljsr
Copy link
Contributor Author

Awesome! thank you

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

Successfully merging this pull request may close these issues.

2 participants