mirror of
https://github.com/actualbudget/actual.git
synced 2026-04-30 18:00:06 -05:00
[AI] Implement sync recovery (#7111)
* [AI] Fix iOS/Safari sync recovery (fixes #7026): useOnVisible hook, re-fetch server version on visible, improved network-failure message Made-with: Cursor * Feedback: coderabbitai * Refactor useOnVisible test: remove unnecessary resolve check and simplify callback definition
This commit is contained in:
committed by
GitHub
parent
b3a86b5392
commit
078603cadf
@@ -314,6 +314,7 @@ Always run `yarn typecheck` before committing.
|
|||||||
|
|
||||||
**React Patterns:**
|
**React Patterns:**
|
||||||
|
|
||||||
|
- The project uses **React Compiler** (`babel-plugin-react-compiler`) in the desktop-client. The compiler auto-memoizes component bodies, so you can omit manual `useCallback`, `useMemo`, and `React.memo` when adding or refactoring code; prefer inline callbacks and values unless a stable identity is required by a non-compiled dependency.
|
||||||
- Don't use `React.FunctionComponent` or `React.FC` - type props directly
|
- Don't use `React.FunctionComponent` or `React.FC` - type props directly
|
||||||
- Don't use `React.*` patterns - use named imports instead
|
- Don't use `React.*` patterns - use named imports instead
|
||||||
- Use `<Link>` instead of `<a>` tags
|
- Use `<Link>` instead of `<a>` tags
|
||||||
|
|||||||
@@ -34,6 +34,7 @@ import {
|
|||||||
import { handleGlobalEvents } from '@desktop-client/global-events';
|
import { handleGlobalEvents } from '@desktop-client/global-events';
|
||||||
import { useIsTestEnv } from '@desktop-client/hooks/useIsTestEnv';
|
import { useIsTestEnv } from '@desktop-client/hooks/useIsTestEnv';
|
||||||
import { useMetadataPref } from '@desktop-client/hooks/useMetadataPref';
|
import { useMetadataPref } from '@desktop-client/hooks/useMetadataPref';
|
||||||
|
import { useOnVisible } from '@desktop-client/hooks/useOnVisible';
|
||||||
import { SpreadsheetProvider } from '@desktop-client/hooks/useSpreadsheet';
|
import { SpreadsheetProvider } from '@desktop-client/hooks/useSpreadsheet';
|
||||||
import { setI18NextLanguage } from '@desktop-client/i18n';
|
import { setI18NextLanguage } from '@desktop-client/i18n';
|
||||||
import { addNotification } from '@desktop-client/notifications/notificationsSlice';
|
import { addNotification } from '@desktop-client/notifications/notificationsSlice';
|
||||||
@@ -179,6 +180,11 @@ export function App() {
|
|||||||
);
|
);
|
||||||
const dispatch = useDispatch();
|
const dispatch = useDispatch();
|
||||||
|
|
||||||
|
useOnVisible(async () => {
|
||||||
|
console.debug('triggering sync because of visibility change');
|
||||||
|
await dispatch(sync());
|
||||||
|
});
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
function checkScrollbars() {
|
function checkScrollbars() {
|
||||||
if (hiddenScrollbars !== hasHiddenScrollbars()) {
|
if (hiddenScrollbars !== hasHiddenScrollbars()) {
|
||||||
@@ -186,25 +192,9 @@ export function App() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let isSyncing = false;
|
|
||||||
|
|
||||||
async function onVisibilityChange() {
|
|
||||||
if (!isSyncing) {
|
|
||||||
console.debug('triggering sync because of visibility change');
|
|
||||||
isSyncing = true;
|
|
||||||
await dispatch(sync());
|
|
||||||
isSyncing = false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
window.addEventListener('focus', checkScrollbars);
|
window.addEventListener('focus', checkScrollbars);
|
||||||
window.addEventListener('visibilitychange', onVisibilityChange);
|
return () => window.removeEventListener('focus', checkScrollbars);
|
||||||
|
}, [hiddenScrollbars]);
|
||||||
return () => {
|
|
||||||
window.removeEventListener('focus', checkScrollbars);
|
|
||||||
window.removeEventListener('visibilitychange', onVisibilityChange);
|
|
||||||
};
|
|
||||||
}, [dispatch, hiddenScrollbars]);
|
|
||||||
|
|
||||||
const [theme] = useTheme();
|
const [theme] = useTheme();
|
||||||
|
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ import { t } from 'i18next';
|
|||||||
import { send } from 'loot-core/platform/client/connection';
|
import { send } from 'loot-core/platform/client/connection';
|
||||||
import type { Handlers } from 'loot-core/types/handlers';
|
import type { Handlers } from 'loot-core/types/handlers';
|
||||||
|
|
||||||
|
import { useOnVisible } from '@desktop-client/hooks/useOnVisible';
|
||||||
import { addNotification } from '@desktop-client/notifications/notificationsSlice';
|
import { addNotification } from '@desktop-client/notifications/notificationsSlice';
|
||||||
import { useDispatch } from '@desktop-client/redux';
|
import { useDispatch } from '@desktop-client/redux';
|
||||||
|
|
||||||
@@ -110,6 +111,16 @@ export function ServerProvider({ children }: { children: ReactNode }) {
|
|||||||
void run();
|
void run();
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
useOnVisible(
|
||||||
|
async () => {
|
||||||
|
const version = await getServerVersion();
|
||||||
|
setVersion(version);
|
||||||
|
},
|
||||||
|
{
|
||||||
|
isEnabled: !!serverURL,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
const refreshLoginMethods = useCallback(async () => {
|
const refreshLoginMethods = useCallback(async () => {
|
||||||
if (serverURL) {
|
if (serverURL) {
|
||||||
const data: Awaited<ReturnType<Handlers['subscribe-get-login-methods']>> =
|
const data: Awaited<ReturnType<Handlers['subscribe-get-login-methods']>> =
|
||||||
|
|||||||
@@ -313,7 +313,7 @@ export function ConfigServer() {
|
|||||||
switch (error) {
|
switch (error) {
|
||||||
case 'network-failure':
|
case 'network-failure':
|
||||||
return t(
|
return t(
|
||||||
'Server is not running at this URL. Make sure you have HTTPS set up properly.',
|
'Connection failed. If you use a self-signed certificate or were recently offline, try refreshing the page. Otherwise ensure you have HTTPS set up properly.',
|
||||||
);
|
);
|
||||||
default:
|
default:
|
||||||
return t(
|
return t(
|
||||||
|
|||||||
108
packages/desktop-client/src/hooks/useOnVisible.test.ts
Normal file
108
packages/desktop-client/src/hooks/useOnVisible.test.ts
Normal file
@@ -0,0 +1,108 @@
|
|||||||
|
import { renderHook } from '@testing-library/react';
|
||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
|
||||||
|
import { useOnVisible } from './useOnVisible';
|
||||||
|
|
||||||
|
function setVisibilityState(value: DocumentVisibilityState) {
|
||||||
|
Object.defineProperty(document, 'visibilityState', {
|
||||||
|
value,
|
||||||
|
configurable: true,
|
||||||
|
writable: true,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function dispatchVisibilityChange() {
|
||||||
|
document.dispatchEvent(new Event('visibilitychange'));
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('useOnVisible', () => {
|
||||||
|
const originalVisibilityState = document.visibilityState;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
setVisibilityState('visible');
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
setVisibilityState(originalVisibilityState);
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('invokes callback when document becomes visible', () => {
|
||||||
|
const callback = vi.fn();
|
||||||
|
renderHook(() => useOnVisible(callback));
|
||||||
|
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
|
||||||
|
expect(callback).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not invoke callback when visibilityState is hidden', () => {
|
||||||
|
const callback = vi.fn();
|
||||||
|
renderHook(() => useOnVisible(callback));
|
||||||
|
|
||||||
|
setVisibilityState('hidden');
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
|
||||||
|
expect(callback).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not attach listener when isEnabled is false', () => {
|
||||||
|
const callback = vi.fn();
|
||||||
|
renderHook(() => useOnVisible(callback, { isEnabled: false }));
|
||||||
|
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
|
||||||
|
expect(callback).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('stops invoking callback after unmount', () => {
|
||||||
|
const callback = vi.fn();
|
||||||
|
const { unmount } = renderHook(() => useOnVisible(callback));
|
||||||
|
|
||||||
|
unmount();
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
|
||||||
|
expect(callback).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('invokes callback on every visibilitychange when visibilityState is visible', async () => {
|
||||||
|
const callback = vi.fn();
|
||||||
|
renderHook(() => useOnVisible(callback));
|
||||||
|
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
expect(callback).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
await Promise.resolve();
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
expect(callback).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not invoke callback again until previous async callback completes', async () => {
|
||||||
|
let resolve: () => void;
|
||||||
|
const callback = vi.fn().mockImplementation(
|
||||||
|
() =>
|
||||||
|
new Promise<void>(r => {
|
||||||
|
resolve = r;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
renderHook(() => useOnVisible(callback));
|
||||||
|
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
expect(callback).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
resolve();
|
||||||
|
await Promise.resolve();
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
expect(callback).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('invokes callback when isEnabled is true by default', () => {
|
||||||
|
const callback = vi.fn();
|
||||||
|
renderHook(() => useOnVisible(callback));
|
||||||
|
|
||||||
|
dispatchVisibilityChange();
|
||||||
|
|
||||||
|
expect(callback).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
47
packages/desktop-client/src/hooks/useOnVisible.ts
Normal file
47
packages/desktop-client/src/hooks/useOnVisible.ts
Normal file
@@ -0,0 +1,47 @@
|
|||||||
|
import { useEffect, useEffectEvent, useRef } from 'react';
|
||||||
|
|
||||||
|
type UseOnVisibleOptions = {
|
||||||
|
/** When false, the visibility listener is not attached. Default true. */
|
||||||
|
isEnabled?: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Runs the given callback when the document becomes visible (e.g. user
|
||||||
|
* switches back to the tab). Uses a guard so the callback is not invoked
|
||||||
|
* again until the previous invocation has finished (handles async callbacks).
|
||||||
|
*/
|
||||||
|
export function useOnVisible(
|
||||||
|
callback: () => void | Promise<void>,
|
||||||
|
options: UseOnVisibleOptions = {},
|
||||||
|
) {
|
||||||
|
const { isEnabled = true } = options;
|
||||||
|
const inProgress = useRef(false);
|
||||||
|
|
||||||
|
const runCallback = useEffectEvent(async () => {
|
||||||
|
if (inProgress.current) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
inProgress.current = true;
|
||||||
|
try {
|
||||||
|
await callback();
|
||||||
|
} finally {
|
||||||
|
inProgress.current = false;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
if (!isEnabled) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
function onVisibilityChange() {
|
||||||
|
if (document.visibilityState !== 'visible') {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
void runCallback();
|
||||||
|
}
|
||||||
|
|
||||||
|
document.addEventListener('visibilitychange', onVisibilityChange);
|
||||||
|
return () =>
|
||||||
|
document.removeEventListener('visibilitychange', onVisibilityChange);
|
||||||
|
}, [isEnabled]);
|
||||||
|
}
|
||||||
6
upcoming-release-notes/7111.md
Normal file
6
upcoming-release-notes/7111.md
Normal file
@@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
category: Enhancements
|
||||||
|
authors: [MatissJanis]
|
||||||
|
---
|
||||||
|
|
||||||
|
Reload server version when visibility to the page changes
|
||||||
Reference in New Issue
Block a user