Merge branch 'main' into fix-app-wrapper-stale-cells · TanStack/table@9245a93 · GitHub
Skip to content

Commit 9245a93

Browse files
authored
Merge branch 'main' into fix-app-wrapper-stale-cells
2 parents 2dfa9d6 + e26e988 commit 9245a93

7 files changed

Lines changed: 362 additions & 17 deletions

File tree

Lines changed: 6 additions & 0 deletions

‎.github/ISSUE_TEMPLATE/bug_report.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ body:
66
value: |
77
Thank you for reporting an issue :pray:.
88
9-
This issue tracker is for reporting reproducible bugs or regression's found in [react-table](https://github.com/tanstack/table)
9+
This issue tracker is for reporting reproducible bugs or regression's found in [TanStack Table](https://github.com/tanstack/table)
1010
If you have a question about how to achieve or implement something and are struggling, please post a question
11-
inside of react-table's [Discussions tab](https://github.com/tanstack/table/discussions) instead of filing an issue.
11+
inside of TanStack Table's [Discussions tab](https://github.com/tanstack/table/discussions) instead of filing an issue.
1212
1313
Before submitting a new bug/issue, please check the links below to see if there is a solution or question posted there already:
1414
- TanStack Table's [Discussions tab](https://github.com/tanstack/table/discussions)

