Update application exit logic #405
Conversation
|
@GregorBiswanger @robertmuehsig Are these build issues expected? |
ElectronNET.Host/api/app.ts
Outdated
There was a problem hiding this comment.
Normally no two exclamation points necessary... reads more complicated... JavaScript already understands what you want :) it only goes through if it has a valid value.. change and test with else if (appWindowAllClosedEventId) {
There was a problem hiding this comment.
I've gotten into the habit of !! simply due to this:
https://brianflove.com/2014-09-02/whats-the-double-exclamation-mark-for-in-javascript/
var foo = '' will cause if (foo) { console.log('foo is true!'); } to output 'foo is true'. In reality, I want to treat an empty string as false. Thus, a !! will convert the var to a bool and then take the inverse. if (!!foo) { console.log('foo is true!'); } will not output a value. This means I don't have to do any extra checking of undefined, null, or empty string.
Let me know if you'd rather me swap it to the single !. Happy to do so.
GregorBiswanger
left a comment
GregorBiswanger
left a comment
There was a problem hiding this comment.
Looks good at first glance
The application was not raising the all windows closed event which didn't allow the .NET application to exit properly when the last window was closed. The recommendation by electronjs is to subscribe to the all windows closed event and exit the app via code on macos. This change allows the event to propagated up to the .NET application. It may be better to move all of the logic surrounding this to the ElectronNET.API project and allow the application developer full control over the behavior. For now, this fixes the immediate bug. ElectronNETGH-346
This change explicitly cleans up the .net core http process when the hosting electron application quits. On macos, the child process was sometimes left running depending on how the application was exited. ElectronNETGH-346
2 participants
On macos, applications using ElectronNET.API were not exiting properly. Specifically, there are two issues contributing to this, both of which are discussed in #346.
The all windows closed event was not firing for macos. This meant that the application developer was not able to automatically close the application when all windows were closed and running on macos.
Depending on how the application was triggered to close, the .NET Core kestrel process was not automatically exiting.
This PR fixes both of these issues. A larger issue, though, is that application behavior for macos is being fixed in the electron main process. It may make sense to make a larger change in which all of the behavior is handled in the Electron.API and consuming application, thus giving the end user complete control over the behavior without introducing inconsistent behavior between platforms. For example, rather than then current logic checking the platform in the electron main process, the ElectronNET.API could check and automatically set the default of
Electron.WindowManager.IsQuitOnWindowAllClosedon macos to false. That would eliminate custom logic in the electron main process and keeps behavior consistent.Since this issue is impacting myself and others, I propose to merge this current change and make the additional change at a later time if deemed appropriate.
Fixes #346