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

Switch to keep alive interval and add unsaved changes warning #65

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

garzj
Copy link

@garzj garzj commented May 28, 2024

I added these two little features, it felt like this warning is a necessity.
Thank you for this project!

@bassmanitram
Copy link
Owner

Hey there - thanks for this. I'll test the warning, but the terminate functionality is intentional, in response to security concerns, so that stays.

@garzj
Copy link
Author

garzj commented Jun 19, 2024

Oh, I did leave the termination logic, just changed it up a bit. The window unload event may be skipped in certain cases, so the server should now close after a timeout of 10 seconds if the frontend stops sending keep alives. Also, another unload handler may interfere with the added warning if the event order is incorrect, that's why I cared in the first place.

@garzj
Copy link
Author

garzj commented Jun 19, 2024

It may still be better to do it your way to prevent unintentional server shutdowns. If the server keeps running, it shouldn't be that of an issue, since it's just reachable from localhost, not from 0.0.0.0.

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