fix: row selection method cloning bug (#6314)

Author: KevinVandy

packages/table-core/src/features/row-selection/rowSelectionFeature.utils.ts

@@ -649,7 +649,7 @@ export function selectRowsFn<
   ): Array<Row<TFeatures, TData>> => {
     const result: Array<Row<TFeatures, TData>> = []
     for (let i = 0; i < rows.length; i++) {
-      let row = rows[i]!
+      const row = rows[i]!
       const isSelected = isRowSelected(row)
 
       if (isSelected) {
@@ -658,13 +658,18 @@ export function selectRowsFn<
       }
 
       if (row.subRows.length) {
-        row = {
-          ...row,
-          subRows: recurseRows(row.subRows, depth + 1),
+        // Always recurse — selected descendants of unselected parents must
+        // still be collected into flatRows/rowsById.
+        const newSubRows = recurseRows(row.subRows, depth + 1)
+
+        if (isSelected) {
+          // Preserve prototype chain so methods like getValue() remain accessible
+          const cloned = Object.create(Object.getPrototypeOf(row))
+          Object.assign(cloned, row)
+          cloned.subRows = newSubRows
+          result.push(cloned)
         }
-      }
-
-      if (isSelected) {
+      } else if (isSelected) {
         result.push(row)
       }
     }

packages/table-core/tests/implementation/features/row-selection/rowSelectionFeature.test.ts

@@ -96,6 +96,65 @@ describe('rowSelectionFeature', () => {
       expect(result.rowsById).toHaveProperty('0.0')
     })
 
+    it('should collect selected descendants of unselected parents', () => {
+      const data = generateTestData(3, 2)
+      const columns = generateColumnDefs(data)
+
+      const table = constructTable({
+        features,
+        rowModels: {},
+        enableRowSelection: true,
+        renderFallbackValue: '',
+        data,
+        getSubRows: (originalRow: Person, _idx: number) => originalRow.subRows,
+        initialState: {
+          rowSelection: {
+            '0.0': true, // child selected, parent '0' is not
+          },
+        },
+        columns,
+      })
+      const rowModel = table.getCoreRowModel()
+
+      const result = RowSelectionUtils.selectRowsFn(rowModel)
+
+      expect(result.rows.length).toBe(0)
+      expect(result.flatRows.length).toBe(1)
+      expect(result.flatRows[0]?.id).toBe('0.0')
+      expect(result.rowsById).toHaveProperty('0.0')
+    })
+
+    it('should preserve row prototype methods on cloned parent rows', () => {
+      const data = generateTestData(3, 2)
+      const columns = generateColumnDefs(data)
+
+      const table = constructTable<typeof features, Person>({
+        features,
+        rowModels: {},
+        enableRowSelection: true,
+        renderFallbackValue: '',
+        data,
+        getSubRows: (originalRow: Person, _idx: number) => originalRow.subRows,
+        initialState: {
+          rowSelection: {
+            '0': true,
+            '0.0': true,
+          },
+        },
+        columns,
+      })
+      const rowModel = table.getCoreRowModel()
+
+      const result = RowSelectionUtils.selectRowsFn(rowModel)
+
+      // Selected parents with subRows are cloned; the clone must keep the
+      // shared row prototype so APIs like getValue() still work
+      const clonedParent = result.rows[0]!
+      expect(clonedParent).not.toBe(rowModel.rows[0])
+      expect(typeof clonedParent.getValue).toBe('function')
+      expect(clonedParent.getValue('id')).toBe(rowModel.rows[0]!.getValue('id'))
+    })
+
     it('should return an empty list if no rows are selected', () => {
       const data = generateTestData(5)
       const columns = generateColumnDefs(data)

perf.md

@@ -94,10 +94,10 @@ Typecheck verified clean after the sweep (`pnpm tsc --noEmit` passes).
 ## Progress
 
 - **Total findings:** 61
-- **Done `[x]`:** 19
+- **Done `[x]`:** 20
 - **Partial `[~]`:** 2
 - **Skipped `[-]`:** 5
-- **Not started `[ ]`:** 35
+- **Not started `[ ]`:** 34
 
 _(Update these counters as you go.)_
 
@@ -1734,8 +1734,8 @@ Replace `let isAll = …; if (cond) isAll = false; return isAll` with `return !p
 
 ## 48. `selectRowsFn` spreads row object even when subRows did not change — Score: 4
 
-**Status:** `[ ]` not started
-**Implementation note:** _(none)_
+**Status:** `[x]` done
+**Implementation note:** Implemented a **reframed fix** — the proposed `newSubRows !== row.subRows` check is dead on arrival: `recurseRows` allocates a fresh `result` array on every call and returns it unconditionally, so the reference is _always_ different and the check could never skip a clone. (The scale table's premise is also inverted: an unmatched subtree returns `[]`, never the original reference.) The real waste in the same lines: the clone ran for **every** row with subRows, but unselected rows are never pushed to `result` — so their clone was allocated and immediately discarded. Fix: recurse unconditionally (required — selected descendants of unselected parents must still be collected into `flatRows`/`rowsById`), then build the clone only when `isSelected`. Under sparse selection on hierarchical data this eliminates nearly all clones. **Bug fix included:** rows are `Object.create(rowPrototype)` instances, but the old clone was a plain spread `{ ...row, subRows }` — which drops the prototype, so cloned parent rows in the selected row models lost all their prototype APIs (`getValue()`, etc.). The clone now uses the #49 precedent: `Object.assign(Object.create(Object.getPrototypeOf(row)), row)` + `subRows` assignment. Added regression tests (selected child under unselected parent; prototype-method survival on cloned parents) in `tests/implementation/features/row-selection/rowSelectionFeature.test.ts` — the existing suite only covered selected-child-under-selected-parent and would not have caught a recursion-skipping mistake.
 
 **Location:** `src/features/row-selection/rowSelectionFeature.utils.ts:618–658`
 **Category:** `micro`