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: add TSend generic to Channel #10139

Merged
merged 5 commits into from
Jul 10, 2024

Conversation

Brendonovich
Copy link
Member

Adds a TSend generic to ipc::Channel. This allows for type-safe calls to send, and makes it possible for tauri-specta to extract the generic type of channels.
The generic isn't used in the on_message function as that introduces quite a bit more complexity to do so, especially when supporting mobile. This can be implemented later if needed.

@Brendonovich Brendonovich requested a review from a team as a code owner June 27, 2024 07:08
@Brendonovich Brendonovich changed the title add TSend to Channel feat: add TSend generic to Channel Jun 27, 2024
@@ -877,7 +877,7 @@ pub(crate) fn init<R: Runtime>() -> TauriPlugin<R> {
.on_event(|app, e| {
if let RunEvent::MenuEvent(e) = e {
if let Some(channel) = app.state::<MenuChannels>().0.lock().unwrap().get(&e.id) {
let _ = channel.send(&e.id);
let _ = channel.send(e.id.clone());
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we allow sending a reference?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did consider that but send immediately serializes the value, so the string would have been cloned anyway. At least this way you can serialize owned values unlike if only references were supported, but maybe there's a way to support both i haven't considered

@lucasfernog lucasfernog merged commit 57612ab into tauri-apps:dev Jul 10, 2024
20 checks passed
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