fix(usegroupby): fix useGroupBy, add grouping example and fix some tests

src/plugin-hooks/tests/__snapshots__/useSortBy.test.js.snap

@@ -0,0 +1,252 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`renders a sortable table 1`] = `
+"Snapshot Diff:
+- First value
++ Second value
+
+@@ -31,11 +31,13 @@
+            colspan=\\"1\\"
+            style=\\"cursor: pointer;\\"
+            title=\\"Toggle SortBy\\"
+          >
+            First Name
+-           <span />
++           <span>
++              🔼
++           </span>
+          </th>
+          <th
+            class=\\"\\"
+            colspan=\\"1\\"
+            style=\\"cursor: pointer;\\"
+@@ -87,104 +89,104 @@
+          class=\\"\\"
+        >
+          <td
+            class=\\"\\"
+          >
+-           firstName: tanner
++           firstName: derek
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           lastName: linsley
++           lastName: perkins
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           age: 29
++           age: 40
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           visits: 100
++           visits: 40
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           status: In Relationship
++           status: Single
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           progress: 50
++           progress: 80
+          </td>
+        </tr>
+        <tr
+          class=\\"\\"
+        >
+          <td
+            class=\\"\\"
+          >
+-           firstName: derek
++           firstName: joe
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           lastName: perkins
++           lastName: bergevin
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           age: 40
++           age: 45
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           visits: 40
++           visits: 20
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           status: Single
++           status: Complicated
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           progress: 80
++           progress: 10
+          </td>
+        </tr>
+        <tr
+          class=\\"\\"
+        >
+          <td
+            class=\\"\\"
+          >
+-           firstName: joe
++           firstName: tanner
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           lastName: bergevin
++           lastName: linsley
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           age: 45
++           age: 29
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           visits: 20
++           visits: 100
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           status: Complicated
++           status: In Relationship
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           progress: 10
++           progress: 50
+          </td>
+        </tr>
+      </tbody>
+    </table>
+  </DocumentFragment>"
+`;
+
+exports[`renders a sortable table 2`] = `
+"Snapshot Diff:
+- First value
++ Second value
+
+@@ -32,11 +32,11 @@
+            style=\\"cursor: pointer;\\"
+            title=\\"Toggle SortBy\\"
+          >
+            First Name
+            <span>
+-              🔼
++              🔽
+            </span>
+          </th>
+          <th
+            class=\\"\\"
+            colspan=\\"1\\"
+@@ -89,36 +89,36 @@
+          class=\\"\\"
+        >
+          <td
+            class=\\"\\"
+          >
+-           firstName: derek
++           firstName: tanner
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           lastName: perkins
++           lastName: linsley
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           age: 40
++           age: 29
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           visits: 40
++           visits: 100
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           status: Single
++           status: In Relationship
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           progress: 80
++           progress: 50
+          </td>
+        </tr>
+        <tr
+          class=\\"\\"
+        >
+@@ -157,36 +157,36 @@
+          class=\\"\\"
+        >
+          <td
+            class=\\"\\"
+          >
+-           firstName: tanner
++           firstName: derek
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           lastName: linsley
++           lastName: perkins
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           age: 29
++           age: 40
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           visits: 100
++           visits: 40
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           status: In Relationship
++           status: Single
+          </td>
+          <td
+            class=\\"\\"
+          >
+-           progress: 50
++           progress: 80
+          </td>
+        </tr>
+      </tbody>
+    </table>
+  </DocumentFragment>"
+`;

src/plugin-hooks/tests/useSortBy.test.js

@@ -3,39 +3,36 @@ import '@testing-library/jest-dom/extend-expect'
 // NOTE: jest-dom adds handy assertions to Jest and is recommended, but not required
 
 import React from 'react'
-import { render } from '@testing-library/react'
-import useTable from '../../hooks/useTable'
-import useSortBy from '../useSortBy'
+import { render, fireEvent } from '@testing-library/react'
+import { useTable } from '../../hooks/useTable'
+import { useSortBy } from '../useSortBy'
 
