search

frankhale / electron-with-express

A simple app that demonstrates spawning an Express app from Electron
MIT License
643 stars 147 forks source link

The server does not close after closing the application (win.close) #15

Closed joelsurgithub closed 5 years ago

joelsurgithub commented 5 years ago

Hi, The server does not close when I exit the app with

const {BrowserWindow} = require('electron').remote

BrowserWindow.getFocusedWindow().close()

But it closes when I leave the application with the cross at the top right... How to solve this problem?

frankhale commented 5 years ago

The thing is, is that the BrowserWindow is a different process than the server so while the window will close it won't automatically close the server. Have a look at this issue: https://github.com/frankhale/electron-with-express/issues/11 as this method was adopted. Before you close the browser window send an IPC message 'stop-server' so that the server will close.

That said, this method is a stop gap measure, we can do better than this and we really should. If you play around with the code and find a better way to do it please send a pull request.

joelsurgithub commented 5 years ago

yes okay but there is already an event 

on("close", ...)

with

mainWindow.webContents.send('stop-server');

in main.js. Then why when I do windoww.close(); does the server not close?

frankhale commented 5 years ago

Then why when I do windoww.close(); does the server not close?

All I can say is that the event may not be firing, or the IPC message isn't working so the server isn't being closed. It used to work. When I get some time I'll take a look.

frankhale commented 5 years ago

What platform are you on? I have been looking at this today and I cannot reproduce the issue on Windows 10.

frankhale commented 5 years ago

I'm running from command line using npm start and then I close the Electron window and the processes are all closing as expected. They are all closing even without the extra bit of code to tell the server to quit. So I think Windows is taking care of it but this may be an issue on Linux or Mac but I'm not sure.

frankhale commented 5 years ago

are you on a platform other than Windows?

joelsurgithub commented 5 years ago

Sorry I don't think to check my emails... Yes, I'm on Linux. But the server does not close only when I close the application with win.close()

frankhale commented 5 years ago

It could be that the Window close event that is standard bypasses the Electron BrowserWindow close event. That's interesting and I'd think it'd be a bug in Electron if that were the case. I'll have to try on a Linux VM to see how it behaves. Give me some time because I just started a new job this morning.

vzqzac commented 5 years ago

Sorry for talking about something not related, but this might be a good time to add an issue template with steps to reproduce and context to approach problems better.

frankhale commented 5 years ago

Totally agree @VzqzAc !!!! If you have some ideas for this template please let me know and I'll gladly incorporate those ideas into it =)

Thank you for the suggestion!!!

frankhale commented 5 years ago

@VzqzAc I don't even know how to create a issue template. Is this something that is added to the code repo and then is used by the issue tracker? Can you show me how to do it or point me to a reference on how to do it?

vzqzac commented 5 years ago

Thanks for your response @frankhale, I just opened https://github.com/frankhale/electron-with-express/pull/16 with what I think is the proper information to add the templates, I'd appreciate your review there and of course any change request is highly appreciated too!

frankhale commented 5 years ago

Yep thank you so much @VzqzAc for the pull request. I've merged it.

frankhale commented 5 years ago

So as I rewrite this I am finding that this is a really big problem. Sometimes the processes will exit and other times they won't. This is super annoying. I wonder if it's any better on Linux or Mac. Windows is a serious pain in the @$$!

frankhale commented 5 years ago

Closing this in favor of tracking this in #21