Fixes infinite loop on record data update in command menu (#12072)
This PR fixes the infinite loop that was happening in `RecordShowEffect` component due to a useEffect on `recordStoreFamilyState` which was creating a non-deterministic open loop. The fix was to use a recoilCallback to avoid reading a stale state from Recoil with `useRecoilValue`, useRecoilCallback always gets the most fresh data and commits everything before React goes on with rendering, thus avoiding any stale value in a useEffect. Fixes https://github.com/twentyhq/twenty/issues/11079 Fixes https://github.com/twentyhq/core-team-issues/issues/957
This commit is contained in:
@ -38,7 +38,7 @@ export const RecordShowRightDrawerOpenRecordButton = ({
|
|||||||
objectNameSingular,
|
objectNameSingular,
|
||||||
recordId,
|
recordId,
|
||||||
}: RecordShowRightDrawerOpenRecordButtonProps) => {
|
}: RecordShowRightDrawerOpenRecordButtonProps) => {
|
||||||
const record = useRecoilValue<ObjectRecord | null>(
|
const record = useRecoilValue<ObjectRecord | null | undefined>(
|
||||||
recordStoreFamilyState(recordId),
|
recordStoreFamilyState(recordId),
|
||||||
);
|
);
|
||||||
const { closeCommandMenu } = useCommandMenu();
|
const { closeCommandMenu } = useCommandMenu();
|
||||||
|
|||||||
@ -9,7 +9,6 @@ import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord';
|
|||||||
import { searchRecordStoreComponentFamilyState } from '@/object-record/record-picker/multiple-record-picker/states/searchRecordStoreComponentFamilyState';
|
import { searchRecordStoreComponentFamilyState } from '@/object-record/record-picker/multiple-record-picker/states/searchRecordStoreComponentFamilyState';
|
||||||
import { RecordPickerPickableMorphItem } from '@/object-record/record-picker/types/RecordPickerPickableMorphItem';
|
import { RecordPickerPickableMorphItem } from '@/object-record/record-picker/types/RecordPickerPickableMorphItem';
|
||||||
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
|
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
|
||||||
import { isNull } from '@sniptt/guards';
|
|
||||||
import { useRecoilCallback, useSetRecoilState } from 'recoil';
|
import { useRecoilCallback, useSetRecoilState } from 'recoil';
|
||||||
import { isDefined } from 'twenty-shared/utils';
|
import { isDefined } from 'twenty-shared/utils';
|
||||||
import { v4 } from 'uuid';
|
import { v4 } from 'uuid';
|
||||||
@ -171,7 +170,7 @@ export const useUpdateActivityTargetFromCell = ({
|
|||||||
}
|
}
|
||||||
|
|
||||||
setActivityFromStore((currentActivity) => {
|
setActivityFromStore((currentActivity) => {
|
||||||
if (isNull(currentActivity)) {
|
if (!isDefined(currentActivity)) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -10,7 +10,6 @@ import { RecordShowContainer } from '@/object-record/record-show/components/Reco
|
|||||||
import { RecordShowEffect } from '@/object-record/record-show/components/RecordShowEffect';
|
import { RecordShowEffect } from '@/object-record/record-show/components/RecordShowEffect';
|
||||||
import { useRecordShowPage } from '@/object-record/record-show/hooks/useRecordShowPage';
|
import { useRecordShowPage } from '@/object-record/record-show/hooks/useRecordShowPage';
|
||||||
import { RecordSortsComponentInstanceContext } from '@/object-record/record-sort/states/context/RecordSortsComponentInstanceContext';
|
import { RecordSortsComponentInstanceContext } from '@/object-record/record-sort/states/context/RecordSortsComponentInstanceContext';
|
||||||
import { RecordValueSetterEffect } from '@/object-record/record-store/components/RecordValueSetterEffect';
|
|
||||||
import { RecordFieldValueSelectorContextProvider } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';
|
import { RecordFieldValueSelectorContextProvider } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';
|
||||||
import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile';
|
import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile';
|
||||||
import { useComponentInstanceStateContext } from '@/ui/utilities/state/component-state/hooks/useComponentInstanceStateContext';
|
import { useComponentInstanceStateContext } from '@/ui/utilities/state/component-state/hooks/useComponentInstanceStateContext';
|
||||||
@ -74,7 +73,6 @@ export const CommandMenuRecordPage = () => {
|
|||||||
>
|
>
|
||||||
<StyledRightDrawerRecord isMobile={isMobile}>
|
<StyledRightDrawerRecord isMobile={isMobile}>
|
||||||
<RecordFieldValueSelectorContextProvider>
|
<RecordFieldValueSelectorContextProvider>
|
||||||
<RecordValueSetterEffect recordId={objectRecordId} />
|
|
||||||
<TimelineActivityContext.Provider
|
<TimelineActivityContext.Provider
|
||||||
value={{
|
value={{
|
||||||
recordId: objectRecordId,
|
recordId: objectRecordId,
|
||||||
|
|||||||
@ -2,11 +2,11 @@ import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadata
|
|||||||
import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems';
|
import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems';
|
||||||
import { useFindOneRecord } from '@/object-record/hooks/useFindOneRecord';
|
import { useFindOneRecord } from '@/object-record/hooks/useFindOneRecord';
|
||||||
import { buildFindOneRecordForShowPageOperationSignature } from '@/object-record/record-show/graphql/operations/factories/findOneRecordForShowPageOperationSignatureFactory';
|
import { buildFindOneRecordForShowPageOperationSignature } from '@/object-record/record-show/graphql/operations/factories/findOneRecordForShowPageOperationSignatureFactory';
|
||||||
|
import { useSetRecordValue } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';
|
||||||
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
|
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
|
||||||
|
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
|
||||||
import { useEffect } from 'react';
|
import { useEffect } from 'react';
|
||||||
import { useRecoilState } from 'recoil';
|
import { useRecoilCallback } from 'recoil';
|
||||||
import { isDeeplyEqual } from '~/utils/isDeeplyEqual';
|
|
||||||
import { isDefined } from 'twenty-shared/utils';
|
|
||||||
|
|
||||||
type RecordShowEffectProps = {
|
type RecordShowEffectProps = {
|
||||||
objectNameSingular: string;
|
objectNameSingular: string;
|
||||||
@ -19,6 +19,7 @@ export const RecordShowEffect = ({
|
|||||||
}: RecordShowEffectProps) => {
|
}: RecordShowEffectProps) => {
|
||||||
const { objectMetadataItem } = useObjectMetadataItem({ objectNameSingular });
|
const { objectMetadataItem } = useObjectMetadataItem({ objectNameSingular });
|
||||||
const { objectMetadataItems } = useObjectMetadataItems();
|
const { objectMetadataItems } = useObjectMetadataItems();
|
||||||
|
const setRecordValueInContextSelector = useSetRecordValue();
|
||||||
|
|
||||||
const FIND_ONE_RECORD_FOR_SHOW_PAGE_OPERATION_SIGNATURE =
|
const FIND_ONE_RECORD_FOR_SHOW_PAGE_OPERATION_SIGNATURE =
|
||||||
buildFindOneRecordForShowPageOperationSignature({
|
buildFindOneRecordForShowPageOperationSignature({
|
||||||
@ -33,15 +34,25 @@ export const RecordShowEffect = ({
|
|||||||
withSoftDeleted: true,
|
withSoftDeleted: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
const [recordFromStore, setRecordFromStore] = useRecoilState(
|
const setRecordStore = useRecoilCallback(
|
||||||
recordStoreFamilyState(recordId),
|
({ snapshot, set }) =>
|
||||||
|
async (newRecord: ObjectRecord | null | undefined) => {
|
||||||
|
const previousRecordValue = snapshot
|
||||||
|
.getLoadable(recordStoreFamilyState(recordId))
|
||||||
|
.getValue();
|
||||||
|
|
||||||
|
if (JSON.stringify(previousRecordValue) !== JSON.stringify(newRecord)) {
|
||||||
|
set(recordStoreFamilyState(recordId), newRecord);
|
||||||
|
}
|
||||||
|
|
||||||
|
setRecordValueInContextSelector(recordId, newRecord);
|
||||||
|
},
|
||||||
|
[recordId, setRecordValueInContextSelector],
|
||||||
);
|
);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (isDefined(record) && !isDeeplyEqual(record, recordFromStore)) {
|
setRecordStore(record);
|
||||||
setRecordFromStore(record);
|
}, [record, setRecordStore]);
|
||||||
}
|
|
||||||
}, [record, recordFromStore, setRecordFromStore]);
|
|
||||||
|
|
||||||
return <></>;
|
return <></>;
|
||||||
};
|
};
|
||||||
|
|||||||
@ -6,7 +6,6 @@ import {
|
|||||||
useSetRecordValue,
|
useSetRecordValue,
|
||||||
} from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';
|
} from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';
|
||||||
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
|
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
|
||||||
import { isDeeplyEqual } from '~/utils/isDeeplyEqual';
|
|
||||||
|
|
||||||
// TODO: should be optimized and put higher up
|
// TODO: should be optimized and put higher up
|
||||||
export const RecordValueSetterEffect = ({ recordId }: { recordId: string }) => {
|
export const RecordValueSetterEffect = ({ recordId }: { recordId: string }) => {
|
||||||
@ -19,7 +18,10 @@ export const RecordValueSetterEffect = ({ recordId }: { recordId: string }) => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!isDeeplyEqual(recordValueFromContextSelector, recordValueFromRecoil)) {
|
if (
|
||||||
|
JSON.stringify(recordValueFromContextSelector) !==
|
||||||
|
JSON.stringify(recordValueFromRecoil)
|
||||||
|
) {
|
||||||
setRecordValueInContextSelector(recordId, recordValueFromRecoil);
|
setRecordValueInContextSelector(recordId, recordValueFromRecoil);
|
||||||
}
|
}
|
||||||
}, [
|
}, [
|
||||||
|
|||||||
@ -2,7 +2,7 @@ import { ObjectRecord } from '@/object-record/types/ObjectRecord';
|
|||||||
import { createFamilyState } from '@/ui/utilities/state/utils/createFamilyState';
|
import { createFamilyState } from '@/ui/utilities/state/utils/createFamilyState';
|
||||||
|
|
||||||
export const recordStoreFamilyState = createFamilyState<
|
export const recordStoreFamilyState = createFamilyState<
|
||||||
ObjectRecord | null,
|
ObjectRecord | null | undefined,
|
||||||
string
|
string
|
||||||
>({
|
>({
|
||||||
key: 'recordStoreFamilyState',
|
key: 'recordStoreFamilyState',
|
||||||
|
|||||||
@ -11,7 +11,6 @@ import { RecordShowContainer } from '@/object-record/record-show/components/Reco
|
|||||||
import { RecordShowEffect } from '@/object-record/record-show/components/RecordShowEffect';
|
import { RecordShowEffect } from '@/object-record/record-show/components/RecordShowEffect';
|
||||||
import { useRecordShowPage } from '@/object-record/record-show/hooks/useRecordShowPage';
|
import { useRecordShowPage } from '@/object-record/record-show/hooks/useRecordShowPage';
|
||||||
import { RecordSortsComponentInstanceContext } from '@/object-record/record-sort/states/context/RecordSortsComponentInstanceContext';
|
import { RecordSortsComponentInstanceContext } from '@/object-record/record-sort/states/context/RecordSortsComponentInstanceContext';
|
||||||
import { RecordValueSetterEffect } from '@/object-record/record-store/components/RecordValueSetterEffect';
|
|
||||||
import { RecordFieldValueSelectorContextProvider } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';
|
import { RecordFieldValueSelectorContextProvider } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';
|
||||||
import { PageHeaderToggleCommandMenuButton } from '@/ui/layout/page-header/components/PageHeaderToggleCommandMenuButton';
|
import { PageHeaderToggleCommandMenuButton } from '@/ui/layout/page-header/components/PageHeaderToggleCommandMenuButton';
|
||||||
import { PageBody } from '@/ui/layout/page/components/PageBody';
|
import { PageBody } from '@/ui/layout/page/components/PageBody';
|
||||||
@ -47,7 +46,6 @@ export const RecordShowPage = () => {
|
|||||||
<ActionMenuComponentInstanceContext.Provider
|
<ActionMenuComponentInstanceContext.Provider
|
||||||
value={{ instanceId: `record-show-${objectRecordId}` }}
|
value={{ instanceId: `record-show-${objectRecordId}` }}
|
||||||
>
|
>
|
||||||
<RecordValueSetterEffect recordId={objectRecordId} />
|
|
||||||
<PageContainer>
|
<PageContainer>
|
||||||
<RecordShowPageTitle
|
<RecordShowPageTitle
|
||||||
objectNameSingular={objectNameSingular}
|
objectNameSingular={objectNameSingular}
|
||||||
|
|||||||
Reference in New Issue
Block a user