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

Remove SendSync (see Electron docs) #181

Merged
merged 5 commits into from
Feb 29, 2024
Merged

Remove SendSync (see Electron docs) #181

merged 5 commits into from
Feb 29, 2024

Conversation

lnrdnl
Copy link
Contributor

@lnrdnl lnrdnl commented Feb 20, 2024

I removed sendSync. Electron docs says you should not use it. Use invoke instead:

See https://www.electronjs.org/docs/latest/tutorial/ipc#using-ipcrenderersendsync

The structure of this code is very similar to the invoke model, but we recommend avoiding this API for performance reasons. Its synchronous nature means that it'll block the renderer process until a reply is received.

I also implemented an invoke example in which you can submit a message (request).

Also I implemented logging to console which helps debugging the communication.

@jooy2
Copy link
Owner

jooy2 commented Feb 29, 2024

Hello, thank you for using the Vutron template and contributing to the work.

The sendSync in your example isn't causing any serious performance degradation, but I agree with replacing it with invoke as it can be confusing to use in the template, and I think the functionality works well enough to replace the existing code.

There are a few things missing (translation keys), but I'll work on that further, and I'll merge this PR.

Thanks a lot!

@jooy2 jooy2 merged commit e33ecfa into jooy2:master Feb 29, 2024
6 checks passed
@jooy2 jooy2 self-requested a review February 29, 2024 00:08
@jooy2 jooy2 added the enhancement New feature or request label Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants