fix(Changing data in rows passed into useTable does not update rows): (#16)

Fix bug causing updated data not updating rows due to improper memoization

Author: Buuntu

src/hooks.tsx

@@ -20,6 +20,12 @@ const createReducer = <T extends DataType>() => (
   action: TableAction<T>
 ): TableState<T> => {
   switch (action.type) {
+    case 'SET_ROWS':
+      return {
+        ...state,
+        rows: action.data,
+        originalRows: action.data,
+      };
     case 'TOGGLE_SORT':
       if (!(action.columnName in state.columnsByName)) {
         throw new Error(`Invalid column, ${action.columnName} not found`);
@@ -216,6 +222,7 @@ export const useTable = <T extends DataType>(
   }, [data, columnsWithSorting, columnsByName]);
 
   const reducer = createReducer<T>();
+
   const [state, dispatch] = useReducer(reducer, {
     columns: columnsWithSorting,
     columnsByName: columnsByName,
@@ -226,6 +233,12 @@ export const useTable = <T extends DataType>(
     filterOn: false,
   });
 
+  useEffect(() => {
+    if (tableData.length > 0) {
+      dispatch({ type: 'SET_ROWS', data: tableData });
+    }
+  }, [tableData]);
+
   const headers: HeaderType<T>[] = useMemo(() => {
     return [
       ...state.columns.map(column => {

src/test/filtering.spec.tsx

@@ -98,7 +98,7 @@ test('Should filter by text', () => {
           </thead>
           <tbody>
             {rows.map((row, idx) => (
-              <tr data-testid={`row-${idx}`} aria-label="row" key={idx}>
+              <tr data-testid={`row-${idx}`} role="table-row" key={idx}>
                 {row.cells.map((cell, idx) => (
                   <td key={idx}>{cell.render()}</td>
                 ))}
@@ -114,20 +114,20 @@ test('Should filter by text', () => {
 
   const input = table.getByTestId('input');
 
-  expect(table.getAllByLabelText('row')).toHaveLength(3);
+  expect(table.getAllByRole('table-row')).toHaveLength(3);
 
   fireEvent.change(input, { target: { value: 'Frodo' } });
 
-  expect(table.getAllByLabelText('row')).toHaveLength(1);
+  expect(table.getAllByRole('table-row')).toHaveLength(1);
   let firstRow = table.getByTestId('row-0');
 
   let { getByText } = within(firstRow);
   expect(getByText('Frodo')).toBeInTheDocument();
 
   fireEvent.change(input, { target: { value: '' } });
   expect(table.getByText('Bilbo')).toBeInTheDocument();
-  expect(table.getAllByLabelText('row')).toHaveLength(3);
+  expect(table.getAllByRole('table-row')).toHaveLength(3);
 
   fireEvent.change(input, { target: { value: 'Bag' } });
-  expect(table.getAllByLabelText('row')).toHaveLength(2);
+  expect(table.getAllByRole('table-row')).toHaveLength(2);
 });

src/test/sorting.spec.tsx

@@ -41,7 +41,7 @@ const Table = <T extends {}>({
       </thead>
       <tbody>
         {rows.map((row, idx) => (
-          <tr data-testid={`row-${idx}`} aria-label="row" key={idx}>
+          <tr data-testid={`row-${idx}`} role="table-row" key={idx}>
             {row.cells.map((cell, idx) => (
               <td key={idx}>{cell.render()}</td>
             ))}
@@ -58,7 +58,7 @@ test('Should render a table with sorting enabled', () => {
 
   const firstNameColumn = screen.getByTestId('column-firstName');
 
-  expect(screen.queryAllByLabelText('row')).toHaveLength(10);
+  expect(screen.queryAllByRole('table-row')).toHaveLength(10);
 
   // should be sorted in ascending order
   fireEvent.click(firstNameColumn);
@@ -95,17 +95,17 @@ test('Should sort by dates correctly', () => {
       );
     },
   };
-  const rtl = render(<Table columns={columns} data={data} />);
+  render(<Table columns={columns} data={data} />);
 
-  const dateColumn = rtl.getByTestId('column-birthDate');
+  const dateColumn = screen.getByTestId('column-birthDate');
 
   // should be sorted in ascending order
   fireEvent.click(dateColumn);
 
-  expect(rtl.queryByTestId('sorted-birthDate')).toBeInTheDocument();
+  expect(screen.queryByTestId('sorted-birthDate')).toBeInTheDocument();
 
-  let firstRow = rtl.getByTestId('row-0');
-  let lastRow = rtl.getByTestId('row-2');
+  let firstRow = screen.getByTestId('row-0');
+  let lastRow = screen.getByTestId('row-2');
 
   let { getByText } = within(firstRow);
   expect(getByText('Bilbo')).toBeInTheDocument();
@@ -115,8 +115,8 @@ test('Should sort by dates correctly', () => {
   // should be sorted in descending order
   fireEvent.click(dateColumn);
 
-  firstRow = rtl.getByTestId('row-0');
-  lastRow = rtl.getByTestId('row-2');
+  firstRow = screen.getByTestId('row-0');
+  lastRow = screen.getByTestId('row-2');
 
   ({ getByText } = within(firstRow));
   expect(getByText('Frodo')).toBeInTheDocument();

src/test/table.spec.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { render } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import '@testing-library/jest-dom/extend-expect';
 import { renderHook } from '@testing-library/react-hooks';
 
@@ -51,7 +51,7 @@ const Table = ({
       </thead>
       <tbody>
         {rows.map((row, idx) => (
-          <tr key={idx}>
+          <tr role="table-row" key={idx}>
             {row.cells.map((cell, idx) => (
               <td key={idx}>{cell.render()}</td>
             ))}
@@ -63,12 +63,12 @@ const Table = ({
 };
 
 test('Should render a basic table', () => {
-  const rtl = render(<Table columns={columns} data={data} />);
+  render(<Table columns={columns} data={data} />);
 
-  expect(rtl.getByText('Frodo')).toBeInTheDocument();
-  expect(rtl.getByText('Baggins')).toBeInTheDocument();
-  expect(rtl.getByText('Samwise')).toBeInTheDocument();
-  expect(rtl.getByText('Gamgee')).toBeInTheDocument();
+  expect(screen.getByText('Frodo')).toBeInTheDocument();
+  expect(screen.getByText('Baggins')).toBeInTheDocument();
+  expect(screen.getByText('Samwise')).toBeInTheDocument();
+  expect(screen.getByText('Gamgee')).toBeInTheDocument();
 });
 
 const reverseData = [
@@ -89,6 +89,16 @@ test('Should be equal regardless of field order in data', () => {
   expect(normalTl.asFragment()).toEqual(reverseTl.asFragment());
 });
 
+test('Should update table rows when data changes', () => {
+  const { rerender } = render(<Table columns={columns} data={data} />);
+  expect(screen.getAllByRole('table-row')).toHaveLength(2);
+
+  let newData = [...data, { firstName: 'Bilbo', lastName: 'Baggins' }];
+  rerender(<Table columns={columns} data={newData} />);
+
+  expect(screen.getAllByRole('table-row')).toHaveLength(3);
+});
+
 const columnsWithRender: ColumnType<any>[] = [
   {
     name: 'firstName',
@@ -102,9 +112,9 @@ const columnsWithRender: ColumnType<any>[] = [
 ];
 
 test('Should see custom row render HTML', () => {
-  const rtl = render(<Table columns={columnsWithRender} data={data} />);
+  render(<Table columns={columnsWithRender} data={data} />);
 
-  expect(rtl.getAllByTestId('first-name')).toHaveLength(2);
+  expect(screen.getAllByTestId('first-name')).toHaveLength(2);
 });
 
 const columnsWithColRender: ColumnType<any>[] = [
@@ -120,9 +130,9 @@ const columnsWithColRender: ColumnType<any>[] = [
 ];
 
 test('Should see custom column render HTML', () => {
-  const rtl = render(<Table columns={columnsWithColRender} data={data} />);
+  render(<Table columns={columnsWithColRender} data={data} />);
 
-  expect(rtl.getAllByTestId('first-name')).toHaveLength(1);
+  expect(screen.getAllByTestId('first-name')).toHaveLength(1);
 });
 
 // to supress console error from test output

src/types.ts

@@ -124,4 +124,5 @@ export type TableAction<T extends DataType> =
   | { type: 'GLOBAL_FILTER'; filter: (row: RowType<T>[]) => RowType<T>[] }
   | { type: 'SEARCH_STRING'; searchString: string }
   | { type: 'GLOBAL_FILTER_OFF' }
+  | { type: 'SET_ROWS'; data: RowType<T>[] }
   | { type: 'TOGGLE_ALL' };