From 5dca5ef42fab48b0e045fb56b83b9db3b896b437 Mon Sep 17 00:00:00 2001 From: Tom Butcher Date: Mon, 14 Sep 2026 10:20:53 +0100 Subject: [PATCH] Enhance ObjectSelect Component with Improved Value Handling and State Management - Updated value comparison logic in areValuesEqual to handle null values and case insensitivity. - Introduced load generation tracking to manage asynchronous updates and prevent stale data handling. - Refactored onTreeSelectChange to improve selection handling for both single and multiple selections. - Added useEffect hooks to manage state changes based on type and masterFilter updates, enhancing component responsiveness. - Implemented findObjectById function for better object retrieval from the list, improving performance and clarity. --- .../Dashboard/common/ObjectSelect.jsx | 264 ++++++++++-------- 1 file changed, 142 insertions(+), 122 deletions(-) diff --git a/src/components/Dashboard/common/ObjectSelect.jsx b/src/components/Dashboard/common/ObjectSelect.jsx index 76ab3a3b..9446bfc1 100644 --- a/src/components/Dashboard/common/ObjectSelect.jsx +++ b/src/components/Dashboard/common/ObjectSelect.jsx @@ -22,7 +22,8 @@ const EMPTY_OBJECT = {} const areValuesEqual = (v1, v2) => { const id1 = v1 && typeof v1 === 'object' && v1._id ? v1._id : v1 const id2 = v2 && typeof v2 === 'object' && v2._id ? v2._id : v2 - return String(id1) === String(id2) + if (id1 == null || id2 == null) return id1 == id2 + return String(id1).toLowerCase() === String(id2).toLowerCase() } const toSelectValue = (id) => { @@ -249,6 +250,15 @@ const ObjectSelect = ({ const filterRef = useRef(filter) const masterFilterRef = useRef(masterFilter) const clearedMissingValueRef = useRef(false) + const loadGenerationRef = useRef(0) + const getSelectKey = useCallback( + (selectType, selectMasterFilter) => + `${selectType}::${JSON.stringify(selectMasterFilter ?? {})}`, + [] + ) + const [committedSelectKey, setCommittedSelectKey] = useState(() => + getSelectKey(type, masterFilter) + ) filterRef.current = filter masterFilterRef.current = masterFilter @@ -366,12 +376,14 @@ const ObjectSelect = ({ // Fetch the object properties tree from the API const handleFetchObjectsProperties = useCallback( async (customFilter = filter, { replace = false } = {}) => { + const generation = loadGenerationRef.current try { const data = await fetchObjectsByProperty(type, { properties: properties, filter: customFilter, masterFilter }) + if (generation !== loadGenerationRef.current) return null if (Array.isArray(data)) { setObjectPropertiesTree((prev) => @@ -551,6 +563,7 @@ const ObjectSelect = ({ (data) => { const { treeNodes, objects } = buildTreeData(data) setObjectList(objects) + objectListRef.current = objects setTreeData(treeNodes) treeDataRef.current = treeNodes @@ -674,52 +687,64 @@ const ObjectSelect = ({ updateEventHandlerRef.current = updateEventHandler newEventHandlerRef.current = newEventHandler + const findObjectById = useCallback((id) => { + if (id == null || id === '') return null + return ( + objectListRef.current.find((obj) => areValuesEqual(obj._id, id)) || null + ) + }, []) + const onTreeSelectChange = useCallback( - (value) => { + (nextValue) => { const isEmptySelection = multiple - ? !Array.isArray(value) || value.length === 0 - : value == null || value === '' - if ( - isEmptySelection && - treeDataRef.current.length === 0 && - valueRef.current != null - ) { + ? !Array.isArray(nextValue) || nextValue.length === 0 + : nextValue == null || nextValue === '' + + const pendingId = getValueId(valueRef.current) + const pendingInTree = + pendingId != null && + (isValueInTree(treeDataRef.current, pendingId) || + findObjectById(pendingId) != null) + + // TreeSelect emits empty/null when treeData is replaced or a controlled + // value is not in the tree yet. Don't wipe an external value we still own. + if (isEmptySelection && pendingId != null && !pendingInTree) { return } setValueNotFound(false) clearedMissingValueRef.current = false - // Mark this as an internal change if (!multiple) setIsSearching(false) isInternalChangeRef.current = true - // value can be a string (single) or array (multiple) if (multiple) { - // Multiple selection let selectedObjects = [] - if (Array.isArray(value)) { - selectedObjects = value - .map((id) => objectList.find((obj) => areValuesEqual(obj._id, id))) - .filter(Boolean) + if (Array.isArray(nextValue)) { + selectedObjects = nextValue.map(findObjectById).filter(Boolean) } - setTreeSelectValue(value) + setTreeSelectValue(nextValue) onChange?.(selectedObjects) - } else { - // Single selection: replace the previous object instead of emitting - // undefined (lodash merge skips undefined and would keep the old value). - if (value == null || value === '') { - setTreeSelectValue(null) - onChange?.(null) - return - } - const selectedObject = objectList.find((obj) => - areValuesEqual(obj._id, value) - ) - setTreeSelectValue(value) - onChange?.(selectedObject ?? null) + return } + + if (nextValue == null || nextValue === '') { + setTreeSelectValue(null) + onChange?.(null) + return + } + + const selectedObject = findObjectById(nextValue) + setTreeSelectValue(nextValue) + if (selectedObject) { + onChange?.(selectedObject) + return + } + if (pendingId != null && areValuesEqual(pendingId, nextValue)) { + return + } + onChange?.(null) }, - [multiple, objectList, onChange] + [multiple, onChange, findObjectById] ) const onSearch = useCallback( @@ -831,8 +856,74 @@ const ObjectSelect = ({ treeDataRef.current = treeData }, [treeData]) + const prevValuesRef = useRef({ type, masterFilter }) + useEffect(() => { + const prevValues = prevValuesRef.current + + // Deep comparison for objects, simple comparison for primitives + const hasChanged = + prevValues.type !== type || + JSON.stringify(prevValues.masterFilter) !== JSON.stringify(masterFilter) + + if (hasChanged) { + loadGenerationRef.current += 1 + searchRequestIdRef.current += 1 + setIsSearching(false) + setSearchValue('') + setObjectPropertiesTree({}) + setObjectList([]) + setTreeData([]) + treeDataRef.current = [] + setTreeVersion((v) => v + 1) + setExpandedKeys([]) + setInitialized(false) + valueRef.current = null + setTreeSelectValue(null) + setInitialLoading(true) + setReloading(false) + setError(false) + setValueNotFound(false) + clearedMissingValueRef.current = false + prevValuesRef.current = { type, masterFilter } + setCommittedSelectKey(getSelectKey(type, masterFilter)) + } + }, [type, masterFilter, getSelectKey]) + + useEffect(() => { + const currentValueIdentity = getValueIdentity(value) + const hasValueChanged = + prevValueIdentityRef.current !== currentValueIdentity + + if (hasValueChanged) { + const changeSource = isInternalChangeRef.current ? 'internal' : 'external' + + if (changeSource == 'external') { + loadGenerationRef.current += 1 + setObjectPropertiesTree({}) + setTreeData([]) + treeDataRef.current = [] + setInitialized(false) + setInitialLoading(true) + valueRef.current = null + clearedMissingValueRef.current = false + setValueNotFound(false) + } + + isInternalChangeRef.current = false + prevValueRef.current = value + prevValueIdentityRef.current = currentValueIdentity + } + }, [value, getValueIdentity]) + + useEffect(() => { + if (getSelectKey(type, masterFilter) !== committedSelectKey) { + return + } + + const generation = loadGenerationRef.current const handleValue = async () => { + if (generation !== loadGenerationRef.current) return if ( multiple && Array.isArray(value) && @@ -841,12 +932,15 @@ const ObjectSelect = ({ ) { valueRef.current = value const fullValues = await Promise.all(value.map(fetchFullObjectIfNeeded)) + if (generation !== loadGenerationRef.current) return const pathKeys = [] if (fullValues.length === 0) { - handleFetchObjectsProperties() + const data = await handleFetchObjectsProperties() + if (generation !== loadGenerationRef.current) return + if (Array.isArray(data)) applyTreeFromData(data) } else { - fullValues.forEach((fullValue) => { + for (const fullValue of fullValues) { const valueFilter = { ...filter } const parentKeys = [] @@ -874,8 +968,10 @@ const ObjectSelect = ({ parentKeys.push(valueString) }) - handleFetchObjectsProperties(valueFilter) - }) + const data = await handleFetchObjectsProperties(valueFilter) + if (generation !== loadGenerationRef.current) return + if (Array.isArray(data)) applyTreeFromData(data) + } } setExpandedKeys([...new Set(pathKeys)]) @@ -895,9 +991,8 @@ const ObjectSelect = ({ type != 'unknown' ) { valueRef.current = value - // Check if value is a minimal object and fetch full object if needed const fullValue = await fetchFullObjectIfNeeded(value) - // Build a new filter from value's properties that are in the properties list + if (generation !== loadGenerationRef.current) return const valueFilter = { ...filter } const pathKeys = [] const parentKeys = [] @@ -922,7 +1017,6 @@ const ObjectSelect = ({ valueFilter[prop] = filterValue valueString = filterValue } - // Build the path key for this property level const nodeKey = parentKeys .concat(prop + ':' + valueString) .join('-') @@ -930,10 +1024,10 @@ const ObjectSelect = ({ parentKeys.push(valueString) } }) - // Expand the path to the object setExpandedKeys(pathKeys) - // Fetch with the new filter - handleFetchObjectsProperties(valueFilter) + const data = await handleFetchObjectsProperties(valueFilter) + if (generation !== loadGenerationRef.current) return + if (Array.isArray(data)) applyTreeFromData(data) setTreeSelectValue(toSelectValue(valueRef.current._id)) setInitialized(true) return @@ -959,13 +1053,7 @@ const ObjectSelect = ({ setInitialized(true) } } - const timeoutId = setTimeout(() => { - handleValue() - }, 10) - - return () => { - clearTimeout(timeoutId) - } + handleValue() }, [ value, filter, @@ -975,73 +1063,15 @@ const ObjectSelect = ({ token, fetchFullObjectIfNeeded, type, + masterFilter, + committedSelectKey, + getSelectKey, connected, getValueIdentity, - multiple + multiple, + applyTreeFromData ]) - const prevValuesRef = useRef({ type, masterFilter }) - - useEffect(() => { - const prevValues = prevValuesRef.current - - // Deep comparison for objects, simple comparison for primitives - const hasChanged = - prevValues.type !== type || - JSON.stringify(prevValues.masterFilter) !== JSON.stringify(masterFilter) - - if (hasChanged) { - searchRequestIdRef.current += 1 - setIsSearching(false) - setSearchValue('') - setObjectPropertiesTree({}) - setObjectList([]) - setTreeData([]) - setTreeVersion((v) => v + 1) - setExpandedKeys([]) - setInitialized(false) - valueRef.current = null - onTreeSelectChange(null) - setTreeSelectValue(null) - setInitialLoading(true) - setReloading(false) - setError(false) - setValueNotFound(false) - clearedMissingValueRef.current = false - prevValuesRef.current = { type, masterFilter } - } - }, [type, masterFilter, onTreeSelectChange]) - - useEffect(() => { - // Check if value has actually changed - const currentValueIdentity = getValueIdentity(value) - const hasValueChanged = - prevValueIdentityRef.current !== currentValueIdentity - - if (hasValueChanged) { - const changeSource = isInternalChangeRef.current ? 'internal' : 'external' - - if (changeSource == 'external') { - setObjectPropertiesTree({}) - setTreeData([]) - treeDataRef.current = [] - setInitialized(false) - setInitialLoading(true) - valueRef.current = null - clearedMissingValueRef.current = false - setValueNotFound(false) - prevValuesRef.current = { type, masterFilter } - } - - // Reset the internal change flag - isInternalChangeRef.current = false - - // Update the previous value reference - prevValueRef.current = value - prevValueIdentityRef.current = currentValueIdentity - } - }, [value, getValueIdentity, type, masterFilter]) - useEffect(() => { objectListRef.current = objectList }, [objectList]) @@ -1137,16 +1167,6 @@ const ObjectSelect = ({ if (missing) { setValueNotFound(true) - if ( - value != null && - !(Array.isArray(value) && value.length === 0) && - !clearedMissingValueRef.current - ) { - clearedMissingValueRef.current = true - isInternalChangeRef.current = true - setTreeSelectValue(multiple ? [] : null) - onChange?.(multiple ? [] : null) - } return } @@ -1154,7 +1174,7 @@ const ObjectSelect = ({ if (value != null) { setValueNotFound(false) } - }, [value, multiple, objectList, treeData, valueReady, onChange]) + }, [value, multiple, objectList, treeData, valueReady]) const displayPlaceholder = valueNotFound ? `${modelLabel.charAt(0).toUpperCase()}${modelLabel.slice(1).toLowerCase()} not found.`