admin/twenty
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix "PageChangeEffect does not run when changing view on the same object" (#12196)

Browse Source 
Fixes https://github.com/twentyhq/core-team-issues/issues/950

This issue was due to the memoization inside `useIsMatchingLocation`,
which was rerendered only if the pathname changed but not the search
params.

After discussion with @lucasbordeau, we decided to remove the hook
`useIsMatchingLocation` and to create an equivalent util function which
takes the location as an argument.

---------

Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com>
This commit is contained in:
Raphaël Bosi
2025-05-22 12:06:07 +02:00
committed by GitHub
parent 6466f3fb45
commit ffdedf7af3
16 changed files with 232 additions and 195 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
packages/twenty-front/src/hooks/__tests__/useIsMatchingLocation.test.tsx
View File

@ -1,74 +0,0 @@
import { renderHook } from '@testing-library/react';
import { useIsMatchingLocation } from '../useIsMatchingLocation';
import { MemoryRouter } from 'react-router-dom';
import { AppBasePath } from '@/types/AppBasePath';
const Wrapper =
(initialIndex = 0) =>
({ children }: { children: React.ReactNode }) => (
<MemoryRouter
initialEntries={['/example', '/other', `${AppBasePath.Settings}/example`]}
initialIndex={initialIndex}
>
{children}
</MemoryRouter>
);
describe('useIsMatchingLocation', () => {
it('returns true when paths match with no basePath', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(),
});
expect(result.current.isMatchingLocation('/example')).toBe(true);
});
it('returns false when paths do not match with no basePath', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(),
});
expect(result.current.isMatchingLocation('/non-match')).toBe(false);
});
it('returns true when paths match with basePath', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(2),
});
expect(
result.current.isMatchingLocation('example', AppBasePath.Settings),
).toBe(true);
});
it('returns false when paths do not match with basePath', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(),
});
expect(
result.current.isMatchingLocation('non-match', AppBasePath.Settings),
).toBe(false);
});
it('handles trailing slashes in basePath correctly', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(2),
});
expect(
result.current.isMatchingLocation(
'example',
(AppBasePath.Settings + '/') as AppBasePath,
),
).toBe(true);
});
it('handles without basePath correctly', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(),
});
expect(result.current.isMatchingLocation('example')).toBe(true);
});
});

12
packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts
View File

@ -8,9 +8,9 @@ import { useRecoilValue } from 'recoil';
import { OnboardingStatus } from '~/generated/graphql';
import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation';
import { usePageChangeEffectNavigateLocation } from '~/hooks/usePageChangeEffectNavigateLocation';
import { UNTESTED_APP_PATHS } from '~/testing/constants/UntestedAppPaths';
import { isMatchingLocation } from '~/utils/isMatchingLocation';
jest.mock('@/onboarding/hooks/useOnboardingStatus');
const setupMockOnboardingStatus = (
@ -28,13 +28,13 @@ const setupMockIsWorkspaceActivationStatusEqualsTo = (
.mockReturnValueOnce(isWorkspaceSuspended);
};
jest.mock('~/hooks/useIsMatchingLocation');
const mockUseIsMatchingLocation = jest.mocked(useIsMatchingLocation);
jest.mock('~/utils/isMatchingLocation');
const mockIsMatchingLocation = jest.mocked(isMatchingLocation);
const setupMockIsMatchingLocation = (pathname: string) => {
mockUseIsMatchingLocation.mockReturnValueOnce({
isMatchingLocation: (path: string) => path === pathname,
});
mockIsMatchingLocation.mockImplementation(
(_location, path) => path === pathname,
);
};
jest.mock('@/auth/hooks/useIsLogged');

34
packages/twenty-front/src/hooks/useIsMatchingLocation.ts
View File

@ -1,34 +0,0 @@
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 isMatchingLocation = useCallback(
(path: string, basePath?: AppBasePath) => {
const match = matchPath(
getConstructedPath(path, basePath),
location.pathname,
);
return isDefined(match);
},
[location.pathname],
);
return {
isMatchingLocation,
};
};

25
packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts
View File

@ -5,24 +5,24 @@ import { useOnboardingStatus } from '@/onboarding/hooks/useOnboardingStatus';
import { AppPath } from '@/types/AppPath';
import { SettingsPath } from '@/types/SettingsPath';
import { useIsWorkspaceActivationStatusEqualsTo } from '@/workspace/hooks/useIsWorkspaceActivationStatusEqualsTo';
import { useParams } from 'react-router-dom';
import { useLocation, useParams } from 'react-router-dom';
import { useRecoilValue } from 'recoil';
import { isDefined } from 'twenty-shared/utils';
import { WorkspaceActivationStatus } from 'twenty-shared/workspace';
import { OnboardingStatus } from '~/generated/graphql';
import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation';
import { isMatchingLocation } from '~/utils/isMatchingLocation';
export const usePageChangeEffectNavigateLocation = () => {
const { isMatchingLocation } = useIsMatchingLocation();
const isLoggedIn = useIsLogged();
const onboardingStatus = useOnboardingStatus();
const isWorkspaceSuspended = useIsWorkspaceActivationStatusEqualsTo(
WorkspaceActivationStatus.SUSPENDED,
);
const { defaultHomePagePath } = useDefaultHomePagePath();
const location = useLocation();
const someMatchingLocationOf = (appPaths: AppPath[]): boolean =>
appPaths.some((appPath) => isMatchingLocation(appPath));
appPaths.some((appPath) => isMatchingLocation(location, appPath));
const onGoingUserCreationPaths = [
AppPath.Invite,
AppPath.SignInUp,
@ -61,7 +61,10 @@ export const usePageChangeEffectNavigateLocation = () => {
return AppPath.PlanRequired;
}
if (isWorkspaceSuspended && !isMatchingLocation(AppPath.SettingsCatchAll)) {
if (
isWorkspaceSuspended &&
!isMatchingLocation(location, AppPath.SettingsCatchAll)
) {
return `${AppPath.SettingsCatchAll.replace('/*', '')}/${
SettingsPath.Billing
}`;
@ -79,21 +82,21 @@ export const usePageChangeEffectNavigateLocation = () => {
if (
onboardingStatus === OnboardingStatus.PROFILE_CREATION &&
!isMatchingLocation(AppPath.CreateProfile)
!isMatchingLocation(location, AppPath.CreateProfile)
) {
return AppPath.CreateProfile;
}
if (
onboardingStatus === OnboardingStatus.SYNC_EMAIL &&
!isMatchingLocation(AppPath.SyncEmails)
!isMatchingLocation(location, AppPath.SyncEmails)
) {
return AppPath.SyncEmails;
}
if (
onboardingStatus === OnboardingStatus.INVITE_TEAM &&
!isMatchingLocation(AppPath.InviteTeam)
!isMatchingLocation(location, AppPath.InviteTeam)
) {
return AppPath.InviteTeam;
}
@ -101,18 +104,18 @@ export const usePageChangeEffectNavigateLocation = () => {
if (
onboardingStatus === OnboardingStatus.COMPLETED &&
someMatchingLocationOf([...onboardingPaths, ...onGoingUserCreationPaths]) &&
!isMatchingLocation(AppPath.ResetPassword) &&
!isMatchingLocation(location, AppPath.ResetPassword) &&
isLoggedIn
) {
return defaultHomePagePath;
}
if (isMatchingLocation(AppPath.Index) && isLoggedIn) {
if (isMatchingLocation(location, AppPath.Index) && isLoggedIn) {
return defaultHomePagePath;
}
if (
isMatchingLocation(AppPath.RecordIndexPage) &&
isMatchingLocation(location, AppPath.RecordIndexPage) &&
!isDefined(objectMetadataItem)
) {
return AppPath.NotFound;