feat: delete updater pending when is not a available update #8875
Conversation
🦋 Changeset detectedLatest commit: 7eec654 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -494,6 +510,7 @@ export abstract class AppUpdater extends (EventEmitter as new () => TypedEmitter | |||
| }).` | |||
| ) | |||
| this.emit("update-not-available", updateInfo) | |||
| await this.deletePendingUpdate() | |||
There was a problem hiding this comment.
Based on #8730, I think the request also is also to clean the pending dir on successful installation of an update as well.
That being said, I'm not sure if we want to do that in our doInstall
What about clearing the pending dir before the first emit? Not sure if that'll break any previous logic on a cached pending update though...might be worth double checking that.
this.emit("cleaning-pending-update-dir") // optional, may not be necessary?
await this.deletePendingUpdate()
this.emit("checking-for-update")
Also, might be worth renaming the function deletePendingUpdate to removePendingUpdateDirectory to be more explicit on purpose
There was a problem hiding this comment.
Based on #8730, I think the request also is also to clean the pending dir on successful installation of an update as well.
Usually, after users complete an upgrade, they need to restart the app. If we delete files at the moment the user starts the app, the timing is not ideal, as it could slow down the app's startup speed.
What about clearing the pending dir before the first emit? Not sure if that'll break any previous logic on a cached pending update though...might be worth double checking that.
Alternatively, we could expose the delete interface, allowing users to decide when to delete the files themselves?
There was a problem hiding this comment.
Hmmm very valid point on that, we can't impact start-up performance.
I'm adverse to that approach, as electron-updater is meant to be a drop-in replacement for electron's internal version. The goal of the package is to limit configuration/setup so that it "just works"
There was a problem hiding this comment.
What about clearing the pending dir before the first emit? Not sure if that'll break any previous logic on a cached pending update though...might be worth double checking that.
If the download has already been completed but the update has not been applied for a while, and then another checkForUpdate is triggered, the downloaded files might be deleted. It seems that simply deleting them is not a good solution.
There was a problem hiding this comment.
Per say you dont need to wait for an update check to do the cleaning, you have the update version on disk and you have the app version.
This could all be done async on start, it should not impact performances.
Also I agree that it should be done on restart just to be safe.
|
Revisiting this. Maybe we need to investigate having updates installed on app launch instead of app-quit? This would need to be an optionally enabled feature (similar to InstallOnAppQuit) to retain backward functionality. Eventually we could migrate it to be the default approach in the next major semver release for instance |
refers to #8730