Enhance ObjectTable component with improved data handling and deduplication
All checks were successful
farmcontrol/farmcontrol-ui/pipeline/head This commit looks good

- Introduced logic to filter out duplicate items in the table data, logging warnings for omitted rows.
- Added mechanisms for managing data load generation to prevent stale data issues during asynchronous operations.
- Refactored page loading and merging logic to ensure proper handling of loaded pages and skeleton states.
- Improved state management for loading and clearing table pages, enhancing overall performance and user experience.
This commit is contained in:
Tom Butcher 2026-08-09 21:07:57 +01:00
parent 78299f688e
commit e885949639

View File

@ -9,6 +9,7 @@ import {
useMemo,
createElement
} from 'react'
import { flushSync } from 'react-dom'
import {
Table,
Skeleton,
@ -200,10 +201,20 @@ const ObjectTable = forwardRef(
// Table state
const [pages, setPages] = useState([])
const pagesRef = useRef(pages)
const tableData = useMemo(
() => pages.flatMap((page) => page.items),
[pages]
)
const tableData = useMemo(() => {
const items = pages.flatMap((page) => page.items)
const seen = new Set()
return items.filter((item) => {
const id = item?._id
if (id == null) return true
if (seen.has(id)) {
logger.warn('Duplicate table row omitted:', id)
return false
}
seen.add(id)
return true
})
}, [pages])
const [loading, setLoading] = useState(true)
const [lazyLoading, setLazyLoading] = useState(false)
@ -261,6 +272,44 @@ const ObjectTable = forwardRef(
const loadingPagesRef = useRef(new Set())
const pendingScrollAnchorRef = useRef(null)
const dataLoadGenerationRef = useRef(0)
const [tableListKey, setTableListKey] = useState(0)
const isInternalSortUpdateRef = useRef(false)
const beginDataReload = useCallback(() => {
dataLoadGenerationRef.current += 1
loadingPagesRef.current.clear()
pendingScrollAnchorRef.current = null
pagesRef.current = []
setTableListKey((key) => key + 1)
return dataLoadGenerationRef.current
}, [])
const isStaleDataLoad = useCallback((generation) => {
return generation !== dataLoadGenerationRef.current
}, [])
const clearTablePages = useCallback(() => {
pagesRef.current = []
flushSync(() => {
setPages([])
})
}, [])
const setTablePages = useCallback((nextPages) => {
pagesRef.current = nextPages
setPages(nextPages)
}, [])
const mergeLoadedPage = useCallback((currentPages, loadedPage) => {
const withoutSkeletons = currentPages.filter((page) => !page.isSkeletonPage)
const withoutDuplicate = withoutSkeletons.filter(
(page) => page.pageNum !== loadedPage.pageNum
)
return [...withoutDuplicate, loadedPage].sort(
(a, b) => a.pageNum - b.pageNum
)
}, [])
const renderActions = (objectData) => {
return (
@ -358,7 +407,11 @@ const ObjectTable = forwardRef(
const createPageWindow = useCallback(
(loadedPages, direction) => {
const sortedPages = [...loadedPages].sort(
const dedupedByPageNum = new Map()
loadedPages.forEach((page) => {
dedupedByPageNum.set(page.pageNum, page)
})
const sortedPages = [...dedupedByPageNum.values()].sort(
(a, b) => a.pageNum - b.pageNum
)
const visiblePages =
@ -384,8 +437,10 @@ const ObjectTable = forwardRef(
const fetchData = useCallback(
async (pageNum = 1, filter = null, sorter = null) => {
const generation = dataLoadGenerationRef.current
try {
const result = await fetchPage(pageNum, filter, sorter)
if (isStaleDataLoad(generation)) return []
const loadedPage = {
pageNum,
items: result.data || [],
@ -397,11 +452,13 @@ const ObjectTable = forwardRef(
setLoading(false)
return result.data || []
} catch (error) {
setLoading(false)
if (!isStaleDataLoad(generation)) {
setLoading(false)
}
throw error
}
},
[fetchPage]
[fetchPage, isStaleDataLoad]
)
const findRenderedRow = useCallback((scrollTarget, id) => {
@ -496,12 +553,14 @@ const ObjectTable = forwardRef(
currentPages
)
const generation = dataLoadGenerationRef.current
loadingPagesRef.current.add(pageNum)
setLazyLoading(true)
logger.debug(`Loading ${direction} page...`)
try {
const result = await fetchPage(pageNum)
if (isStaleDataLoad(generation)) return
const loadedPage = {
pageNum,
items: result.data || [],
@ -522,21 +581,28 @@ const ObjectTable = forwardRef(
return prev
}
const loadedPages = prev
.filter((page) => !page.isSkeletonPage)
.concat(loadedPage)
const loadedPages = mergeLoadedPage(prev, loadedPage)
return createPageWindow(loadedPages, direction)
})
} catch (error) {
logger.error(`Error loading page ${pageNum}:`, error)
} finally {
loadingPagesRef.current.delete(pageNum)
if (loadingPagesRef.current.size === 0) {
if (
!isStaleDataLoad(generation) &&
loadingPagesRef.current.size === 0
) {
setLazyLoading(false)
}
}
},
[captureScrollAnchor, createPageWindow, fetchPage]
[
captureScrollAnchor,
createPageWindow,
fetchPage,
isStaleDataLoad,
mergeLoadedPage
]
)
const loadNextPage = useCallback(
@ -783,11 +849,20 @@ const ObjectTable = forwardRef(
const loadPage = useCallback(
async (pageNum, filter = null, sorter = null) => {
setPages([createSkeletonPage(pageNum)])
setLoading(true)
const generation = beginDataReload()
clearTablePages()
if (isStaleDataLoad(generation)) return
const skeletonPage = createSkeletonPage(pageNum)
flushSync(() => {
setTablePages([skeletonPage])
setLoading(true)
})
if (isStaleDataLoad(generation)) return
try {
const firstResult = await fetchPage(pageNum, filter, sorter)
if (isStaleDataLoad(generation)) return
const loadedPages = [
{
pageNum,
@ -796,25 +871,29 @@ const ObjectTable = forwardRef(
}
]
if (firstResult.hasMore) {
setPages([...loadedPages, createSkeletonPage(pageNum + 1)])
const secondResult = await fetchPage(pageNum + 1)
loadedPages.push({
pageNum: pageNum + 1,
items: secondResult.data || [],
hasMore: secondResult.hasMore
})
if (!isStaleDataLoad(generation)) {
setTablePages(createPageWindow(loadedPages, 'next'))
}
setPages(createPageWindow(loadedPages, 'next'))
} catch (error) {
logger.error(`Error loading page ${pageNum}:`, error)
setPages([])
if (!isStaleDataLoad(generation)) {
logger.error(`Error loading page ${pageNum}:`, error)
setTablePages([])
}
} finally {
setLoading(false)
if (!isStaleDataLoad(generation)) {
setLoading(false)
}
}
},
[createPageWindow, createSkeletonPage, fetchPage]
[
beginDataReload,
clearTablePages,
createPageWindow,
createSkeletonPage,
fetchPage,
isStaleDataLoad,
setTablePages
]
)
const loadInitialPage = useCallback(async () => {
@ -868,6 +947,8 @@ const ObjectTable = forwardRef(
JSON.stringify(prevValues.masterFilter) !== JSON.stringify(masterFilter)
if (hasChanged) {
beginDataReload()
pagesRef.current = []
setPages([])
activeFilterRef.current = {}
tableSorterRef.current = {}
@ -878,7 +959,7 @@ const ObjectTable = forwardRef(
setLazyLoading(false)
prevValuesRef.current = { type, masterFilter }
}
}, [type, masterFilter])
}, [type, masterFilter, beginDataReload])
useEffect(() => {
registerPageFilter(sidebarFilter)
@ -922,6 +1003,8 @@ const ObjectTable = forwardRef(
}
const handleTableChange = (pagination, filters, sorter) => {
if (isInternalSortUpdateRef.current) return
const next = { ...sidebarFilter }
Object.entries(filters).forEach(([key, value]) => {
@ -940,8 +1023,6 @@ const ObjectTable = forwardRef(
setSidebarFilter(next)
setTableSorter(nextSorter)
persistTableState(next, nextSorter)
setPages([])
setLoading(true)
loadPage(initialPage, getActiveFilterValues(next), nextSorter)
}
@ -949,8 +1030,6 @@ const ObjectTable = forwardRef(
(newSidebarFilter) => {
setSidebarFilter(newSidebarFilter)
persistFilter(newSidebarFilter)
setPages([])
setLoading(true)
loadPage(
initialPage,
getActiveFilterValues(newSidebarFilter),
@ -965,16 +1044,18 @@ const ObjectTable = forwardRef(
const nextSorter = newSorter?.field
? { field: newSorter.field, order: newSorter.order }
: {}
isInternalSortUpdateRef.current = true
setTableSorter(nextSorter)
tableSorterRef.current = nextSorter
persistSort(nextSorter)
setPages([])
setLoading(true)
loadPage(
initialPage,
getActiveFilterValues(sidebarFilter),
nextSorter.field ? nextSorter : null
)
requestAnimationFrame(() => {
isInternalSortUpdateRef.current = false
})
},
[initialPage, loadPage, persistSort, sidebarFilter]
)
@ -1235,7 +1316,7 @@ const ObjectTable = forwardRef(
}}
>
<div className='objectTableCards'>
<Row gutter={[16, 16]} key={cardColSpan}>
<Row gutter={[16, 16]} key={`${cardColSpan}-${tableListKey}`}>
{tableData.map((record) => {
if (record?._id == undefined) {
return null
@ -1327,6 +1408,7 @@ const ObjectTable = forwardRef(
</div>
) : (
<Table
key={tableListKey}
ref={tableRef}
dataSource={tableData}
columns={columnsWithSkeleton}