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

feat(client): add callback/event mechanism between TypeScript and Go #2330

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

jyyi1
Copy link
Contributor

@jyyi1 jyyi1 commented Jan 15, 2025

This PR introduces a callback/event mechanism that allows TypeScript code to subscribe to events triggered in the Go code.

To demonstrate the functionality, this PR implements a VPN connection status change event. The TypeScript code can now subscribe to this event and receive updates on the VPN connection status.

This mechanism can be leveraged for both electron and the Cordova code.

@jyyi1 jyyi1 marked this pull request as ready for review January 16, 2025 03:17
@jyyi1 jyyi1 requested review from a team as code owners January 16, 2025 03:17
@github-actions github-actions bot added size/XL and removed size/L labels Jan 16, 2025
@jyyi1 jyyi1 requested review from fortuna and sbruens January 16, 2025 03:18
client/electron/go_plugin.ts Outdated Show resolved Hide resolved
* @param name The name of the event to unsubscribe from.
* @returns A Promise that resolves when the unsubscription is successful.
*/
export async function unsubscribeEvent(name: string): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you link the unsubscribe action to the actual subscription instead of the event name? It's a cleaner way to manage subscriptions and allows the TypeScript code to subscribe to the same event twice if needed.

Looks like Koffi already gives you a unique callback object, so you could wrap that in some dedicated Subscription type with an unsubscribe() method. Or assign it a unique ID and pass that around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I also decided to differentiate a Callback and an Event handler. A Callback can be used as an Event handler, it can also be used in other MethodChannel functions as well (for example, calling TestConnectivity and have an ResultCallback to receive the result). Then we will have a general way to call TypeScript functions from Go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored, now it accepts multiple callbacks.

Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

It seems you still need to update the PR to decouple the callback from the event handling. Let me know when I should take a look

* @param data The event data string passed from the source.
* @param param The param string provided during registration.
*/
export type CallbackFunction = (data: string, param: string) => void;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I understand you are decoupling callbacks from events like we discussed. So this would have a single data parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

*
* @remarks Subscribing to an event will replace any previously subscribed callback for that event.
*/
export async function subscribeEvent(
Copy link
Collaborator

Choose a reason for hiding this comment

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

My understanding is that this will become a regular MethodChannel call that takes a callback id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, done.

@jyyi1 jyyi1 force-pushed the junyi/vpn-callback-linux branch from f617f0b to 3ced93e Compare January 18, 2025 01:58
@jyyi1 jyyi1 requested review from fortuna and sbruens January 22, 2025 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants