From 58b40b1f8936ab7f873e006fe5560a27a11673d0 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Sat, 17 May 2025 15:41:01 +0200 Subject: [PATCH] Removed value setter effect completely (#12101) This PR removes the effect component that was synchronizing the record store recoil state with the context selector equivalent state that is used for performance on the tables. Now we only set the context selector in parallel with the recoil state, thus avoiding any synchronization side-effect between those two states. --- .../components/CalendarEventDetailsEffect.tsx | 5 ++- .../CommandMenuCalendarEventPage.tsx | 14 +++++--- .../components/RecordBoardCard.tsx | 2 -- .../hooks/useLoadRecordIndexBoardColumn.ts | 7 +++- .../RecordDetailRelationRecordsListItem.tsx | 2 -- ...ordDetailRelationRecordsListItemEffect.tsx | 6 +++- .../components/RecordValueSetterEffect.tsx | 35 ------------------- .../hooks/internal/useSetRecordTableData.ts | 11 ++++-- .../record-table/hooks/useRecordTable.ts | 4 --- .../components/RecordTableRow.tsx | 2 -- 10 files changed, 34 insertions(+), 54 deletions(-) delete mode 100644 packages/twenty-front/src/modules/object-record/record-store/components/RecordValueSetterEffect.tsx diff --git a/packages/twenty-front/src/modules/activities/calendar/components/CalendarEventDetailsEffect.tsx b/packages/twenty-front/src/modules/activities/calendar/components/CalendarEventDetailsEffect.tsx index 970cfc412..2f8b72a85 100644 --- a/packages/twenty-front/src/modules/activities/calendar/components/CalendarEventDetailsEffect.tsx +++ b/packages/twenty-front/src/modules/activities/calendar/components/CalendarEventDetailsEffect.tsx @@ -1,4 +1,5 @@ import { CalendarEvent } from '@/activities/calendar/types/CalendarEvent'; +import { useSetRecordValue } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext'; import { useUpsertRecordsInStore } from '@/object-record/record-store/hooks/useUpsertRecordsInStore'; import { useEffect } from 'react'; @@ -10,6 +11,7 @@ export const CalendarEventDetailsEffect = ({ record, }: CalendarEventDetailsEffectProps) => { const { upsertRecords } = useUpsertRecordsInStore(); + const setRecordValueInContextSelector = useSetRecordValue(); useEffect(() => { if (!record) { @@ -17,7 +19,8 @@ export const CalendarEventDetailsEffect = ({ } upsertRecords([record]); - }, [record, upsertRecords]); + setRecordValueInContextSelector(record.id, record); + }, [record, upsertRecords, setRecordValueInContextSelector]); return <>; }; diff --git a/packages/twenty-front/src/modules/command-menu/pages/calendar-event/components/CommandMenuCalendarEventPage.tsx b/packages/twenty-front/src/modules/command-menu/pages/calendar-event/components/CommandMenuCalendarEventPage.tsx index e63155544..d1208e4bd 100644 --- a/packages/twenty-front/src/modules/command-menu/pages/calendar-event/components/CommandMenuCalendarEventPage.tsx +++ b/packages/twenty-front/src/modules/command-menu/pages/calendar-event/components/CommandMenuCalendarEventPage.tsx @@ -4,8 +4,10 @@ import { FIND_ONE_CALENDAR_EVENT_OPERATION_SIGNATURE } from '@/activities/calend import { CalendarEvent } from '@/activities/calendar/types/CalendarEvent'; import { viewableRecordIdComponentState } from '@/command-menu/pages/record-page/states/viewableRecordIdComponentState'; import { useFindOneRecord } from '@/object-record/hooks/useFindOneRecord'; -import { RecordValueSetterEffect } from '@/object-record/record-store/components/RecordValueSetterEffect'; -import { RecordFieldValueSelectorContextProvider } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext'; +import { + RecordFieldValueSelectorContextProvider, + useSetRecordValue, +} from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext'; import { useUpsertRecordsInStore } from '@/object-record/record-store/hooks/useUpsertRecordsInStore'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; @@ -14,13 +16,18 @@ export const CommandMenuCalendarEventPage = () => { const viewableRecordId = useRecoilComponentValueV2( viewableRecordIdComponentState, ); + const setRecordValueInContextSelector = useSetRecordValue(); const { record: calendarEvent } = useFindOneRecord({ objectNameSingular: FIND_ONE_CALENDAR_EVENT_OPERATION_SIGNATURE.objectNameSingular, objectRecordId: viewableRecordId ?? '', recordGqlFields: FIND_ONE_CALENDAR_EVENT_OPERATION_SIGNATURE.fields, - onCompleted: (record) => upsertRecords([record]), + // TODO: this is not executed on sub-sequent runs, make sure that it is intended + onCompleted: (record) => { + upsertRecords([record]); + setRecordValueInContextSelector(record.id, record); + }, }); if (!calendarEvent) { @@ -30,7 +37,6 @@ export const CommandMenuCalendarEventPage = () => { return ( - ); diff --git a/packages/twenty-front/src/modules/object-record/record-board/record-board-card/components/RecordBoardCard.tsx b/packages/twenty-front/src/modules/object-record/record-board/record-board-card/components/RecordBoardCard.tsx index b77f0a166..94437960a 100644 --- a/packages/twenty-front/src/modules/object-record/record-board/record-board-card/components/RecordBoardCard.tsx +++ b/packages/twenty-front/src/modules/object-record/record-board/record-board-card/components/RecordBoardCard.tsx @@ -15,7 +15,6 @@ import { RecordBoardCardBody } from '@/object-record/record-board/record-board-c import { RecordBoardCardHeader } from '@/object-record/record-board/record-board-card/components/RecordBoardCardHeader'; import { useRecordIndexContextOrThrow } from '@/object-record/record-index/contexts/RecordIndexContext'; import { recordIndexOpenRecordInState } from '@/object-record/record-index/states/recordIndexOpenRecordInState'; -import { RecordValueSetterEffect } from '@/object-record/record-store/components/RecordValueSetterEffect'; import { AppPath } from '@/types/AppPath'; import { useDropdownV2 } from '@/ui/layout/dropdown/hooks/useDropdownV2'; import { useAvailableScopeIdOrThrow } from '@/ui/utilities/recoil-scope/scopes-internal/hooks/useAvailableScopeId'; @@ -201,7 +200,6 @@ export const RecordBoardCard = () => { className="record-board-card" onContextMenu={handleActionMenuDropdown} > - { + const setRecordValueInContextSelector = useSetRecordValue(); const { objectMetadataItem } = useObjectMetadataItem({ objectNameSingular, }); @@ -100,7 +102,10 @@ export const useLoadRecordIndexBoardColumn = ({ useEffect(() => { upsertRecordsInStore(records); - }, [records, upsertRecordsInStore]); + for (const record of records) { + setRecordValueInContextSelector(record.id, record); + } + }, [records, upsertRecordsInStore, setRecordValueInContextSelector]); return { records, diff --git a/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx b/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx index a34057399..ded78d000 100644 --- a/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx +++ b/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx @@ -24,7 +24,6 @@ import { FieldRelationMetadata } from '@/object-record/record-field/types/FieldM import { RecordInlineCell } from '@/object-record/record-inline-cell/components/RecordInlineCell'; import { PropertyBox } from '@/object-record/record-inline-cell/property-box/components/PropertyBox'; import { RecordDetailRecordsListItem } from '@/object-record/record-show/record-detail-section/components/RecordDetailRecordsListItem'; -import { RecordValueSetterEffect } from '@/object-record/record-store/components/RecordValueSetterEffect'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; import { getForeignKeyNameFromRelationFieldName } from '@/object-record/utils/getForeignKeyNameFromRelationFieldName'; import { getRecordFieldInputId } from '@/object-record/utils/getRecordFieldInputId'; @@ -221,7 +220,6 @@ export const RecordDetailRelationRecordsListItem = ({ return ( <> - { + const setRecordValueInContextSelector = useSetRecordValue(); + const { fieldDefinition } = useContext(FieldContext); const { relationObjectMetadataNameSingular } = @@ -28,8 +31,9 @@ export const RecordDetailRelationRecordsListItemEffect = ({ useEffect(() => { if (isDefined(record)) { upsertRecords([record]); + setRecordValueInContextSelector(record.id, record); } - }, [record, upsertRecords]); + }, [record, upsertRecords, setRecordValueInContextSelector]); return null; }; diff --git a/packages/twenty-front/src/modules/object-record/record-store/components/RecordValueSetterEffect.tsx b/packages/twenty-front/src/modules/object-record/record-store/components/RecordValueSetterEffect.tsx deleted file mode 100644 index c5588f509..000000000 --- a/packages/twenty-front/src/modules/object-record/record-store/components/RecordValueSetterEffect.tsx +++ /dev/null @@ -1,35 +0,0 @@ -import { useEffect } from 'react'; -import { useRecoilValue } from 'recoil'; - -import { - useRecordValue, - useSetRecordValue, -} from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext'; -import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState'; - -// TODO: should be optimized and put higher up -export const RecordValueSetterEffect = ({ recordId }: { recordId: string }) => { - const setRecordValueInContextSelector = useSetRecordValue(); - - const recordValueFromContextSelector = useRecordValue(recordId); - - const recordValueFromRecoil = useRecoilValue( - recordStoreFamilyState(recordId), - ); - - useEffect(() => { - if ( - JSON.stringify(recordValueFromContextSelector) !== - JSON.stringify(recordValueFromRecoil) - ) { - setRecordValueInContextSelector(recordId, recordValueFromRecoil); - } - }, [ - setRecordValueInContextSelector, - recordValueFromRecoil, - recordId, - recordValueFromContextSelector, - ]); - - return null; -}; diff --git a/packages/twenty-front/src/modules/object-record/record-table/hooks/internal/useSetRecordTableData.ts b/packages/twenty-front/src/modules/object-record/record-table/hooks/internal/useSetRecordTableData.ts index db22c3357..4c4d80991 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/hooks/internal/useSetRecordTableData.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/hooks/internal/useSetRecordTableData.ts @@ -2,6 +2,7 @@ import { useRecoilCallback } from 'recoil'; import { recordIndexRecordIdsByGroupComponentFamilyState } from '@/object-record/record-index/states/recordIndexRecordIdsByGroupComponentFamilyState'; import { recordIndexAllRecordIdsComponentSelector } from '@/object-record/record-index/states/selectors/recordIndexAllRecordIdsComponentSelector'; +import { useSetRecordValue } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext'; import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState'; import { useSetIsRecordTableFocusActive } from '@/object-record/record-table/record-table-cell/hooks/useSetIsRecordTableFocusActive'; import { hasUserSelectedAllRowsComponentState } from '@/object-record/record-table/record-table-row/states/hasUserSelectedAllRowsFamilyState'; @@ -26,6 +27,8 @@ export const useSetRecordTableData = ({ recordTableId, onEntityCountChange, }: useSetRecordTableDataProps) => { + const setRecordValueInContextSelector = useSetRecordValue(); + const recordIndexRecordIdsByGroupFamilyState = useRecoilComponentCallbackStateV2( recordIndexRecordIdsByGroupComponentFamilyState, @@ -73,10 +76,13 @@ export const useSetRecordTableData = ({ .getValue(); if (JSON.stringify(currentRecord) !== JSON.stringify(record)) { - set(recordStoreFamilyState(record.id), { + const newRecord = { ...currentRecord, ...record, - }); + }; + + set(recordStoreFamilyState(record.id), newRecord); + setRecordValueInContextSelector(record.id, newRecord); } } @@ -124,6 +130,7 @@ export const useSetRecordTableData = ({ setRecordTableHoverPosition, onEntityCountChange, isRowSelectedFamilyState, + setRecordValueInContextSelector, ], ); }; diff --git a/packages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTable.ts b/packages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTable.ts index 93a12a9b5..ab4bdf8af 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTable.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTable.ts @@ -8,7 +8,6 @@ import { getSnapshotValue } from '@/ui/utilities/recoil-scope/utils/getSnapshotV import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; import { RecordIndexHotkeyScope } from '@/object-record/record-index/types/RecordIndexHotkeyScope'; -import { useUpsertRecordFromState } from '../../hooks/useUpsertRecordFromState'; import { ColumnDefinition } from '../types/ColumnDefinition'; import { TableHotkeyScope } from '../types/TableHotkeyScope'; @@ -136,8 +135,6 @@ export const useRecordTable = (props?: useRecordTableProps) => { const resetTableRowSelection = useResetTableRowSelection(recordTableId); - const upsertRecordTableItem = useUpsertRecordFromState; - const setFocusPosition = useSetRecordTableFocusPosition(recordTableId); const { setHotkeyScopeAndMemorizePreviousScope } = usePreviousHotkeyScope(); @@ -212,7 +209,6 @@ export const useRecordTable = (props?: useRecordTableProps) => { leaveTableFocus, setRowSelected, resetTableRowSelection, - upsertRecordTableItem, move, useMapKeyboardToFocus, selectAllRows, diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRow.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRow.tsx index 32c2007e5..f3b4cfe61 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRow.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRow.tsx @@ -1,5 +1,4 @@ import { useRecordIndexContextOrThrow } from '@/object-record/record-index/contexts/RecordIndexContext'; -import { RecordValueSetterEffect } from '@/object-record/record-store/components/RecordValueSetterEffect'; import { RecordTableCellCheckbox } from '@/object-record/record-table/record-table-cell/components/RecordTableCellCheckbox'; import { RecordTableCellGrip } from '@/object-record/record-table/record-table-cell/components/RecordTableCellGrip'; import { RecordTableLastEmptyCell } from '@/object-record/record-table/record-table-cell/components/RecordTableLastEmptyCell'; @@ -47,7 +46,6 @@ export const RecordTableRow = ({ -