Refactor app updater a bit

- fixes things discovered in manual testing
- avoid $effect loops

apps/desktop/src/lib/backend/updater.ts

@@ -14,13 +14,21 @@ import posthog from 'posthog-js';
 import { derived, writable, type Readable } from 'svelte/store';
 
 // TODO: Investigate why 'DOWNLOADED' is not in the type provided by Tauri.
-export type Update =
+export type Update = {
-	| { version?: string; status?: UpdateStatus | 'DOWNLOADED'; body?: string }
+	version?: string;
-	| undefined;
+	status?: UpdateStatus | 'DOWNLOADED';
+	body?: string;
+	manual: boolean;
+};
 
 export class UpdaterService {
-	private status = writable<UpdateStatus>(undefined);
+	// True if manually initiated check.
-	private result = writable<UpdateManifest | undefined>(undefined, () => {
+	private manual = writable(false);
+
+	// An object rather than string to prevent unique deduplication.
+	private status = writable<{ status: UpdateStatus } | undefined>(undefined);
+
+	private manifest = writable<UpdateManifest | undefined>(undefined, () => {
 		this.start();
 		return () => {
 			this.stop();
@@ -28,41 +36,42 @@ export class UpdaterService {
 	});
 
 	update: Readable<Update | undefined> = derived(
-		[this.status, this.result],
+		[this.status, this.manifest, this.manual],
-		([status, update]) => {
+		([status, result, manual]) => {
-			return { ...update, status };
+			// Do not emit when up-to-date unless manually initiated.
+			if (status?.status === 'UPTODATE' && result && !manual) {
+				return;
+			}
+			return { ...result, ...status, manual };
 		},
 		undefined
 	);
 
+	// Needed to reset dismissed modal when version changes.
 	currentVersion = writable<string | undefined>(undefined);
 	readonly version = derived(this.update, (update) => update?.version);
 
-	prev: Update | undefined;
+	intervalId: any;
 	unlistenStatusFn: any;
 	unlistenManualCheckFn: any;
-	intervalId: any;
 
 	constructor() {}
 
 	private async start() {
-		const currentVersion = await getVersion();
+		this.currentVersion.set(await getVersion());
-		this.currentVersion.set(currentVersion);
 		this.unlistenManualCheckFn = listen<string>('menu://global/update/clicked', () => {
 			this.checkForUpdate(true);
 		});
-
+		this.unlistenStatusFn = await onUpdaterEvent((event) => {
-		this.unlistenStatusFn = await onUpdaterEvent((status) => {
+			const { error, status } = event;
-			const err = status.error;
+			if (error) {
-			if (err) {
+				showErrorToast(error);
-				showErrorToast(err);
+				posthog.capture('App Update Status Error', { error });
-				posthog.capture('App Update Status Error', { error: err });
 			}
-			this.status.set(status.status);
+			this.status.set({ status });
 		});
-
-		await this.checkForUpdate();
 		setInterval(async () => await this.checkForUpdate(), 3600000); // hourly
+		this.checkForUpdate();
 	}
 
 	private async stop() {
@@ -74,17 +83,18 @@ export class UpdaterService {
 		}
 	}
 
-	async checkForUpdate(isManual = false) {
+	async checkForUpdate(manual = false) {
 		const update = await Promise.race([
 			checkUpdate(), // In DEV mode this never returns.
 			new Promise<UpdateResult>((resolve) =>
 				setTimeout(() => resolve({ shouldUpdate: false }), 30000)
 			)
 		]);
-		if (!update.shouldUpdate && isManual) {
+		this.manual.set(manual);
-			this.status.set('UPTODATE');
+		if (!update.shouldUpdate && manual) {
+			this.status.set({ status: 'UPTODATE' });
 		} else if (update.manifest) {
-			this.result.set(update.manifest);
+			this.manifest.set(update.manifest);
 		}
 	}
 

apps/desktop/src/lib/components/AppUpdater.svelte

@@ -2,50 +2,58 @@
 	import Link from '../shared/Link.svelte';
 	import newVersionSvg from '$lib/assets/empty-state/app-new-version.svg?raw';
 	import upToDateSvg from '$lib/assets/empty-state/app-up-to-date.svg?raw';
-	import { UpdaterService } from '$lib/backend/updater';
+	import { UpdaterService, type Update } from '$lib/backend/updater';
 	import { getContext } from '$lib/utils/context';
 	import Button from '@gitbutler/ui/inputs/Button.svelte';
 	import Modal from '@gitbutler/ui/modal/Modal.svelte';
 
 	const updaterService = getContext(UpdaterService);
-	const update = updaterService.update;
 	const currentVersion = updaterService.currentVersion;
 
-	const status = $derived($update?.status);
+	const update = updaterService.update;
-	const version = $derived($update?.version);
 
-	let dismissed = $state(false);
+	let status = $state<Update['status']>();
-	let open = $state(false);
+	let version = $state<Update['version']>();
-
+	let lastVersion: string | undefined;
-	let modalRef: Modal;
+	let dismissed = false;
+	let modal: Modal;
 
 	$effect(() => {
-		if (version || (status && status !== 'ERROR' && !dismissed && !open)) {
+		if ($update) {
-			open = true;
+			console.log($update);
+			handleUpdate($update);
 		}
 	});
 
-	$effect(() => {
+	function handleUpdate(update: Update) {
-		if (status === 'ERROR') {
+		version = update?.version;
-			open = false;
-		}
-	});
 
-	$effect(() => {
+		if (version !== lastVersion) {
-		if (open) {
+			dismissed = false;
-			modalRef.show();
-		} else {
-			modalRef.close();
 		}
-	});
+
+		status = update?.status;
+		const manual = update?.manual;
+
+		if (manual) {
+			modal.show();
+		} else if (status === 'ERROR') {
+			modal.close();
+		} else if (status && status !== 'UPTODATE' && !dismissed) {
+			modal.show();
+		} else if (version && !dismissed) {
+			modal.show();
+		}
+		lastVersion = version;
+	}
 
 	function handleDismiss() {
 		dismissed = true;
-		open = false;
+		modal.close();
 	}
 </script>
 
-<Modal width="xsmall" bind:this={modalRef}>
+<Modal width="xsmall" bind:this={modal}>
 	<div class="modal-illustration">
 		{#if status === 'UPTODATE' || status === 'DONE'}
 			{@html upToDateSvg}
@@ -88,22 +96,32 @@
 
 		{#if status === 'UPTODATE'}
 			<Button style="pop" kind="solid" wide outline onclick={handleDismiss}>Got it!</Button>
+		{:else if status === 'DONE'}
+			<Button
+				style="pop"
+				kind="solid"
+				wide
+				outline
+				onclick={() => {
+					updaterService.relaunchApp();
+				}}
+			>
+				Restart
+			</Button>
 		{:else}
 			<Button
 				style="pop"
 				kind="solid"
 				wide
 				loading={status === 'PENDING' || status === 'DOWNLOADED'}
-				onclick={async () => {
+				onclick={() => {
-					await updaterService.installUpdate();
+					updaterService.installUpdate();
 				}}
 			>
 				{#if status === 'PENDING'}
 					Downloading update...
 				{:else if status === 'DOWNLOADED'}
 					Installing update...
-				{:else if status === 'DONE'}
-					Restart
 				{:else}
 					Download {version}
 				{/if}