feat(#2287): Add ExitRequested event to let users prevent app from exiting

[woubuc]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Docs
• New Binding Issue #___
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes. Issue [Feature request] Keep app running after all windows are closed #2287
• No

The PR fulfills these requirements:

• When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
• A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
See issue #2287

[amrbashir]

LGTM, @lucasfernog @chippers would it be better to combine CloseRequestApi and ExitRequestApi into one type with prevent_default() method that can be used with all events.
With the current implementation, we will have to keep adding new types for every event that user want to take control of.

[woubuc]

combine CloseRequestApi and ExitRequestApi into one type with prevent_default() method that can be used with all events

My 2c: I've personally never been a big fan of Javascript's generic-sounding preventDefault. I think the API is clearer if it explicitly says prevent_close() and prevent_exit() so you know what specific behaviour you're preventing.

[amrbashir]

combine CloseRequestApi and ExitRequestApi into one type with prevent_default() method that can be used with all events

My 2c: I've personally never been a big fan of Javascript's generic-sounding preventDefault. I think the API is clearer if it explicitly says prevent_close() and prevent_exit() so you know what specific behaviour you're preventing.

Yeah that's a good point, I just don't want to have a lot of types that will be harder to maintain as we add similar types for more events. I want to somehow scope this functionality into one type or an enum of some sort and if we can't then your PR is good to go.

[woubuc]

I just don't want to have a lot of types that will be harder to maintain as we add similar types for more events. I want to somehow scope this functionality into one type or an enum of some sort and if we can't then your PR is good to go.

We could probably achieve both goals, at least somewhat. What if we implement the logic for sending and checking messages in one internal struct that contains all the needed functionality, but use differently typed wrappers for the public API?

Something like:

// Shared internal struct, events just use this if they want to get a response
struct OneTimeResponse<T>(Sender<T>);

impl<T> OneTimeResponse<T> {
    fn new() {
        // Shared channel creation logic (no longer in each individual event)
    }
    fn send(self, response: T) {
        // One-time response send logic
        // Each API wrapper just calls this without needing to know internal implementation
    }
}

// The public API wrappers only contain calls to the shared struct, no own logic
struct ExitRequestedApi(OneTimeResponse<ExitRequestedAction>);

impl ExitRequestedApi {
    fn prevent_exit(self) {
        self.0.send(ExitRequestedAction::Prevent);
    }
}

This way, all the actual logic that we need to maintain for sending & receiving these one-time responses is located in 1 struct. The internal events and APIs all use this main struct, but the public API types remain user-friendly, specific and flexible.

[amrbashir]

We could probably achieve both goals, at least somewhat. What if we implement the logic for sending and checking messages in one internal struct that contains all the needed functionality, but use differently typed wrappers for the public API?

Something like:

// Shared internal struct, events just use this if they want to get a response
struct OneTimeResponse<T>(Sender<T>);

impl<T> OneTimeResponse<T> {
    fn new() {
        // Shared channel creation logic (no longer in each individual event)
    }
    fn send(self, response: T) {
        // One-time response send logic
        // Each API wrapper just calls this without needing to know internal implementation
    }
}

// The public API wrappers only contain calls to the shared struct, no own logic
struct ExitRequestedApi(OneTimeResponse<ExitRequestedAction>);

impl ExitRequestedApi {
    fn prevent_exit(self) {
        self.0.send(ExitRequestedAction::Prevent);
    }
}

This way, all the actual logic that we need to maintain for sending & receiving these one-time responses is located in 1 struct. The internal events and APIs all use this main struct, but the public API types remain user-friendly, specific and flexible.

tbh, I don't think this helps a lot, and since I don't have any good ideas about this, I won't hold this PR because of it.

[woubuc]

tbh, I don't think this helps a lot

I think it would help keep maintenance down by having the actual logic implementation in 1 place instead of spread out & duplicated across multiple structs/functions in various crates.

But I agree that it's not a big issue at this point so it can be revisited later if (when) we get more events using the same pattern, if y'all prefer that.

[amrbashir]

combine CloseRequestApi and ExitRequestApi into one type with prevent_default() method that can be used with all events

My 2c: I've personally never been a big fan of Javascript's generic-sounding preventDefault. I think the API is clearer if it explicitly says prevent_close() and prevent_exit() so you know what specific behaviour you're preventing.

I have thought about this again and I think prevent_default() should be fine, users will know exactly what they are preventing because it will be passed through the Event itself.

match event {
    Event::WindowCloseRequested { api, .. } => {
        api.prevent_default(); // here it is obvious we are preventing Window closing
    }
    Event::ExitRequested { api, .. } => {
       api.prevent_default(); // and here prevent app exit
    }
}

[woubuc]

I have thought about this again and I think prevent_default() should be fine

For the 2 events that use it at this point, I agree. But I'm looking to the future:

• What if an event has more options than just "prevent"?
• What if we add events where a non-specific "prevent" isn't entirely clear?

Since we're establishing a pattern here for how to handle this type of event (like you said in #2287, we should strive for consistency in this data flow), I think it's worth taking a moment to figure out how to handle this pattern properly in a more general sense.

I also follow your earlier suggestion regarding maintainability:

I want to somehow scope this functionality into one type or an enum of some sort

With the only difference being that I think it would make more sense to abstract the actual functionality into a generic implementation, rather than reducing the public API for it down to a single rigid type. The end result in both cases is that each individual event doesn't add to the maintenance load since it reuses the same implementation that's already there.

Perhaps it might be useful to get some more feedback from a code architecture standpoint on this issue, and determine whether we want to establish patterns for things like this in the codebase, or we just look at every piece of the puzzle separately - personally I tend to look at the bigger picture, especially for larger projects like Tauri.

Of course, I don't want this discussion to block the current PR, if you'd prefer to just merge this in and figure out the bigger picture stuff later then go ahead. Although in my experience "later" can often turn into "never" because, hey, it works doesn't it 😄

[amrbashir]

I have thought about this again and I think prevent_default() should be fine

For the 2 events that use it at this point, I agree. But I'm looking to the future:

• What if an event has more options than just "prevent"?
• What if we add events where a non-specific "prevent" isn't entirely clear?

I hardly think that would ever be the case. we just want users to have the ability to prevent our internal handling of the event, nothing more.
gtk has Inhibit(true or false) and javascript has preventDefault() for all kinds of events so I don't think it is bad idea in general

Perhaps it might be useful to get some more feedback from a code architecture standpoint on this issue, and determine whether we want to establish patterns for things like this in the codebase, or we just look at every piece of the puzzle separately - personally I tend to look at the bigger picture, especially for larger projects like Tauri.

yeah this needs to be discussed further, probably in discord. and you as a tauri user/contributor your feedback is extremely important.

Of course, I don't want this discussion to block the current PR, if you'd prefer to just merge this in and figure out the bigger picture stuff later then go ahead. Although in my experience "later" can often turn into "never" because, hey, it works doesn't it 😄

yeah the PR is fine to me but I can't merge it. @lucasfernog @chippers are the main maintainers.

[lucasfernog]

Nice work @woubuc