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: introduce App::run_return #12668

Open
wants to merge 24 commits into
base: dev
Choose a base branch
from

Conversation

thomaseizinger
Copy link

The current App::run_iteration function is buggy because it busy-loops. To address the use-case of cleaning up resources in the host program, we introduce App::run_return which builds on top of tao's Eventloop::run_return.

Related: #8631.

@thomaseizinger thomaseizinger requested a review from a team as a code owner February 10, 2025 05:50
Copy link
Contributor

github-actions bot commented Feb 10, 2025

Package Changes Through 71c51ea

There are 8 changes which include tauri-cli with minor, tauri-runtime with minor, tauri-runtime-wry with minor, tauri-utils with minor, tauri with minor, @tauri-apps/api with minor, @tauri-apps/cli with minor, tauri-bundler with patch

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
@tauri-apps/api 2.2.0 2.3.0
tauri-utils 2.1.1 2.2.0
tauri-bundler 2.2.3 2.2.4
tauri-runtime 2.3.0 2.4.0
tauri-runtime-wry 2.3.0 2.4.0
tauri-codegen 2.0.4 2.0.5
tauri-macros 2.0.4 2.0.5
tauri-plugin 2.0.4 2.0.5
tauri-build 2.0.5 2.0.6
tauri 2.2.5 2.3.0
@tauri-apps/cli 2.2.7 2.3.0
tauri-cli 2.2.7 2.3.0

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@thomaseizinger
Copy link
Author

Do we want to deprecate run_iteration as part of this?

@thomaseizinger thomaseizinger force-pushed the feat/introduce-run-return branch from 5b41a90 to e48e8c3 Compare February 11, 2025 07:02
@@ -2840,13 +2843,13 @@ impl<T: UserEvent> Runtime<T> for Wry<T> {
let active_tracing_spans = self.context.main_thread.active_tracing_spans.clone();
let proxy = self.event_loop.create_proxy();

self.event_loop.run(move |event, event_loop, control_flow| {
self.event_loop.run_return(move |e, event_loop, cf| {
Copy link
Author

Choose a reason for hiding this comment

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

I renamed these variables here so that this stays on one line. That way, the git diff shows nicely that run_return is effectively what run used to be.

pub fn run_return<F: FnMut(&AppHandle<R>, RunEvent) + 'static>(
mut self,
mut callback: F,
) -> std::result::Result<i32, Box<dyn std::error::Error>> {
Copy link
Author

Choose a reason for hiding this comment

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

I've just used the same error as from the setup callback. It is missing Send + Sync + 'static bounds unfortunately but that is already the case and can't be changed now I think.

@FabianLars
Copy link
Member

Do we want to deprecate run_iteration as part of this?

imo yes

@FabianLars
Copy link
Member

iirc run_return also works on android (though probably good to test that first), only iOS should be a problem. I'm wondering whether we should really go for the cfg flag you used or just try to do what winit for example does and just document that it doesn't return ever on iOS (using tao's run instead of run_return for iOS internally). didn't look too much into the code yet so idk how feasible that is.

@thomaseizinger
Copy link
Author

iirc run_return also works on android (though probably good to test that first), only iOS should be a problem. I'm wondering whether we should really go for the cfg flag you used or just try to do what winit for example does and just document that it doesn't return ever on iOS (using tao's run instead of run_return for iOS internally). didn't look too much into the code yet so idk how feasible that is.

Removing the cfg is a semver-compatible change so we can always do that later? I'd prefer an incremental approach if possible! :)

run_iteration is only exposed on desktop hence why I copied that. (To me, adding run_return is a bugfix for run_iteration).

Deciding on what the mobile story is here seems like a different problem to me that I'd rather not tackle, also because I don't know the internals of tao and Tauri well enough :)

@thomaseizinger
Copy link
Author

@FabianLars

  • App::run_iteration has been deprecated.
  • I had to re-introduce an actual implementation of Wry::run because run_return in tao is not available on iOS and Android. To deduplicate the code, I extracted a factory fn for the event handler.

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