-
Notifications
You must be signed in to change notification settings - Fork 864
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
fix(windows): autoInstallOnAppQuit #1682
Conversation
autoInstallOnAppQuit works fine on windows, but we had a bunch of code responsible for doing it manually due to macOS not being supported natively by autoInstallOnAppQuit. Due to this fix from #1679 was not applied correctly, and macOS-specific handling broke updates on windows. This change ensures that macOS keeps using the old upgrade paths, but Windows and AppImage leverage upstream code for applying upgrades. License: MIT Signed-off-by: Marcin Rataj <[email protected]>
b90d68b
to
0544aff
Compare
Sadly this last mile is specific to our codebase and won't fix old versions. To properly test this we need to tag v0.13.1 and v0.13.2 and do quick smoke test by installing v0.13.1 and testing if it upgrades fine to v0.13.2.
|
Ok, tested this on Windows with my fork and both per user and per all users upgraded correctly on Windows. |
e.preventDefault() | ||
await stopIpfs() | ||
app.quit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what was the purpose of e.preventDefault()
, but it seemed to interfere with autoInstallOnAppQuit
on Windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
License: MIT Signed-off-by: Marcin Rataj <[email protected]>
lidel/electron-updater-altschmerz#1 demonstrated that
autoInstallOnAppQuit
works fine on Windows, but we had a bunch of code responsible for doing it manually, probably due to macOS not being supported natively byautoInstallOnAppQuit
.Docs for
electron-updater
4.3.5 state:Due to this, the fix from #1679 was not applied correctly: macOS-specific handling executed first and broke updates on Windows.
This PR ensures that macOS keeps using the old upgrade paths, but Windows and AppImage leverage upstream code for applying upgrades.
Closes #1570 and most likely closes #1630 too.