Skip to content

Commit

Permalink
Merge pull request #940 from jbetancur/fix/930
Browse files Browse the repository at this point in the history 
fix infinite loop edge case
  • Loading branch information
@jbetancur
jbetancur authored Oct 10, 2021
2 parents 6013414 + fae9196 commit 260a438
Show file tree
Hide file tree
Showing 16 changed files with 751 additions and 764 deletions.
18 changes: 9 additions & 9 deletions package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-data-table-component",
"version": "7.4.1",
"version": "7.4.2",
"description": "A simple to use declarative react based data table",
"main": "dist/index.cjs.js",
"module": "dist/index.es.js",
Expand Down Expand Up @@ -42,12 +42,12 @@
"devDependencies": {
"@material-ui/core": "^4.12.3",
"@material-ui/icons": "^4.11.2",
"@storybook/addon-a11y": "^6.3.9",
"@storybook/addon-essentials": "^6.3.9",
"@storybook/addon-storysource": "^6.3.9",
"@storybook/addons": "^6.3.9",
"@storybook/react": "^6.3.9",
"@storybook/theming": "^6.3.9",
"@storybook/addon-a11y": "^6.3.10",
"@storybook/addon-essentials": "^6.3.10",
"@storybook/addon-storysource": "^6.3.10",
"@storybook/addons": "^6.3.10",
"@storybook/react": "^6.3.10",
"@storybook/theming": "^6.3.10",
"@testing-library/react": "^12.1.2",
"@types/faker": "^5.5.8",
"@types/jest": "^27.0.2",
Expand All @@ -56,7 +56,7 @@
"@types/node": "^16.10.3",
"@types/react": "^17.0.27",
"@types/react-dom": "^17.0.9",
"@types/styled-components": "^5.1.14",
"@types/styled-components": "^5.1.15",
"@typescript-eslint/eslint-plugin": "^4.33.0",
"@typescript-eslint/parser": "^4.33.0",
"axios": "^0.21.4",
Expand All @@ -73,7 +73,7 @@
"eslint-plugin-react-hooks": "^4.2.0",
"faker": "^5.5.3",
"gh-pages": "^3.2.3",
"jest": "^27.2.4",
"jest": "^27.2.5",
"jest-styled-components": "^7.0.5",
"jest-watch-typeahead": "^0.6.4",
"lodash-es": "^4.17.21",
Expand Down
77 changes: 48 additions & 29 deletions src/DataTable/DataTable.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,19 @@ import { CellBase } from './Cell';
import NoData from './NoDataWrapper';
import NativePagination from './Pagination';
import useDidUpdateEffect from '../hooks/useDidUpdateEffect';
import { prop, getNumberOfPages, setRowData, isEmpty, isRowSelected, recalculatePage } from './util';
import { prop, getNumberOfPages, sort, isEmpty, isRowSelected, recalculatePage } from './util';
import { defaultProps } from './defaultProps';
import { createStyles } from './styles';
import { Action, AllRowsAction, SingleRowAction, TableRow, SortAction, TableProps, TableState } from './types';
import {
Action,
AllRowsAction,
SingleRowAction,
TableRow,
SortAction,
TableProps,
TableState,
SortOrder,
} from './types';
import useColumns from '../hooks/useColumns';

function DataTable<T>(props: TableProps<T>): JSX.Element {
Expand Down Expand Up @@ -121,7 +130,6 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
const [
{
rowsPerPage,
rows,
currentPage,
selectedRows,
allSelected,
Expand All @@ -132,7 +140,6 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
},
dispatch,
] = React.useReducer<React.Reducer<TableState<T>, Action<T>>>(tableReducer, {
rows: setRowData(data, defaultSortColumn?.selector, defaultSortDirection, sortServer, sortFunction),
allSelected: false,
selectedCount: 0,
selectedRows: [],
Expand All @@ -153,17 +160,33 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
const currentTheme = React.useMemo(() => createStyles(customStyles, theme), [customStyles, theme]);
const wrapperProps = React.useMemo(() => ({ ...(direction !== 'auto' && { dir: direction }) }), [direction]);

const sortedData = React.useMemo(() => {
// server-side sorting bypasses internal sorting
if (sortServer) {
return data;
}

if (selectedColumn?.sortFunction && typeof selectedColumn.sortFunction === 'function') {
const sortFn = selectedColumn.sortFunction;
const customSortFunction = sortDirection === SortOrder.ASC ? sortFn : (a: T, b: T) => sortFn(a, b) * -1;

return [...data].sort(customSortFunction);
}

return sort(data, selectedColumn?.selector, sortDirection, sortFunction);
}, [sortServer, selectedColumn, sortDirection, data, sortFunction]);

const tableRows = React.useMemo(() => {
if (pagination && !paginationServer) {
// when using client-side pagination we can just slice the rows set
const lastIndex = currentPage * rowsPerPage;
const firstIndex = lastIndex - rowsPerPage;

return rows.slice(firstIndex, lastIndex);
return sortedData.slice(firstIndex, lastIndex);
}

return rows;
}, [currentPage, pagination, paginationServer, rows, rowsPerPage]);
return sortedData;
}, [currentPage, pagination, paginationServer, rowsPerPage, sortedData]);

const handleSort = React.useCallback((action: SortAction<T>) => {
dispatch(action);
Expand Down Expand Up @@ -219,7 +242,7 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
return true;
}

return rows.length > 0 && !progressPending;
return sortedData.length > 0 && !progressPending;
};

const showHeader = () => {
Expand All @@ -239,8 +262,8 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
};

// recalculate the pagination and currentPage if the rows length changes
if (pagination && !paginationServer && rows.length > 0 && tableRows.length === 0) {
const updatedPage = getNumberOfPages(rows.length, rowsPerPage);
if (pagination && !paginationServer && sortedData.length > 0 && tableRows.length === 0) {
const updatedPage = getNumberOfPages(sortedData.length, rowsPerPage);
const recalculatedPage = recalculatePage(currentPage, updatedPage);

handleChangePage(recalculatedPage);
Expand All @@ -256,7 +279,7 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
}, [selectedColumn, sortDirection]);

useDidUpdateEffect(() => {
onChangePage(currentPage, paginationTotalRows || rows.length);
onChangePage(currentPage, paginationTotalRows || sortedData.length);
}, [currentPage]);

useDidUpdateEffect(() => {
Expand All @@ -278,29 +301,27 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
}
}, [paginationTotalRows]);

// handle updating data and persisting sort state when data changes after initial re-render
useDidUpdateEffect(() => {
dispatch({
type: 'UPDATE_ROWS',
rows: setRowData(data, selectedColumn?.selector, sortDirection, sortServer, sortFunction),
});
}, [data]);

React.useEffect(() => {
dispatch({ type: 'CLEAR_SELECTED_ROWS', selectedRowsFlag: clearSelectedRows });
}, [selectableRowsSingle, clearSelectedRows]);

React.useEffect(() => {
if (selectableRowSelected && !selectableRowsSingle) {
const preSelectedRows = rows.filter(row => selectableRowSelected(row));
const preSelectedRows = sortedData.filter(row => selectableRowSelected(row));

dispatch({ type: 'SELECT_MULTIPLE_ROWS', keyField, selectedRows: preSelectedRows, rows: rows, mergeSelections });
dispatch({
type: 'SELECT_MULTIPLE_ROWS',
keyField,
selectedRows: preSelectedRows,
totalRows: sortedData.length,
mergeSelections,
});
}
// We only want to update the selectedRowState if data changes
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [data]);

const visibleRows = selectableRowsVisibleOnly ? tableRows : rows;
const visibleRows = selectableRowsVisibleOnly ? tableRows : sortedData;
const showSelectAll = persistSelectedOnPageChange || selectableRowsSingle || selectableRowsNoSelectAll;

return (
Expand Down Expand Up @@ -359,13 +380,11 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
key={column.id}
column={column}
selectedColumn={selectedColumn}
disabled={progressPending || rows.length === 0}
rows={rows}
disabled={progressPending || sortedData.length === 0}
pagination={pagination}
paginationServer={paginationServer}
persistSelectedOnSort={persistSelectedOnSort}
selectableRowsVisibleOnly={selectableRowsVisibleOnly}
sortFunction={sortFunction}
sortDirection={sortDirection}
sortIcon={sortIcon}
sortServer={sortServer}
Expand All @@ -382,11 +401,11 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
</Head>
)}

{!rows.length && !progressPending && <NoData>{noDataComponent}</NoData>}
{!sortedData.length && !progressPending && <NoData>{noDataComponent}</NoData>}

{progressPending && persistTableHead && <ProgressWrapper>{progressComponent}</ProgressWrapper>}

{!progressPending && rows.length > 0 && (
{!progressPending && sortedData.length > 0 && (
<Body className="rdt_TableBody" role="rowgroup">
{tableRows.map((row, i) => {
const key = prop(row as TableRow, keyField) as string | number;
Expand All @@ -403,7 +422,7 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
data-row-id={id}
columns={tableColumns}
row={row}
rowCount={rows.length}
rowCount={sortedData.length}
rowIndex={i}
selectableRows={selectableRows}
expandableRows={expandableRows}
Expand Down Expand Up @@ -451,7 +470,7 @@ function DataTable<T>(props: TableProps<T>): JSX.Element {
<Pagination
onChangePage={handleChangePage}
onChangeRowsPerPage={handleChangeRowsPerPage}
rowCount={paginationTotalRows || rows.length}
rowCount={paginationTotalRows || sortedData.length}
currentPage={currentPage}
rowsPerPage={rowsPerPage}
direction={direction}
Expand Down
28 changes: 4 additions & 24 deletions src/DataTable/TableCol.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import * as React from 'react';
import styled, { css } from 'styled-components';
import { CellExtended, CellProps } from './Cell';
import NativeSortIcon from '../icons/NativeSortIcon';
import { equalizeId, sort } from './util';
import { TableColumn, SortAction, SortDirection, SortFunction } from './types';
import { equalizeId } from './util';
import { TableColumn, SortAction, SortOrder } from './types';

interface ColumnStyleProps extends CellProps {
isDragging?: boolean;
Expand Down Expand Up @@ -81,7 +81,6 @@ const ColumnText = styled.div`
`;

type TableColProps<T> = {
rows: T[];
column: TableColumn<T>;
disabled: boolean;
draggingColumnId?: string | number;
Expand All @@ -90,8 +89,7 @@ type TableColProps<T> = {
paginationServer: boolean;
persistSelectedOnSort: boolean;
selectedColumn: TableColumn<T>;
sortDirection: SortDirection;
sortFunction: SortFunction<T> | null;
sortDirection: SortOrder;
sortServer: boolean;
selectableRowsVisibleOnly: boolean;
onSort: (action: SortAction<T>) => void;
Expand All @@ -103,13 +101,11 @@ type TableColProps<T> = {
};

function TableCol<T>({
rows,
column,
disabled,
draggingColumnId,
selectedColumn = {},
sortDirection,
sortFunction,
sortIcon,
sortServer,
pagination,
Expand Down Expand Up @@ -153,27 +149,11 @@ function TableCol<T>({
let direction = sortDirection;

if (equalizeId(selectedColumn.id, column.id)) {
direction = sortDirection === 'asc' ? 'desc' : 'asc';
}

let sortedRows = [...rows];

if (!sortServer) {
sortedRows = sort(rows, column.selector, direction, sortFunction);

// colCustomSortFn is still checked for undefined or null
const colCustomSortFn = column.sortFunction;

if (colCustomSortFn) {
const customSortFunction = direction === 'asc' ? colCustomSortFn : (a: T, b: T) => colCustomSortFn(a, b) * -1;

sortedRows = [...rows].sort(customSortFunction);
}
direction = sortDirection === SortOrder.ASC ? SortOrder.DESC : SortOrder.ASC;
}

onSort({
type: 'SORT_CHANGE',
rows: sortedRows,
sortDirection: direction,
selectedColumn: column,
clearSelectedOnSort:
Expand Down
8 changes: 4 additions & 4 deletions src/DataTable/__tests__/DataTable.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { render, fireEvent } from '@testing-library/react';
import DataTable from '../DataTable';
import { Direction, STOP_PROP_TAG } from '../constants';
import { Alignment } from '../../index';
import { ConditionalStyles } from '../types';
import { ConditionalStyles, SortOrder } from '../types';

interface Data {
id: number;
Expand Down Expand Up @@ -654,7 +654,7 @@ describe('DataTable::sorting', () => {

fireEvent.click(container.querySelector('div[data-sort-id="1"]') as HTMLElement);

expect(onSortMock).toBeCalledWith({ id: 1, ...mock.columns[0] }, 'asc');
expect(onSortMock).toBeCalledWith({ id: 1, ...mock.columns[0] }, SortOrder.ASC);
});

test('should call onSort with the correct params if the sort is clicked twice', () => {
Expand All @@ -663,10 +663,10 @@ describe('DataTable::sorting', () => {
const { container } = render(<DataTable data={mock.data} columns={mock.columns} onSort={onSortMock} />);

fireEvent.click(container.querySelector('div[data-sort-id="1"]') as HTMLElement);
expect(onSortMock).toBeCalledWith({ id: 1, ...mock.columns[0] }, 'asc');
expect(onSortMock).toBeCalledWith({ id: 1, ...mock.columns[0] }, SortOrder.ASC);

fireEvent.click(container.querySelector('div[data-sort-id="1"]') as HTMLElement);
expect(onSortMock).toBeCalledWith({ id: 1, ...mock.columns[0] }, 'desc');
expect(onSortMock).toBeCalledWith({ id: 1, ...mock.columns[0] }, SortOrder.DESC);
});

test('should render correctly with a custom sortIcon', () => {
Expand Down
Loading

0 comments on commit 260a438

Please sign in to comment.