Skip to content

Commit

Permalink
fix: random app crash on wallet page (#7585)
Browse files Browse the repository at this point in the history
* fix: infinite render and crash when clicking accounts

* perf: better memoization for react table component

---------

Co-authored-by: Apotheosis <[email protected]>
  • Loading branch information
woodenfurniture and 0xApotheosis authored Aug 21, 2024
1 parent 95f1315 commit 639ede6
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,12 @@ export const LpPositionsByProvider: React.FC<LpPositionsByProviderProps> = ({ id
const marketDataUserCurrency = useAppSelector(selectMarketDataUserCurrency)
const lpOpportunities = useAppSelector(selectAggregatedEarnUserLpOpportunities)

const filteredDown = lpOpportunities.filter(
e => ids.includes(e.assetId as OpportunityId) || ids.includes(e.id as OpportunityId),
const filteredDown = useMemo(
() =>
lpOpportunities.filter(
e => ids.includes(e.assetId as OpportunityId) || ids.includes(e.id as OpportunityId),
),
[ids, lpOpportunities],
)

const handleClick = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,12 @@ export const StakingPositionsByProvider: React.FC<StakingPositionsByProviderProp
const stakingOpportunities = useAppSelector(
selectAggregatedEarnUserStakingOpportunitiesIncludeEmpty,
)
const filteredDown = stakingOpportunities.filter(
e => ids.includes(e.assetId as OpportunityId) || ids.includes(e.id as OpportunityId),
const filteredDown = useMemo(
() =>
stakingOpportunities.filter(
e => ids.includes(e.assetId as OpportunityId) || ids.includes(e.id as OpportunityId),
),
[ids, stakingOpportunities],
)

const handleClick = useCallback(
Expand Down
170 changes: 109 additions & 61 deletions src/components/ReactTable/ReactTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ import {
useColorModeValue,
} from '@chakra-ui/react'
import type { ReactNode } from 'react'
import { Fragment, useMemo, useRef } from 'react'
import { Fragment, useCallback, useMemo, useRef } from 'react'
import { useTranslate } from 'react-polyglot'
import type { Column, Row, TableState } from 'react-table'
import type { Cell, Column, ColumnInstance, Row, TableState } from 'react-table'
import { useExpanded, usePagination, useSortBy, useTable } from 'react-table'
import { RawText } from 'components/Text'

Expand All @@ -27,7 +27,7 @@ type ReactTableProps<T extends {}> = {
rowDataTestKey?: keyof T
rowDataTestPrefix?: string
onRowClick?: (row: Row<T>) => void
initialState?: Partial<TableState<{}>>
initialState?: Partial<TableState<T>>
renderSubComponent?: (row: Row<T>) => ReactNode
renderEmptyComponent?: () => ReactNode
isLoading?: boolean
Expand All @@ -41,6 +41,79 @@ const tableSize = { base: 'sm', md: 'md' }
const arrowBackIcon = <ArrowBackIcon />
const arrowForwardIcon = <ArrowForwardIcon />

const CellWrap = <T extends {}>({ cell }: { cell: Cell<T, unknown> }) => {
const cellProps = useMemo(() => cell.getCellProps(), [cell])
const dataLabel = useMemo(() => {
return typeof cell.column.Header === 'string' ? cell.column.Header : undefined
}, [cell.column.Header])

return (
<Td
{...cellProps}
data-label={dataLabel}
display={cell.column.display}
textAlign={cell.column.textAlign}
key={cell.column.id}
>
{cell.render('Cell')}
</Td>
)
}

const RowWrap = <T extends {}>({
row,
rowDataTestKey,
rowDataTestPrefix,
onRowClick,
renderSubComponent,
visibleColumns,
}: {
row: Row<T>
rowDataTestKey: string | number | symbol | undefined
rowDataTestPrefix?: string
onRowClick?: (row: Row<T>) => void
renderSubComponent?: (row: Row<T>) => React.ReactNode
visibleColumns: ColumnInstance<T>[]
}) => {
const handleClick = useCallback(() => {
onRowClick?.(row)
}, [onRowClick, row])

const rowProps = useMemo(() => row.getRowProps(), [row])

const dataTest = useMemo(() => {
if (!rowDataTestKey) return undefined
const value =
(row.original as Record<string, unknown> | undefined)?.[rowDataTestKey as string] ?? ''
return `${rowDataTestPrefix}-${String(value).split(' ').join('-').toLowerCase()}`
}, [row.original, rowDataTestKey, rowDataTestPrefix])

return (
<Fragment>
<Tr
{...rowProps}
key={row.id}
tabIndex={row.index}
onClick={handleClick}
className={row.isExpanded ? 'expanded' : ''}
data-test={dataTest}
cursor={onRowClick ? 'pointer' : undefined}
>
{row.cells.map(cell => (
<CellWrap key={cell.column.id} cell={cell} />
))}
</Tr>
{!!renderSubComponent && row.isExpanded ? (
<Tr className='expanded-details'>
<Td colSpan={visibleColumns.length} style={tdStyle}>
{renderSubComponent(row)}
</Td>
</Tr>
) : null}
</Fragment>
)
}

export const ReactTable = <T extends {}>({
columns,
data,
Expand All @@ -57,16 +130,15 @@ export const ReactTable = <T extends {}>({
const translate = useTranslate()
const tableRef = useRef<HTMLTableElement | null>(null)
const hoverColor = useColorModeValue('black', 'white')
const tableColumns = useMemo(
() =>
isLoading
? columns.map((column, index) => ({
...column,
Cell: () => <Skeleton key={index} height='16px' />,
}))
: columns,
[columns, isLoading],
)
const tableColumns = useMemo(() => {
return isLoading
? columns.map((column, index) => ({
...column,
Cell: () => <Skeleton key={index} height='16px' />,
}))
: columns
}, [columns, isLoading])

const {
getTableProps,
getTableBodyProps,
Expand All @@ -85,57 +157,29 @@ export const ReactTable = <T extends {}>({
columns: tableColumns,
data,
initialState,
// The following two options are needed to prevent the table from infinite render loop and crash
// https://github.com/TanStack/table/issues/2369#issuecomment-644481605
autoResetSortBy: false,
autoResetPage: false,
},
useSortBy,
useExpanded,
usePagination,
)
const renderRows = useMemo(() => {

const renderedRows = useMemo(() => {
return page.map(row => {
prepareRow(row)
return (
<Fragment key={row.id}>
<Tr
{...row.getRowProps()}
key={row.id}
tabIndex={row.index}
// we need to pass an arg here, so we need an anonymous function wrapper
// eslint-disable-next-line react-memo/require-usememo
onClick={() => onRowClick?.(row)}
className={row.isExpanded ? 'expanded' : ''}
{...(rowDataTestKey
? {
'data-test': `${rowDataTestPrefix}-${String(row.original?.[rowDataTestKey] ?? '')
.split(' ')
.join('-')
.toLowerCase()}`,
}
: {})}
cursor={onRowClick ? 'pointer' : undefined}
>
{row.cells.map(cell => (
<Td
{...cell.getCellProps()}
// Header can be () => null or a string, only use data-label if it's a string
{...(typeof cell.column.Header === 'string'
? { 'data-label': cell.column.Header }
: undefined)}
display={cell.column.display}
textAlign={cell.column.textAlign}
key={cell.column.id}
>
{cell.render('Cell')}
</Td>
))}
</Tr>
{!!renderSubComponent && row.isExpanded ? (
<Tr className='expanded-details'>
<Td colSpan={visibleColumns.length} style={tdStyle}>
{renderSubComponent(row)}
</Td>
</Tr>
) : null}
</Fragment>
<RowWrap
key={row.id}
row={row}
rowDataTestKey={rowDataTestKey}
rowDataTestPrefix={rowDataTestPrefix}
onRowClick={onRowClick}
renderSubComponent={renderSubComponent}
visibleColumns={visibleColumns}
/>
)
})
}, [
Expand All @@ -145,11 +189,15 @@ export const ReactTable = <T extends {}>({
rowDataTestPrefix,
onRowClick,
renderSubComponent,
visibleColumns.length,
visibleColumns,
])

const tableBodyProps = useMemo(() => getTableBodyProps(), [getTableBodyProps])
const tableProps = useMemo(() => getTableProps(), [getTableProps])
const emptyComponent = useMemo(() => renderEmptyComponent?.(), [renderEmptyComponent])

return (
<Table ref={tableRef} variant={variant} size={tableSize} {...getTableProps()}>
<Table ref={tableRef} variant={variant} size={tableSize} {...tableProps}>
{displayHeaders && (
<Thead>
{headerGroups.map(headerGroup => (
Expand Down Expand Up @@ -182,12 +230,12 @@ export const ReactTable = <T extends {}>({
))}
</Thead>
)}
<Tbody {...getTableBodyProps()}>{renderRows}</Tbody>
{page.length === 0 && !isLoading && renderEmptyComponent && (
<Tbody {...tableBodyProps}>{renderedRows}</Tbody>
{page.length === 0 && !isLoading && emptyComponent && (
<Tfoot>
<Tr>
<Td colSpan={visibleColumns.length} py={0}>
{renderEmptyComponent()}
{emptyComponent}
</Td>
</Tr>
</Tfoot>
Expand Down
7 changes: 4 additions & 3 deletions src/components/StakingVaults/StakingTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { bnOrZero } from 'lib/bignumber/bignumber'
import type { EarnOpportunityType } from 'state/slices/opportunitiesSlice/types'
import { DefiType } from 'state/slices/opportunitiesSlice/types'
import { makeDefiProviderDisplayName } from 'state/slices/opportunitiesSlice/utils'
import { store } from 'state/store'
import { selectAssets } from 'state/slices/selectors'
import { useAppSelector } from 'state/store'

import { AssetCell } from './Cells'

Expand All @@ -25,6 +26,7 @@ const tagSize = { base: 'sm', md: 'md' }

export const StakingTable = ({ data, onClick, showTeaser }: StakingTableProps) => {
const translate = useTranslate()
const assets = useAppSelector(selectAssets)
const columns: Column<EarnOpportunityType>[] = useMemo(
() => [
{
Expand Down Expand Up @@ -57,7 +59,6 @@ export const StakingTable = ({ data, onClick, showTeaser }: StakingTableProps) =
accessor: 'provider',
display: { base: 'none', lg: 'table-cell' },
Cell: ({ value, row }: { value: string; row: RowProps }) => {
const assets = store.getState().assets.byId
const asset = assets[row.original.assetId]
const assetName = asset?.name ?? ''
const providerDisplayName = makeDefiProviderDisplayName({
Expand Down Expand Up @@ -133,7 +134,7 @@ export const StakingTable = ({ data, onClick, showTeaser }: StakingTableProps) =
),
},
],
[showTeaser, translate],
[assets, showTeaser, translate],
)

const handleRowClick = useCallback(
Expand Down

0 comments on commit 639ede6

Please sign in to comment.