fix(table-core): getTopRows and getBottomRows ref stability - fixes #…

…5624 (#5637) * fix(table-core): fixes #5624 * update tests
TanStack · Jun 30, 2024 · ace8a46 · ace8a46
1 parent 18d581e
commit ace8a46
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 40 deletions.
diff --git a/examples/react/row-pinning/src/main.tsx b/examples/react/row-pinning/src/main.tsx
@@ -1,4 +1,4 @@
-import React, { HTMLProps } from 'react'
+import React from 'react'
 import ReactDOM from 'react-dom/client'
 
 import './index.css'

diff --git a/packages/table-core/src/features/RowPinning.ts b/packages/table-core/src/features/RowPinning.ts
@@ -76,7 +76,11 @@ export interface RowPinningRow {
 }
 
 export interface RowPinningInstance<TData extends RowData> {
-  _getPinnedRows: (position: 'top' | 'bottom') => Row<TData>[]
+  _getPinnedRows: (
+    visiblePinnedRows: Array<Row<TData>>,
+    pinnedRowIds: Array<string> | undefined,
+    position: 'top' | 'bottom'
+  ) => Row<TData>[]
   /**
    * Returns all bottom pinned rows.
    * @link [API Docs](https://tanstack.com/table/v8/docs/api/features/row-pinning#getbottomrows)
@@ -199,9 +203,9 @@ export const RowPinning: TableFeature = {
       const position = row.getIsPinned()
       if (!position) return -1
 
-      const visiblePinnedRowIds = table
-        ._getPinnedRows(position)
-        ?.map(({ id }) => id)
+      const visiblePinnedRowIds = (
+        position === 'top' ? table.getTopRows() : table.getBottomRows()
+      )?.map(({ id }) => id)
 
       return visiblePinnedRowIds?.indexOf(row.id) ?? -1
     }
@@ -226,36 +230,36 @@ export const RowPinning: TableFeature = {
       return Boolean(pinningState[position]?.length)
     }
 
-    table._getPinnedRows = memo(
-      position => [
-        table.getRowModel().rows,
-        table.getState().rowPinning[position!],
-        position,
-      ],
-      (visibleRows, pinnedRowIds, position) => {
-        const rows =
-          table.options.keepPinnedRows ?? true
-            ? //get all rows that are pinned even if they would not be otherwise visible
-              //account for expanded parent rows, but not pagination or filtering
-              (pinnedRowIds ?? []).map(rowId => {
-                const row = table.getRow(rowId, true)
-                return row.getIsAllParentsExpanded() ? row : null
-              })
-            : //else get only visible rows that are pinned
-              (pinnedRowIds ?? []).map(
-                rowId => visibleRows.find(row => row.id === rowId)!
-              )
+    table._getPinnedRows = (visibleRows, pinnedRowIds, position) => {
+      const rows =
+        table.options.keepPinnedRows ?? true
+          ? //get all rows that are pinned even if they would not be otherwise visible
+            //account for expanded parent rows, but not pagination or filtering
+            (pinnedRowIds ?? []).map(rowId => {
+              const row = table.getRow(rowId, true)
+              return row.getIsAllParentsExpanded() ? row : null
+            })
+          : //else get only visible rows that are pinned
+            (pinnedRowIds ?? []).map(
+              rowId => visibleRows.find(row => row.id === rowId)!
+            )
 
-        return rows
-          .filter(Boolean)
-          .map(d => ({ ...d, position })) as Row<TData>[]
-      },
-      getMemoOptions(table.options, 'debugRows', '_getPinnedRows')
-    )
+      return rows.filter(Boolean).map(d => ({ ...d, position })) as Row<TData>[]
+    }
 
-    table.getTopRows = () => table._getPinnedRows('top')
+    table.getTopRows = memo(
+      () => [table.getRowModel().rows, table.getState().rowPinning.top],
+      (allRows, topPinnedRowIds) =>
+        table._getPinnedRows(allRows, topPinnedRowIds, 'top'),
+      getMemoOptions(table.options, 'debugRows', 'getTopRows')
+    )
 
-    table.getBottomRows = () => table._getPinnedRows('bottom')
+    table.getBottomRows = memo(
+      () => [table.getRowModel().rows, table.getState().rowPinning.bottom],
+      (allRows, bottomPinnedRowIds) =>
+        table._getPinnedRows(allRows, bottomPinnedRowIds, 'bottom'),
+      getMemoOptions(table.options, 'debugRows', 'getBottomRows')
+    )
 
     table.getCenterRows = memo(
       () => [

diff --git a/packages/table-core/tests/RowPinning.test.ts b/packages/table-core/tests/RowPinning.test.ts
@@ -6,7 +6,6 @@ import {
   getCoreRowModel,
   getPaginationRowModel,
 } from '../src'
-import * as RowPinning from '../src/features/RowPinning'
 import { makeData, Person } from './makeTestData'
 
 type personKeys = keyof Person
@@ -23,7 +22,7 @@ function generateColumns(people: Person[]): PersonColumn[] {
 
 describe('RowPinning', () => {
   describe('createTable', () => {
-    describe('_getPinnedRows', () => {
+    describe('getTopRows', () => {
       it('should return pinned rows when keepPinnedRows is true rows are visible', () => {
         const data = makeData(10)
         const columns = generateColumns(data)
@@ -48,7 +47,7 @@ describe('RowPinning', () => {
           getCoreRowModel: getCoreRowModel(),
         })
 
-        const result = table._getPinnedRows('top')
+        const result = table.getTopRows()
 
         expect(result.length).toBe(2)
         expect(result[0].id).toBe('0')
@@ -78,7 +77,7 @@ describe('RowPinning', () => {
           getCoreRowModel: getCoreRowModel(),
         })
 
-        const result = table._getPinnedRows('top')
+        const result = table.getTopRows()
 
         expect(result.length).toBe(2)
         expect(result[0].id).toBe('0')
@@ -108,7 +107,7 @@ describe('RowPinning', () => {
           getCoreRowModel: getCoreRowModel(),
         })
 
-        const result = table._getPinnedRows('top')
+        const result = table.getTopRows()
 
         expect(result.length).toBe(2)
         expect(result[0].id).toBe('0')
@@ -138,12 +137,11 @@ describe('RowPinning', () => {
           getCoreRowModel: getCoreRowModel(),
         })
 
-        const result = table._getPinnedRows('top')
+        const result = table.getTopRows()
 
         expect(result.length).toBe(0)
       })
-    })
-    describe('getTopRows', () => {
+
       it('should return correct top rows', () => {
         const data = makeData(10)
         const columns = generateColumns(data)