Skip to content

Commit

Permalink
Removed memory leaks in MonkAppStateProvider and LiveConfigAppProvide…
Browse files Browse the repository at this point in the history
…r components (#882)

* Added useIsMounted hook

* Removed memory leaks in MonkAppStateProvider and LiveConfigAppProvider components
  • Loading branch information
souyahia-monk authored Nov 19, 2024
1 parent 1ce5f6e commit c53e15d
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 23 deletions.
1 change: 1 addition & 0 deletions configs/test-utils/src/__mocks__/@monkvision/common.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,5 @@ export = {
isInputErrorDisplayed: jest.fn(() => false),
isInputTouchedOrDirty: jest.fn(() => false),
})),
useIsMounted: jest.fn(() => jest.fn(() => true)),
};
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import {
MonkAppStateProvider,
MonkAppStateProviderProps,
useAsyncEffect,
useI18nSync,
useLoadingState,
} from '@monkvision/common';
import { PropsWithChildren, useCallback, useEffect, useState } from 'react';
import { PropsWithChildren, useState } from 'react';
import { CaptureAppConfig } from '@monkvision/types';
import { MonkApi } from '@monkvision/network';
import { useMonitoring } from '@monkvision/monitoring';
Expand Down Expand Up @@ -50,29 +51,29 @@ export function LiveConfigAppProvider({
const loading = useLoadingState(true);
const [config, setConfig] = useState<CaptureAppConfig | null>(null);
const { handleError } = useMonitoring();
const [retry, setRetry] = useState(0);

const fetchLiveConfig = useCallback(() => {
if (localConfig) {
loading.onSuccess();
setConfig(localConfig);
return;
}
loading.start();
setConfig(null);
MonkApi.getLiveConfig(id)
.then((result) => {
useAsyncEffect(
() => {
if (localConfig) {
return Promise.resolve(localConfig);
}
loading.start();
setConfig(null);
return MonkApi.getLiveConfig(id);
},
[id, localConfig, retry],
{
onResolve: (result) => {
loading.onSuccess();
setConfig(result);
})
.catch((err) => {
},
onReject: (err) => {
handleError(err);
loading.onError();
});
}, [id, localConfig]);

useEffect(() => {
fetchLiveConfig();
}, [fetchLiveConfig]);
},
},
);

if (loading.isLoading || loading.error || !config) {
return (
Expand All @@ -83,7 +84,7 @@ export function LiveConfigAppProvider({
<div style={styles['errorMessage']} data-testid='error-msg'>
Unable to fetch application configuration. Please try again in a few minutes.
</div>
<Button variant='outline' icon='refresh' onClick={fetchLiveConfig}>
<Button variant='outline' icon='refresh' onClick={() => setRetry((value) => value + 1)}>
Retry
</Button>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@ jest.mock('../../src/components/Button', () => ({
jest.mock('../../src/components/Spinner', () => ({
Spinner: jest.fn(() => <></>),
}));
const { useAsyncEffect: useAsyncEffectActual } = jest.requireActual('@monkvision/common');

import { act, render, screen, waitFor } from '@testing-library/react';
import { createFakePromise, expectPropsOnChildMock } from '@monkvision/test-utils';
import { MonkAppStateProvider, useLoadingState } from '@monkvision/common';
import { MonkAppStateProvider, useAsyncEffect, useLoadingState } from '@monkvision/common';
import { MonkApi } from '@monkvision/network';
import { Button, LiveConfigAppProvider, Spinner } from '../../src';

describe('LiveConfigAppProvider component', () => {
beforeEach(() => {
(useAsyncEffect as jest.Mock).mockImplementation(useAsyncEffectActual);
});

afterEach(() => {
jest.clearAllMocks();
});
Expand Down
20 changes: 20 additions & 0 deletions packages/common/README/HOOKS.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,26 @@ function TestComponent() {
This custom hook creates an interval that calls the provided callback every `delay` milliseconds. If `delay` is `null`
or less than 0, the callback will not be called.

### useIsMounted
```tsx
import { useIsMounted } from '@monkvision/common';

function TestComponent() {
const [example, setExample] = useState(0);
const isMounted = useIsMounted();

useEffect(() => {
myAsyncFunc().then((value) => {
if (isMounted()) {
setExample(value);
}
}).catch(console.error);
}, [isMounted]);
}
```
Custom hook returning a ref to a util function returning `true` if the component using the hook is mounted, and false
otherwise. Can be used to cancel asynchronous calls on component unmounts.

### useForm

```tsx
Expand Down
6 changes: 4 additions & 2 deletions packages/common/src/apps/appStateProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { MonkAppState, MonkAppStateContext } from './appState';
import { useAppStateMonitoring } from './monitoring';
import { useAppStateAnalytics } from './analytics';
import { getAvailableVehicleTypes } from '../utils';
import { useIsMounted } from '../hooks/useIsMounted';

/**
* Local storage key used within Monk web applications to store the authentication token.
Expand Down Expand Up @@ -87,6 +88,7 @@ export function MonkAppStateProvider({
const [steeringWheel, setSteeringWheel] = useState<SteeringWheelPosition | null>(null);
const availableVehicleTypes = useMemo(() => getAvailableVehicleTypes(config), [config]);
const monkSearchParams = useMonkSearchParams({ availableVehicleTypes });
const isMounted = useIsMounted();
useAppStateMonitoring({ authToken, inspectionId, vehicleType, steeringWheel });
useAppStateAnalytics({ inspectionId });

Expand All @@ -100,12 +102,12 @@ export function MonkAppStateProvider({
setVehicleType((param) => monkSearchParams.get(MonkSearchParam.VEHICLE_TYPE) ?? param);
setSteeringWheel((param) => monkSearchParams.get(MonkSearchParam.STEERING_WHEEL) ?? param);
const lang = monkSearchParams.get(MonkSearchParam.LANGUAGE);
if (lang) {
if (lang && isMounted()) {
onFetchLanguage?.(lang);
}
}

if (fetchedToken) {
if (fetchedToken && isMounted()) {
setAuthToken(fetchedToken);
onFetchAuthToken?.();
}
Expand Down
19 changes: 19 additions & 0 deletions packages/common/src/hooks/useIsMounted.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { useCallback, useEffect, useRef } from 'react';

/**
* Custom hook returning a ref to a util function returning `true` if the component using the hook is mounted, and false
* otherwise.
*/
export function useIsMounted(): () => boolean {
const isMounted = useRef(false);

useEffect(() => {
isMounted.current = true;

return () => {
isMounted.current = false;
};
}, []);

return useCallback(() => isMounted.current, []);
}
11 changes: 11 additions & 0 deletions packages/common/test/hooks/useIsMounted.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { useIsMounted } from '../../src/hooks/useIsMounted';
import { renderHook } from '@testing-library/react-hooks';

describe('useIsMounted hook', () => {
it('should return true when the component is mounted and false when unmounted', () => {
const { result, unmount } = renderHook(useIsMounted);
expect(result.current()).toBe(true);
unmount();
expect(result.current()).toBe(false);
});
});

0 comments on commit c53e15d

Please sign in to comment.