-const data = React.useMemo(
-  () => [
-    {
-      firstName: 'tanner',
-      lastName: 'linsley',
-      age: 29,
-      visits: 100,
-      status: 'In Relationship',
-      progress: 50,
-    },
-    {
-      firstName: 'derek',
-      lastName: 'perkins',
-      age: 40,
-      visits: 40,
-      status: 'Single',
-      progress: 80,
-    },
-    {
-      firstName: 'joe',
-      lastName: 'bergevin',
-      age: 45,
-      visits: 20,
-      status: 'Complicated',
-      progress: 10,
-    },
-  ],
-  []
-)
+const data = [
+  {
+    firstName: 'tanner',
+    lastName: 'linsley',
+    age: 29,
+    visits: 100,
+    status: 'In Relationship',
+    progress: 50,
+  },
+  {
+    firstName: 'derek',
+    lastName: 'perkins',
+    age: 40,
+    visits: 40,
+    status: 'Single',
+    progress: 80,
+  },
+  {
+    firstName: 'joe',
+    lastName: 'bergevin',
+    age: 45,
+    visits: 20,
+    status: 'Complicated',
+    progress: 10,
+  },
+]
 
 const defaultColumn = {
   Cell: ({ value, column: { id } }) => `${id}: ${value}`,
@@ -131,16 +128,18 @@ function App() {
 }
 
 test('renders a sortable table', () => {
-  const { getAllByText } = render(<App />)
+  const { getByText, asFragment } = render(<App />)
 
-  const firstNames = getAllByText('firstName')
+  const beforeSort = asFragment()
 
-  console.log(firstNames)
+  fireEvent.click(getByText('First Name'))
 
-  // expect(getByText('tanner')).toBeInTheDocument()
-  // expect(getByText('linsley')).toBeInTheDocument()
-  // expect(getByText('29')).toBeInTheDocument()
-  // expect(getByText('100')).toBeInTheDocument()
-  // expect(getByText('In Relationship')).toBeInTheDocument()
-  // expect(getByText('50')).toBeInTheDocument()
+  const afterSort1 = asFragment()
+
+  fireEvent.click(getByText('First Name'))
+
+  const afterSort2 = asFragment()
+
+  expect(beforeSort).toMatchDiffSnapshot(afterSort1)
+  expect(afterSort1).toMatchDiffSnapshot(afterSort2)
 })

src/plugin-hooks/useExpanded.js

@@ -14,8 +14,12 @@ const propTypes = {
   paginateSubRows: PropTypes.bool,
 }
 
-export const useExpanded = props => {
-  PropTypes.checkPropTypes(propTypes, props, 'property', 'useExpanded')
+export const useExpanded = hooks => {
+  hooks.useMain.push(useMain)
+}
+
+function useMain(instance) {
+  PropTypes.checkPropTypes(propTypes, instance, 'property', 'useExpanded')
 
   const {
     debug,
@@ -24,7 +28,7 @@ export const useExpanded = props => {
     hooks,
     state: [{ expanded }, setState],
     paginateSubRows = true,
-  } = props
+  } = instance
 
   const toggleExpandedByPath = (path, set) => {
     return setState(old => {
@@ -52,23 +56,19 @@ export const useExpanded = props => {
 
       // Here we do some mutation, but it's the last stage in the
       // immutable process so this is safe
-      const handleRow = (row, depth = 0, parentPath = []) => {
-        // Compute some final state for the row
-        const path = [...parentPath, row.index]
-
-        row.path = path
-        row.depth = depth
-
+      const handleRow = row => {
         row.isExpanded =
           (row.original && row.original[manualExpandedKey]) ||
-          getBy(expanded, path)
+          getBy(expanded, row.path)
 
         if (paginateSubRows || (!paginateSubRows && row.depth === 0)) {
           expandedRows.push(row)
         }
 
+        row.canExpand = row.subRows && !!row.subRows.length
+
         if (row.isExpanded && row.subRows && row.subRows.length) {
-          row.subRows.forEach((row, i) => handleRow(row, depth + 1, path))
+          row.subRows.forEach((row, i) => handleRow(row))
         }
 
         return row
@@ -84,7 +84,7 @@ export const useExpanded = props => {
   const expandedDepth = findExpandedDepth(expanded)
 
   return {
-    ...props,
+    ...instance,
     toggleExpandedByPath,
     expandedDepth,
     rows: expandedRows,

src/plugin-hooks/useGroupBy.js

@@ -1,4 +1,3 @@
-import React from 'react'
 import { useMemo } from 'react'
 import PropTypes from 'prop-types'
 
@@ -20,7 +19,13 @@ const propTypes = {
   // General
   columns: PropTypes.arrayOf(
     PropTypes.shape({
-      aggregate: PropTypes.func,
+      aggregate: PropTypes.oneOfType([
+        PropTypes.func,
+        PropTypes.string,
+        PropTypes.arrayOf(
+          PropTypes.oneOfType([PropTypes.func, PropTypes.string])
+        ),
+      ]),
       disableGrouping: PropTypes.bool,
       Aggregated: PropTypes.any,
     })
@@ -31,33 +36,35 @@ const propTypes = {
   aggregations: PropTypes.object,
 }
 
-export const useGroupBy = props => {
-  PropTypes.checkPropTypes(propTypes, props, 'property', 'useGroupBy')
+export const useGroupBy = hooks => {
+  hooks.columnsBeforeHeaderGroups.push(columnsBeforeHeaderGroups)
+  hooks.useMain.push(useMain)
+}
+
+function columnsBeforeHeaderGroups(columns, { state: [{ groupBy }] }) {
+  // Sort grouped columns to the start of the column list
+  // before the headers are built
+  return [
+    ...groupBy.map(g => columns.find(col => col.id === g)),
+    ...columns.filter(col => !groupBy.includes(col.id)),
+  ]
+}
+
+function useMain(instance) {
+  PropTypes.checkPropTypes(propTypes, instance, 'property', 'useGroupBy')
 
   const {
     debug,
     rows,
     columns,
+    headers,
     groupByFn = defaultGroupByFn,
     manualGroupBy,
     disableGrouping,
     aggregations: userAggregations = {},
     hooks,
     state: [{ groupBy }, setState],
-  } = props
-
-  // Sort grouped columns to the start of the column list
-  // before the headers are built
-  hooks.useColumnsBeforeHeaderGroups.push(columns => {
-    // eslint-disable-next-line react-hooks/rules-of-hooks
-    return React.useMemo(
-      () => [
-        ...groupBy.map(g => columns.find(col => col.id === g)),
-        ...columns.filter(col => !groupBy.includes(col.id)),
-      ],
-      [columns]
-    )
-  })
+  } = instance
 
   columns.forEach(column => {
     const { id, accessor, disableGrouping: columnDisableGrouping } = column
@@ -72,6 +79,10 @@ export const useGroupBy = props => {
         )
       : false
 
+    if (column.canGroupBy) {
+      column.toggleGroupBy = () => toggleGroupBy(column.id)
+    }
+
     column.Aggregated = column.Aggregated || column.Cell
   })
 
@@ -92,100 +103,134 @@ export const useGroupBy = props => {
     }, actions.toggleGroupBy)
   }
 
-  hooks.useColumns.push(columns => {
-    columns.forEach(column => {
-      if (column.canGroupBy) {
-        column.toggleGroupBy = () => toggleGroupBy(column.id)
-      }
-    })
-    return columns
-  })
-
   hooks.getGroupByToggleProps = []
 
-  const addGroupByToggleProps = (columns, api) => {
-    columns.forEach(column => {
-      const { canGroupBy } = column
-      column.getGroupByToggleProps = props => {
-        return mergeProps(
-          {
-            onClick: canGroupBy
-              ? e => {
-                  e.persist()
-                  column.toggleGroupBy()
-                }
-              : undefined,
-            style: {
-              cursor: canGroupBy ? 'pointer' : undefined,
-            },
-            title: 'Toggle GroupBy',
+  //
+  ;[...columns, ...headers].forEach(column => {
+    const { canGroupBy } = column
+    column.getGroupByToggleProps = props => {
+      return mergeProps(
+        {
+          onClick: canGroupBy
+            ? e => {
+                e.persist()
+                column.toggleGroupBy()
+              }
+            : undefined,
+          style: {
+            cursor: canGroupBy ? 'pointer' : undefined,
           },
-          applyPropHooks(api.hooks.getGroupByToggleProps, column, api),
-          props
-        )
-      }
-    })
-    return columns
-  }
+          title: 'Toggle GroupBy',
+        },
+        applyPropHooks(instance.hooks.getGroupByToggleProps, column, instance),
+        props
+      )
+    }
+  })
 
-  hooks.useColumns.push(addGroupByToggleProps)
-  hooks.useHeaders.push(addGroupByToggleProps)
+  hooks.prepareRow.push(row => {
+    row.cells.forEach(cell => {
+      // Grouped cells are in the groupBy and the pivot cell for the row
+      cell.grouped = cell.column.grouped && cell.column.id === row.groupByID
+      // Repeated cells are any columns in the groupBy that are not grouped
+      cell.repeatedValue = !cell.grouped && cell.column.grouped
+      // Aggregated cells are not grouped, not repeated, but still have subRows
+      cell.aggregated = !cell.grouped && !cell.repeatedValue && row.canExpand
+    })
+    return row
+  })
 
   const groupedRows = useMemo(
     () => {
       if (manualGroupBy || !groupBy.length) {
         return rows
       }
+
       if (debug) console.info('getGroupedRows')
       // Find the columns that can or are aggregating
 
       // Uses each column to aggregate rows into a single value
-      const aggregateRowsToValues = rows => {
+      const aggregateRowsToValues = (rows, isSourceRows) => {
         const values = {}
+
         columns.forEach(column => {
+          // Don't aggregate columns that are in the groupBy
+          if (groupBy.includes(column.id)) {
+            values[column.id] = rows[0] ? rows[0].values[column.id] : null
+            return
+          }
+
           const columnValues = rows.map(d => d.values[column.id])
-          let aggregate =
-            userAggregations[column.aggregate] ||
-            aggregations[column.aggregate] ||
-            column.aggregate
-          if (typeof aggregate === 'function') {
-            values[column.id] = aggregate(columnValues, rows)
-          } else if (aggregate) {
+
+          let aggregator = column.aggregate
+
+          if (Array.isArray(aggregator)) {
+            if (aggregator.length !== 2) {
+              console.info({ column })
+              throw new Error(
+                `React Table: Complex aggregators must have 2 values, eg. aggregate: ['sum', 'count']. More info above...`
+              )
+            }
+            if (isSourceRows) {
+              aggregator = aggregator[1]
+            } else {
+              aggregator = aggregator[0]
+            }
+          }
+
+          let aggregateFn =
+            typeof aggregator === 'function'
+              ? aggregator
+              : userAggregations[aggregator] || aggregations[aggregator]
+
+          if (aggregateFn) {
+            values[column.id] = aggregateFn(columnValues, rows)
+          } else if (aggregator) {
+            console.info({ column })
             throw new Error(
-              `Invalid aggregate "${aggregate}" passed to column with ID: "${
-                column.id
-              }"`
+              `React Table: Invalid aggregate option for column listed above`
             )
           } else {
-            values[column.id] = columnValues[0]
+            values[column.id] = null
           }
         })
         return values
       }
 
       // Recursively group the data
-      const groupRecursively = (rows, groupBy, depth = 0) => {
+      const groupRecursively = (rows, depth = 0, parentPath = []) => {
         // This is the last level, just return the rows
         if (depth >= groupBy.length) {
           return rows
         }
 
-        // Group the rows together for this level
-        let groupedRows = Object.entries(groupByFn(rows, groupBy[depth])).map(
-          ([groupByVal, subRows], index) => {
-            // Recurse to sub rows before aggregation
-            subRows = groupRecursively(subRows, groupBy, depth + 1)
+        const columnID = groupBy[depth]
 
-            const values = aggregateRowsToValues(subRows)
+        // Group the rows together for this level
+        let groupedRows = groupByFn(rows, columnID)
+
+        // Recurse to sub rows before aggregation
+        groupedRows = Object.entries(groupedRows).map(
+          ([groupByVal, subRows], index) => {
+            const path = [...parentPath, groupByVal]
+
+            subRows = groupRecursively(subRows, depth + 1, path)
+
+            const values = aggregateRowsToValues(
+              subRows,
+              depth + 1 >= groupBy.length
+            )
 
             const row = {
-              groupByID: groupBy[depth],
+              groupByID: columnID,
               groupByVal,
               values,
               subRows,
               depth,
               index,
+              path,
             }
+
             return row
           }
         )
@@ -194,13 +239,13 @@ export const useGroupBy = props => {
       }
 
       // Assign the new data
-      return groupRecursively(rows, groupBy)
+      return groupRecursively(rows)
     },
     [manualGroupBy, groupBy, debug, rows, columns, userAggregations, groupByFn]
   )
 
   return {
-    ...props,
+    ...instance,
     toggleGroupBy,
     rows: groupedRows,
     preGroupedRows: rows,