From 1ba0c240719a4ae650ce37f51c415802d0dbcaa9 Mon Sep 17 00:00:00 2001 From: Baptiste Devessier Date: Thu, 17 Apr 2025 16:57:49 +0200 Subject: [PATCH] Set the page hotkey scope only once (#11626) The `useEffect` in the `PageChangeEffect` component that sets the page's hotkey scope is run 4-5 times every time the page changes. It should be executed only once when the location changes. The re-runs are due to unstable dependencies that shouldn't re-trigger the `useEffect`. This PR minimizes the re-runs of the `useEffect`. In the following video, I discuss a more global pattern. The documentation for the `useEffectEvent` hook: https://react.dev/learn/separating-events-from-effects#extracting-non-reactive-logic-out-of-effects. https://github.com/user-attachments/assets/8c0c238b-f1c4-4bbb-b083-0825f7176599 --- .../src/hooks/useIsMatchingLocation.ts | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/twenty-front/src/hooks/useIsMatchingLocation.ts b/packages/twenty-front/src/hooks/useIsMatchingLocation.ts index 456efaf1a..70c175e3e 100644 --- a/packages/twenty-front/src/hooks/useIsMatchingLocation.ts +++ b/packages/twenty-front/src/hooks/useIsMatchingLocation.ts @@ -2,27 +2,31 @@ import { matchPath, useLocation } from 'react-router-dom'; import { AppBasePath } from '@/types/AppBasePath'; import { isNonEmptyString } from '@sniptt/guards'; +import { useCallback } from 'react'; import { isDefined } from 'twenty-shared/utils'; +const addTrailingSlash = (path: string) => + path.endsWith('/') ? path : path + '/'; + +const getConstructedPath = (path: string, basePath?: AppBasePath) => { + if (!isNonEmptyString(basePath)) return path; + + return addTrailingSlash(basePath) + path; +}; + export const useIsMatchingLocation = () => { const location = useLocation(); - const addTrailingSlash = (path: string) => - path.endsWith('/') ? path : path + '/'; - - const getConstructedPath = (path: string, basePath?: AppBasePath) => { - if (!isNonEmptyString(basePath)) return path; - - return addTrailingSlash(basePath) + path; - }; - - const isMatchingLocation = (path: string, basePath?: AppBasePath) => { - const match = matchPath( - getConstructedPath(path, basePath), - location.pathname, - ); - return isDefined(match); - }; + const isMatchingLocation = useCallback( + (path: string, basePath?: AppBasePath) => { + const match = matchPath( + getConstructedPath(path, basePath), + location.pathname, + ); + return isDefined(match); + }, + [location.pathname], + ); return { isMatchingLocation,