‎docs/framework/react/guide/react-compiler.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ function TableBody({ table }) {
192192

193193
The selector must include every state slice that can change the result of `table.getRowModel()` for this table. That may include filtering, sorting, grouping, expansion, or pagination, depending on the features in use.
194194

195+
State is not the only input to `table.getRowModel()`. Replacing `data` from outside the table, such as a `setState` call, a query refetch, or a WebSocket message, does not change any state slice, so a `Subscribe` selector cannot detect it. When the body reads the table from `useTableContext()` inside `<table.AppTable>`, `AppTable` provides a new table value whenever the owner of `useAppTable` renders with new options, so the body updates without extra wiring. When you pass `table` as a prop through components that the compiler may skip, also pass `data` (or the rows you render) as a prop so the body renders again when the data changes.
196+
195197
Use `table.Subscribe` when `table` is the React-facing value returned by `useTable`. In a cell or header render context, `table` is the core Table instead. Import the standalone `Subscribe` component and pass `table.store` or one of the values in `table.atoms` as its source.
196198

197199
Do not add a subscription to every cell by default. Add one when a nested component reads state that is not represented by its props, or when profiling shows that moving an update lower in the tree would help.

‎packages/preact-table/src/createTableHook.tsx‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
'use client'
22
import { createContext } from 'preact'
3-
import { useContext, useMemo, useRef } from 'preact/hooks'
3+
import {
4+
useContext,
5+
useEffect,
6+
useLayoutEffect,
7+
useMemo,
8+
useReducer,
9+
useRef,
10+
useState,
11+
} from 'preact/hooks'
412
import { createColumnHelper as coreCreateColumnHelper } from '@tanstack/table-core'
513
import { useTable } from './useTable'
614
import { FlexRender } from './FlexRender'
@@ -30,6 +38,9 @@ import type {
3038
import type { ComponentChildren, ComponentType, Context } from 'preact'
3139
import type { PreactTable } from './useTable'
3240

41+
const useIsomorphicLayoutEffect =
42+
typeof window === 'undefined' ? useEffect : useLayoutEffect
43+
3344
const sharedTableContext = createContext<PreactTable<any, any> | null>(null)
3445
const sharedCellContext = createContext<Cell<any, any, any> | null>(null)
3546
const sharedHeaderContext = createContext<Header<any, any, any> | null>(null)
@@ -944,12 +955,16 @@ export function createTableHook<
944955
TCellComponents,
945956
THeaderComponents
946957
> {
947-
// Merge default options with provided options (provided takes precedence)
958+
// Merge default options with provided options (provided takes precedence).
959+
// Memoize the merge so stable caller options keep `table` stable; otherwise
960+
// every owner render would refresh the AppTable context and re-render all
961+
// `useTableContext()` consumers, even under a cached AppTable element.
962+
const mergedTableOptions = useMemo(
963+
() => ({ ...defaultTableOptions, ...tableOptions }),
964+
[tableOptions],
965+
)
948966
const table = useTable<TFeatures, TData, TSelected>(
949-
{ ...defaultTableOptions, ...tableOptions } as TableOptions<
950-
TFeatures,
951-
TData
952-
>,
967+
mergedTableOptions as TableOptions<TFeatures, TData>,
953968
selector,
954969
)
955970

@@ -963,6 +978,15 @@ export function createTableHook<
963978
const tableRef = useRef(table)
964979
tableRef.current = table
965980

981+
// When the owner's output is cached (`useMemo`, `memo`), Preact skips the
982+
// stable `AppTable` vnode, so its provider would keep the table from an
983+
// earlier render. AppTable subscribes to owner commits and re-renders only
984+
// when the table it last provided is outdated.
985+
const [tableListeners] = useState(() => new Set<() => void>())
986+
useIsomorphicLayoutEffect(() => {
987+
tableListeners.forEach((listener) => listener())
988+
}, [table])
989+
966990
// AppTable - Root wrapper that provides table context with optional Subscribe
967991
const AppTable = useMemo(() => {
968992
function AppTableImpl(
@@ -978,6 +1002,21 @@ export function createTableHook<
9781002
): ComponentChildren {
9791003
const { children, selector: appTableSelector } = props as any
9801004
const currentTable = tableRef.current
1005+
const providedTableRef = useRef(currentTable)
1006+
providedTableRef.current = currentTable
1007+
const [, forceRender] = useReducer((count: number) => count + 1, 0)
1008+
1009+
useIsomorphicLayoutEffect(() => {
1010+
const listener = () => {
1011+
if (providedTableRef.current !== tableRef.current) {
1012+
forceRender(undefined)
1013+
}
1014+
}
1015+
tableListeners.add(listener)
1016+
return () => {
1017+
tableListeners.delete(listener)
1018+
}
1019+
}, [])
9811020

9821021
return (
9831022
<TableContext.Provider value={currentTable}>

‎packages/preact-table/tests/unit/createTableHook.test.tsx‎

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// @vitest-environment jsdom
22

33
import { cleanup, fireEvent, render, screen } from '@testing-library/preact'
4-
import { useEffect, useState } from 'preact/hooks'
4+
import { useEffect, useMemo, useState } from 'preact/hooks'
55
import { act } from 'preact/test-utils'
66
import { rowSelectionFeature, tableFeatures } from '@tanstack/table-core'
77
import { afterEach, describe, expect, test, vi } from 'vitest'
@@ -216,6 +216,137 @@ describe('createTableHook runtime', () => {
216216
expect(unmountCaptor).not.toHaveBeenCalled()
217217
})
218218

219+
test.each([
220+
['cached', true],
221+
['uncached', false],
222+
])(
223+
'refreshes the AppTable context once when the owner replaces data under a %s element',
224+
(_label, cacheTree) => {
225+
const renderCaptor = vi.fn()
226+
227+
function RowNames() {
228+
const table = appTable.useTableContext<Person>()
229+
renderCaptor()
230+
231+
return (
232+
<output aria-label="Context row names">
233+
{table
234+
.getRowModel()
235+
.rows.map((row) => row.original.name)
236+
.join(',')}
237+
</output>
238+
)
239+
}
240+
241+
function TableHarness() {
242+
const [data, setData] = useState([{ id: '1', name: 'Ada' }])
243+
const table = appTable.useAppTable(
244+
{ data, columns, getRowId: (row) => row.id },
245+
() => null,
246+
)
247+
// The React Compiler caches this element because `table.AppTable` is
248+
// stable, so React skips AppTable when only `data` changes.
249+
const cachedTree = useMemo(
250+
() => (
251+
<table.AppTable>
252+
<RowNames />
253+
</table.AppTable>
254+
),
255+
[table.AppTable],
256+
)
257+
const tree = cacheTree ? (
258+
cachedTree
259+
) : (
260+
<table.AppTable>
261+
<RowNames />
262+
</table.AppTable>
263+
)
264+
265+
return (
266+
<>
267+
{tree}
268+
<button onClick={() => setData([{ id: '1', name: 'Grace' }])}>
269+
Replace data
270+
</button>
271+
</>
272+
)
273+
}
274+
275+
render(<TableHarness />)
276+
277+
expect(outputText('Context row names')).toBe('Ada')
278+
expect(renderCaptor).toHaveBeenCalledTimes(1)
279+
280+
act(() => {
281+
click('Replace data')
282+
})
283+
284+
expect(outputText('Context row names')).toBe('Grace')
285+
expect(renderCaptor).toHaveBeenCalledTimes(2)
286+
},
287+
)
288+
289+
test('keeps the AppTable context stable when the owner re-renders with the same options', () => {
290+
const renderCaptor = vi.fn()
291+
292+
function RowNames() {
293+
const table = appTable.useTableContext<Person>()
294+
renderCaptor()
295+
296+
return (
297+
<output aria-label="Context row names">
298+
{table
299+
.getRowModel()
300+
.rows.map((row) => row.original.name)
301+
.join(',')}
302+
</output>
303+
)
304+
}
305+
306+
const data = [{ id: '1', name: 'Ada' }]
307+
308+
function TableHarness() {
309+
const [count, setCount] = useState(0)
310+
// The React Compiler memoizes these options because their inputs are
311+
// stable, so an unrelated owner render should not refresh the context.
312+
const tableOptions = useMemo(
313+
() => ({ data, columns, getRowId: (row: Person) => row.id }),
314+
[],
315+
)
316+
const table = appTable.useAppTable(tableOptions, () => null)
317+
const cachedTree = useMemo(
318+
() => (
319+
<table.AppTable>
320+
<RowNames />
321+
</table.AppTable>
322+
),
323+
[table.AppTable],
324+
)
325+
326+
return (
327+
<>
328+
{cachedTree}
329+
<button onClick={() => setCount(count + 1)}>Render owner</button>
330+
</>
331+
)
332+
}
333+
334+
render(<TableHarness />)
335+
336+
expect(outputText('Context row names')).toBe('Ada')
337+
expect(renderCaptor).toHaveBeenCalledTimes(1)
338+
339+
act(() => {
340+
click('Render owner')
341+
})
342+
act(() => {
343+
click('Render owner')
344+
})
345+
346+
expect(outputText('Context row names')).toBe('Ada')
347+
expect(renderCaptor).toHaveBeenCalledTimes(1)
348+
})
349+
219350
test('throws focused errors when context hooks are used outside wrappers', () => {
220351
function MissingTableContext() {
221352
appTable.useTableContext<Person>()

‎packages/react-table/src/createTableHook.tsx‎

Lines changed: 44 additions & 8 deletions

0 commit comments

Comments
 (0)