Repository: gregnb/mui-datatables
Pull request #997 (2019-10-11 02:52:59)

Stop mutating column.options props and support immutable data object
I notice that the `MUIDataTable` is mutating the props.columns.options value and it causes error when passing an immutable object to it. 

Example, I'm using `immer` to update state and it produces immutable data object (using deep-freeze under the hood).

I think it is better to not mutate the props values inside the component.

**Change**
- Replace code that mutate `props.columns.options` by spreading `column.options` into new options before mutating

**Example**
https://codesandbox.io/s/dazzling-dijkstra-kmuf2


coveralls: 
[![Coverage Status](https://coveralls.io/builds/26251770/badge)](https://coveralls.io/builds/26251770)

Coverage decreased (-0.08%) to 77.279% when pulling **316690fb1ea3c49cb4ccbc88872ee35eaf249f7f on n3tr:stop-mutating-column-props** into **b8bee522b0e554522daa34d4cb3a41ffe95bbe03 on gregnb:master**.

n3tr: @gabrielliwerant Fixed :)

Sorry, I forgot to run test before push the change. Didn't realize it breaks a test case.
gabrielliwerant: Looks like you accidentally push your local yarn lockfile with the last commit! I noticed when the line changes jumped by about 9k. :)
n3tr: @gabrielliwerant removed 😅 
gabrielliwerant: @n3tr Hm, I guess that wouldn't hurt. Could also take it out if we switched to yarn in the future. I'll leave that up to you.
n3tr: @gabrielliwerant I will open separate PR for removing `yarn.lock`.

Do you know why TravisCI doesn't send the build status here? 
gabrielliwerant: @n3tr Not sure. I actually don't have access to the CI administration for this library. My guess is that it just wasn't set up.

Patch:
diff --git a/src/MUIDataTable.js b/src/MUIDataTable.js
index ecae4a8..5aa94a5 100644
--- a/src/MUIDataTable.js
+++ b/src/MUIDataTable.js
@@ -17,7 +17,7 @@ import TableResize from './components/TableResize';
 import TableToolbar from './components/TableToolbar';
 import TableToolbarSelect from './components/TableToolbarSelect';
 import textLabels from './textLabels';
