Skip to content

Commit

Permalink
fix: separate global hooks from app (#1487)
Browse files Browse the repository at this point in the history
Description
---
Most of the hooks were place inside `AppWrapper` which caused constant
rerender of the whole app when any of these hooks had side effects.

Case I also fixed that visualize the problem:
There was a hook 
```
    const { setReleaseNotes, setIsAppUpdateAvailable } = useAppStateStore((state) => ({
        setReleaseNotes: state.setReleaseNotes,
        setIsAppUpdateAvailable: state.setIsAppUpdateAvailable,
    }));
```
Whenever any property of the `AppStateStore` changed, there was a check
if the value had actually changed and the component needs to be
rerendered. In this case, even though the `setReleaseNotes` and
`setIsAppUpdateAvailable` references remained unchanged, we returned
them together as an object, which reference actually is always
different. As a result, the component was rerendered, even though no
actual changes occurred.

Simple solution:
```
    const setReleaseNotes = useAppStateStore((state) => state.setReleaseNotes);
    const setIsAppUpdateAvailable = useAppStateStore((state) => state.setIsAppUpdateAvailable);
```

* The issue was that this hook was positioned at the top of the tree,
causing a **RERENDER OF THE ENTIRE APP**.

These seemingly minor details can significantly impact overall
performance. To mitigate this, I decided to encapsulate all of these
hooks within separate component that don't render anything, ensuring
that this issue does not happen again.

Co-authored-by: shan <[email protected]>
Co-authored-by: Brian Pearce <[email protected]>
  • Loading branch information
3 people authored Feb 7, 2025
1 parent ffad14d commit ff59462
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 35 deletions.
2 changes: 1 addition & 1 deletion dist-isolation/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@
</script>
</body>

</html>
</html>
34 changes: 34 additions & 0 deletions src/App/AppEffects.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { useEffect } from 'react';

import { useDetectMode, useDisableRefresh, useLangaugeResolver, useListenForExternalDependencies } from '@app/hooks';

import { fetchAppConfig } from '../store/useAppConfigStore.ts';
import setupLogger from '../utils/shared-logger.ts';
import useListenForCriticalProblem from '@app/hooks/useListenForCriticalProblem.tsx';
import { setMiningNetwork } from '@app/store/miningStoreActions.ts';
import useTauriEventsListener from '@app/hooks/app/useTauriEventsListener.ts';
import { useListenForAppUpdated } from '@app/hooks/app/useListenForAppUpdated.ts';

// This component is used to initialise the app and listen for any events that need to be listened to
// Created as separate component to avoid cluttering the main App component and unwanted re-renders

setupLogger();
export default function AppEffects() {
useDetectMode();
useDisableRefresh();
useLangaugeResolver();
useListenForExternalDependencies();
useListenForCriticalProblem();
useTauriEventsListener();
useListenForAppUpdated({ triggerEffect: true });

useEffect(() => {
async function initialize() {
await fetchAppConfig();
await setMiningNetwork();
}
void initialize();
}, []);

return null;
}
37 changes: 7 additions & 30 deletions src/App/AppWrapper.tsx
Original file line number Diff line number Diff line change
@@ -1,34 +1,11 @@
import { useEffect } from 'react';

import { useDetectMode, useDisableRefresh, useLangaugeResolver, useListenForExternalDependencies } from '@app/hooks';

import { fetchAppConfig } from '../store/useAppConfigStore.ts';
import setupLogger from '../utils/shared-logger.ts';
import useListenForCriticalProblem from '@app/hooks/useListenForCriticalProblem.tsx';
import { setMiningNetwork } from '@app/store/miningStoreActions.ts';
import App from './App.tsx';
import useTauriEventsListener from '@app/hooks/app/useTauriEventsListener.ts';
import { useListenForAppUpdated } from '@app/hooks/app/useListenForAppUpdated.ts';

// FOR ANYTHING THAT NEEDS TO BE INITIALISED
import AppEffects from './AppEffects.tsx';

setupLogger();
export default function AppWrapper() {
useDetectMode();
useDisableRefresh();
useLangaugeResolver();
useListenForExternalDependencies();
useListenForCriticalProblem();
useTauriEventsListener();
useListenForAppUpdated({ triggerEffect: true });

useEffect(() => {
async function initialize() {
await fetchAppConfig();
await setMiningNetwork();
}
void initialize();
}, []);

return <App />;
return (
<>
<AppEffects />
<App />
</>
);
}
7 changes: 3 additions & 4 deletions src/hooks/app/useListenForAppUpdated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ interface ShowReleaseNotesPayload {
export const useListenForAppUpdated = (options: UseListenForAppUpdatedOptions) => {
const { triggerEffect } = options;
const { setDialogToShow } = useUIStore();
const { setReleaseNotes, setIsAppUpdateAvailable } = useAppStateStore((state) => ({
setReleaseNotes: state.setReleaseNotes,
setIsAppUpdateAvailable: state.setIsAppUpdateAvailable,
}));
const setReleaseNotes = useAppStateStore((state) => state.setReleaseNotes);
const setIsAppUpdateAvailable = useAppStateStore((state) => state.setIsAppUpdateAvailable);

useEffect(() => {
if (!triggerEffect) return;

Expand Down

0 comments on commit ff59462

Please sign in to comment.