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

Auto-reload on app updates #3693

Merged
merged 18 commits into from
Nov 3, 2024
23 changes: 22 additions & 1 deletion packages/desktop-client/src/browser-preload.browser.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { initBackend as initSQLBackend } from 'absurd-sql/dist/indexeddb-main-thread';
import { registerSW } from 'virtual:pwa-register';

import * as Platform from 'loot-core/src/client/platform';

Expand Down Expand Up @@ -39,6 +40,19 @@ function createBackendWorker() {

createBackendWorker();

let isUpdateReadyForDownload = false;
let markUpdateReadyForDownload;
const isUpdateReadyForDownloadPromise = new Promise(resolve => {
markUpdateReadyForDownload = () => {
isUpdateReadyForDownload = true;
resolve(true);
};
});
const updateSW = registerSW({
immediate: true,
onNeedRefresh: markUpdateReadyForDownload,
});

global.Actual = {
IS_DEV,
ACTUAL_VERSION,
Expand Down Expand Up @@ -140,7 +154,14 @@ global.Actual = {
window.open(url, '_blank');
},
onEventFromMain: () => {},
applyAppUpdate: () => {},
isUpdateReadyForDownload: () => isUpdateReadyForDownload,
waitForUpdateReadyForDownload: () => isUpdateReadyForDownloadPromise,
applyAppUpdate: async () => {
updateSW();

// Wait for the app to reload
await new Promise(() => {});
},
jfdoming marked this conversation as resolved.
Show resolved Hide resolved
updateAppMenu: () => {},

ipcConnect: () => {},
Expand Down
30 changes: 22 additions & 8 deletions packages/desktop-client/src/components/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,20 @@ function AppInner() {
const { showBoundary: showErrorBoundary } = useErrorBoundary();
const dispatch = useDispatch();

const maybeUpdate = async <T,>(cb?: () => T): Promise<T> => {
if (global.Actual.isUpdateReadyForDownload()) {
dispatch(
setAppState({
loadingText: t('Downloading and applying update...'),
}),
);
await global.Actual.applyAppUpdate();
}
return cb?.();
};

async function init() {
const socketName = await global.Actual.getServerSocket();
const socketName = await maybeUpdate(() => global.Actual.getServerSocket());

dispatch(
setAppState({
Expand Down Expand Up @@ -86,14 +98,16 @@ function AppInner() {
loadingText: t('Retrieving remote files...'),
}),
);
send('get-remote-files').then(files => {
if (files) {
const remoteFile = files.find(f => f.fileId === cloudFileId);
if (remoteFile && remoteFile.deleted) {
dispatch(closeBudget());
}

const files = await send('get-remote-files');
if (files) {
const remoteFile = files.find(f => f.fileId === cloudFileId);
if (remoteFile && remoteFile.deleted) {
dispatch(closeBudget());
}
});
}

await maybeUpdate();
}
}

Expand Down
23 changes: 23 additions & 0 deletions packages/desktop-client/src/components/FinancesApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,29 @@ export function FinancesApp() {
}, 100);
}, []);

useEffect(() => {
async function run() {
await global.Actual.waitForUpdateReadyForDownload();
jfdoming marked this conversation as resolved.
Show resolved Hide resolved
dispatch(
addNotification({
type: 'message',
title: t('A new version of Actual is available!'),
message: t('Click the button below to reload and apply the update.'),
sticky: true,
id: 'update-reload-notification',
button: {
title: t('Update now'),
action: async () => {
await global.Actual.applyAppUpdate();
},
},
}),
);
}

run();
}, []);

useEffect(() => {
async function run() {
const latestVersion = await getLatestVersion();
Expand Down
7 changes: 6 additions & 1 deletion packages/desktop-client/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ function rootReducer(state, action) {
return appReducer(state, action);
}

const store = createStore(rootReducer, undefined, applyMiddleware(thunk));
const compose = window['__REDUX_DEVTOOLS_EXTENSION_COMPOSE__'] || (f => f);
const store = createStore(
rootReducer,
undefined,
compose(applyMiddleware(thunk)),
);
const boundActions = bindActionCreators(
Comment on lines +61 to 67
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling Redux devtools to work. Let me know if I should remove the change, I found it useful when debugging.

actions,
store.dispatch,
Expand Down
2 changes: 1 addition & 1 deletion packages/desktop-client/vite.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export default defineConfig(async ({ mode }) => {
mode === 'desktop'
? undefined
: VitePWA({
registerType: 'autoUpdate',
registerType: 'prompt',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'prompt' is misleading here. It's just needed so that we can hook into the "update ready" event. We only show a prompt if an update is available later

workbox: {
globPatterns: [
'**/*.{js,css,html,txt,wasm,sql,sqlite,ico,png,woff2,webmanifest}',
Expand Down
4 changes: 4 additions & 0 deletions packages/desktop-electron/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ contextBridge.exposeInMainWorld('Actual', {
ipcRenderer.send('update-menu', budgetId);
},

// No auto-updates in the desktop app
isUpdateReadyForDownload: () => false,
waitForUpdateReadyForDownload: () => new Promise<void>(() => {}),
Comment on lines +69 to +71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve the update-related implementations and documentation.

The current implementation has a few areas for improvement:

  1. The never-resolving promise in waitForUpdateReadyForDownload could lead to memory leaks
  2. The comment could be more descriptive about why auto-updates are disabled

Consider applying these changes:

-  // No auto-updates in the desktop app
+  // Auto-updates are handled differently in the desktop app through Electron's built-in
+  // auto-update mechanism rather than through the web app's service worker updates
   isUpdateReadyForDownload: () => false,
-  waitForUpdateReadyForDownload: () => new Promise<void>(() => {}),
+  waitForUpdateReadyForDownload: () => Promise.resolve(),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// No auto-updates in the desktop app
isUpdateReadyForDownload: () => false,
waitForUpdateReadyForDownload: () => new Promise<void>(() => {}),
// Auto-updates are handled differently in the desktop app through Electron's built-in
// auto-update mechanism rather than through the web app's service worker updates
isUpdateReadyForDownload: () => false,
waitForUpdateReadyForDownload: () => Promise.resolve(),


getServerSocket: () => {
return null;
},
Expand Down
2 changes: 1 addition & 1 deletion packages/loot-core/src/client/actions/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function setAppState(state: Partial<AppState>): SetAppStateAction {

export function updateApp() {
return async (dispatch: Dispatch) => {
global.Actual.applyAppUpdate();
await global.Actual.applyAppUpdate();
dispatch(setAppState({ updateInfo: null }));
};
}
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/3693.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Enhancements
authors: [jfdoming]
---

Auto-reload on app updates if possible, and show a notification if not possible