-import { buildMap, getCollatorComparator, sortCompare } from './utils';
+import { buildMap, getCollatorComparator, sortCompare, getPageValue } from './utils';
 
 const defaultTableStyles = theme => ({
   root: {},
@@ -409,22 +409,23 @@ class MUIDataTable extends React.Component {
       };
 
       if (typeof column === 'object') {
-        if (column.options) {
-          if (column.options.display !== undefined) {
-            column.options.display = column.options.display.toString();
+        const options = { ...column.options };
+        if (options) {
+          if (options.display !== undefined) {
+            options.display = options.display.toString();
           }
 
-          if (column.options.sortDirection === null) {
+          if (options.sortDirection === null) {
             console.error(
               'The "null" option for sortDirection is deprecated. sortDirection is an enum, use "asc" | "desc" | "none"',
             );
-            column.options.sortDirection = 'none';
+            options.sortDirection = 'none';
           }
 
-          if (column.options.sortDirection !== undefined && column.options.sortDirection !== 'none') {
+          if (options.sortDirection !== undefined && options.sortDirection !== 'none') {
             if (sortDirectionSet) {
               console.error('sortDirection is set for more than one column. Only the first column will be considered.');
-              column.options.sortDirection = 'none';
+              options.sortDirection = 'none';
             } else {
               sortDirectionSet = true;
             }
@@ -435,7 +436,7 @@ class MUIDataTable extends React.Component {
           name: column.name,
           label: column.label ? column.label : column.name,
           ...columnOptions,
-          ...(column.options ? column.options : {}),
+          ...options,
         };
       } else {
         columnOptions = { ...columnOptions, name: column, label: column };
@@ -886,20 +887,16 @@ class MUIDataTable extends React.Component {
   };
 
   changeRowsPerPage = rows => {
-    /**
-     * After changing rows per page recalculate totalPages and checks its if current page not higher.
-     * Otherwise sets current page the value of nextTotalPages
-     */
     const rowCount = this.options.count || this.state.displayData.length;
-    const nextTotalPages = Math.floor(rowCount / rows);
 
     this.setState(
       () => ({
         rowsPerPage: rows,
-        page: this.state.page > nextTotalPages ? nextTotalPages : this.state.page,
+        page: getPageValue(rowCount, rows, this.state.page),
       }),
       () => {
         this.setTableAction('changeRowsPerPage');
+
         if (this.options.onChangeRowsPerPage) {
           this.options.onChangeRowsPerPage(this.state.rowsPerPage);
         }
diff --git a/src/components/TableBody.js b/src/components/TableBody.js
index a8ed200..a55558e 100644
--- a/src/components/TableBody.js
+++ b/src/components/TableBody.js
@@ -7,6 +7,7 @@ import TableBodyRow from './TableBodyRow';
 import TableSelectCell from './TableSelectCell';
 import { withStyles } from '@material-ui/core/styles';
 import cloneDeep from 'lodash.clonedeep';
+import { getPageValue } from '../utils';
 
 const defaultBodyStyles = {
   root: {},
@@ -55,12 +56,12 @@ class TableBody extends React.Component {
     if (this.props.options.serverSide) return data.length ? data : null;
 
     let rows = [];
-    const totalPages = Math.floor(count / rowsPerPage);
-    const fromIndex = page === 0 ? 0 : page * rowsPerPage;
-    const toIndex = Math.min(count, (page + 1) * rowsPerPage);
+    const highestPageInRange = getPageValue(count, rowsPerPage, page);
+    const fromIndex = highestPageInRange === 0 ? 0 : highestPageInRange * rowsPerPage;
+    const toIndex = Math.min(count, (highestPageInRange + 1) * rowsPerPage);
 
-    if (page > totalPages && totalPages !== 0) {
-      console.warn('Current page is out of range.');
+    if (page > highestPageInRange) {
+      console.warn('Current page is out of range, using the highest page that is in range instead.');
     }
 
     for (let rowIndex = fromIndex; rowIndex < count && rowIndex < toIndex; rowIndex++) {
diff --git a/src/components/TablePagination.js b/src/components/TablePagination.js
index cc20ea1..e091164 100644
--- a/src/components/TablePagination.js
+++ b/src/components/TablePagination.js
@@ -4,6 +4,7 @@ import MuiTableRow from '@material-ui/core/TableRow';
 import MuiTableFooter from '@material-ui/core/TableFooter';
 import MuiTablePagination from '@material-ui/core/TablePagination';
 import { withStyles } from '@material-ui/core/styles';
+import { getPageValue } from '../utils';
 
 const defaultPaginationStyles = {
   root: {
@@ -63,7 +64,7 @@ class TablePagination extends React.Component {
             }}
             count={count}
             rowsPerPage={rowsPerPage}
-            page={page}
+            page={getPageValue(count, rowsPerPage, page)}
             labelRowsPerPage={textLabels.rowsPerPage}
             labelDisplayedRows={({ from, to, count }) => `${from}-${to} ${textLabels.displayRows} ${count}`}
             backIconButtonProps={{
diff --git a/src/utils.js b/src/utils.js
index e376eb1..0e8fcb6 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -5,6 +5,12 @@ function buildMap(rows) {
   }, {});
 }
 
+function getPageValue(count, rowsPerPage, page) {
+  const totalPages = count === rowsPerPage ? 0 : Math.floor(count / rowsPerPage);
+
+  return page > totalPages ? totalPages : page;
+}
+
 function getCollatorComparator() {
   if (!!Intl) {
     const collator = new Intl.Collator(undefined, { numeric: true, sensitivity: 'base' });
@@ -46,6 +52,7 @@ function createCSVDownload(columns, data, options) {
   const CSVHead = buildHead(columns);
 
   const buildBody = data => {
+    if (!data.length) return '';
     return data
       .reduce(
         (soFar, row) =>
@@ -90,4 +97,4 @@ function createCSVDownload(columns, data, options) {
   }
 }
 
-export { buildMap, getCollatorComparator, sortCompare, createCSVDownload };
+export { buildMap, getPageValue, getCollatorComparator, sortCompare, createCSVDownload };


Test patch:
diff --git a/test/MUIDataTablePagination.test.js b/test/MUIDataTablePagination.test.js
index a5a6e9b..71719c1 100644
--- a/test/MUIDataTablePagination.test.js
+++ b/test/MUIDataTablePagination.test.js
@@ -46,4 +46,13 @@ describe('<TablePagination />', function() {
     instance.handlePageChange(null, 1);
     assert.strictEqual(changePage.callCount, 1);
   });
+
+  it('should correctly change page to be in bounds if out of bounds page was set', () => {
+    // Set a page that is too high for the count and rowsPerPage
+    const mountWrapper = mount(<TablePagination options={options} count={5} page={1} rowsPerPage={10} />);
+    const actualResult = mountWrapper.find(MuiTablePagination).props().page;
+
+    // material-ui v3 does some internal calculations to protect against out of bounds pages, but material v4 does not
+    assert.strictEqual(actualResult, 0);
+  });
 });