From 2dfa9d61cc351a400851f23dc3c96a04feb69116 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Mon, 12 Oct 2026 10:29:10 -0500 Subject: [PATCH] fix(svelte-table, vue-table): Keep App* wrappers in sync with new cells and headers Svelte's App* wrappers destructured their props, which read the prop getters once, and Vue's setup() read props.cell once. A keyed {#each} or v-for that reused a wrapper with a new cell or header (same id, e.g. after replacing data or columns) kept the first instance for both the snippet/slot value and the context. Forward wrapper props through getters, read the prop reactively, and provide a live proxy that forwards to the current instance. Prototype methods run against the real instance so per-instance memos never capture the proxy. --- .changeset/app-wrapper-live-cells.md | 6 ++ packages/svelte-table/src/AppCell.svelte | 12 ++- packages/svelte-table/src/AppHeader.svelte | 12 ++- .../src/createTableHook.svelte.ts | 45 +++++++--- packages/svelte-table/src/live-proxy.ts | 50 +++++++++++ .../tests/fixtures/HookDataRefresh.svelte | 47 ++++++++++ .../tests/fixtures/HookHeaderLabel.svelte | 10 +++ packages/svelte-table/tests/rendering.test.ts | 20 +++++ packages/vue-table/src/createTableHook.ts | 83 +++++++++++------- packages/vue-table/src/live-proxy.ts | 50 +++++++++++ .../vue-table/tests/unit/rendering.test.ts | 86 +++++++++++++++++++ 11 files changed, 369 insertions(+), 52 deletions(-) create mode 100644 .changeset/app-wrapper-live-cells.md create mode 100644 packages/svelte-table/src/live-proxy.ts create mode 100644 packages/svelte-table/tests/fixtures/HookDataRefresh.svelte create mode 100644 packages/svelte-table/tests/fixtures/HookHeaderLabel.svelte create mode 100644 packages/vue-table/src/live-proxy.ts diff --git a/.changeset/app-wrapper-live-cells.md b/.changeset/app-wrapper-live-cells.md new file mode 100644 index 0000000000..1d85f35248 --- /dev/null +++ b/.changeset/app-wrapper-live-cells.md @@ -0,0 +1,6 @@ +--- +'@tanstack/svelte-table': patch +'@tanstack/vue-table': patch +--- + +Keep `AppCell`, `AppHeader`, and `AppFooter` in sync with their `cell`/`header` prop. When `data` or `columns` are replaced, a keyed `{#each}` or `v-for` reuses the wrapper with a new instance that has the same id. The wrapper kept the first instance, so the snippet/slot value and `useCellContext()`/`useHeaderContext()` showed stale values. The wrappers now read the current prop, and the context forwards to it. diff --git a/packages/svelte-table/src/AppCell.svelte b/packages/svelte-table/src/AppCell.svelte index db2c670968..7da5df0d7a 100644 --- a/packages/svelte-table/src/AppCell.svelte +++ b/packages/svelte-table/src/AppCell.svelte @@ -1,6 +1,7 @@ -{@render children?.(Object.assign(cell, cellComponents))} +{@render children?.(extendedCell)} diff --git a/packages/svelte-table/src/AppHeader.svelte b/packages/svelte-table/src/AppHeader.svelte index 99ba4f5cd8..1e838e4224 100644 --- a/packages/svelte-table/src/AppHeader.svelte +++ b/packages/svelte-table/src/AppHeader.svelte @@ -1,6 +1,7 @@ -{@render children?.(Object.assign(header, headerComponents))} +{@render children?.(extendedHeader)} diff --git a/packages/svelte-table/src/createTableHook.svelte.ts b/packages/svelte-table/src/createTableHook.svelte.ts index 79ea7153bc..6f1a709497 100644 --- a/packages/svelte-table/src/createTableHook.svelte.ts +++ b/packages/svelte-table/src/createTableHook.svelte.ts @@ -577,8 +577,8 @@ export function createTableHook< } // `` Object.assign-es `cellComponents` and `FlexRender` onto - // the same cell instance it puts in context, so this asserts the runtime - // shape. + // its cell and puts a live view of the current cell in context, so this + // asserts the runtime shape. return cell as unknown as Cell & TCellComponents & { FlexRender: typeof FlexRenderSvelte } } @@ -603,7 +603,7 @@ export function createTableHook< } // `` / `` Object.assign `headerComponents` - // and `FlexRender` onto the same header instance they put in context. + // and `FlexRender` onto their header and put a live view of it in context. return header as unknown as Header & THeaderComponents & { FlexRender: typeof FlexRenderSvelte } } @@ -650,26 +650,41 @@ export function createTableHook< // They only add props: each App*Svelte component sets its own context while // it initializes. Calling setContext here would run before that component // exists and write into the context of the component rendering it. + // Props are forwarded through getters: Svelte props are getters, and reading + // them here would pass the first value to the inner component forever. const AppTable = ((internal: any, props: any) => { - return AppTableSvelte(internal, { ...props, table }) + return AppTableSvelte(internal, { + get children() { + return props.children + }, + table, + }) }) as Component<{ children: Snippet }> - const AppCell = ((internal: any, { children, cell }: any) => { + const AppCell = ((internal: any, props: any) => { return AppCellSvelte(internal, { - cell, + get cell() { + return props.cell + }, cellComponents: cellComponentsWithFlexRender, - children, + get children() { + return props.children + }, }) }) as Component<{ cell: Cell children: Snippet<[any]> }> - const AppHeader = ((internal: any, { children, header }: any) => { + const AppHeader = ((internal: any, props: any) => { return AppHeaderSvelte(internal, { - header, + get header() { + return props.header + }, headerComponents: headerComponentsWithFlexRender, - children, + get children() { + return props.children + }, }) }) as Component<{ header: Header @@ -677,11 +692,15 @@ export function createTableHook< }> // AppFooter reuses AppHeaderSvelte (footers use Header type in table-core) - const AppFooter = ((internal: any, { children, header }: any) => { + const AppFooter = ((internal: any, props: any) => { return AppHeaderSvelte(internal, { - header, + get header() { + return props.header + }, headerComponents: headerComponentsWithFlexRender, - children, + get children() { + return props.children + }, }) }) as Component<{ header: Header diff --git a/packages/svelte-table/src/live-proxy.ts b/packages/svelte-table/src/live-proxy.ts new file mode 100644 index 0000000000..c3801e40a6 --- /dev/null +++ b/packages/svelte-table/src/live-proxy.ts @@ -0,0 +1,50 @@ +/** + * Creates a stable object that forwards every read to `getTarget()`. + * + * `App*` wrappers provide a cell or header through context once, while the + * instance behind them can change (e.g. `data` is replaced and a keyed + * `{#each}` reuses the component with a new cell). Reading `getTarget()` inside + * a reactive scope also tracks the source prop, so consumers update with it. + * + * Prototype methods are called with the current instance as `this`, so + * per-instance memos never see the proxy. The forwarders are cached per key so + * method identity stays stable across target changes. + */ +export function createLiveProxy(getTarget: () => T): T { + const forwarders = new Map) => any>() + + return new Proxy({} as T, { + get(_, key) { + const target = getTarget() + const value = Reflect.get(target, key, target) + if (typeof value !== 'function' || Object.hasOwn(target, key)) { + return value + } + let forwarder = forwarders.get(key) + if (!forwarder) { + forwarder = function (...args) { + const current = getTarget() as any + return current[key](...args) + } + forwarders.set(key, forwarder) + } + return forwarder + }, + set(_, key, value) { + return Reflect.set(getTarget(), key, value) + }, + has(_, key) { + return key in getTarget() + }, + ownKeys() { + return Reflect.ownKeys(getTarget()) + }, + getOwnPropertyDescriptor(_, key) { + const descriptor = Reflect.getOwnPropertyDescriptor(getTarget(), key) + return descriptor && { ...descriptor, configurable: true } + }, + getPrototypeOf() { + return Reflect.getPrototypeOf(getTarget()) + }, + }) +} diff --git a/packages/svelte-table/tests/fixtures/HookDataRefresh.svelte b/packages/svelte-table/tests/fixtures/HookDataRefresh.svelte new file mode 100644 index 0000000000..48eee23a27 --- /dev/null +++ b/packages/svelte-table/tests/fixtures/HookDataRefresh.svelte @@ -0,0 +1,47 @@ + + + + + +{#each table.getHeaderGroups()[0]!.headers as header (header.id)} + + + +{/each} + +{#each table.getRowModel().rows as row (row.id)} + {#each row.getAllCells() as cell (cell.id)} + + {#snippet children(value)} + {value.getValue()} + + {/snippet} + + {/each} +{/each} diff --git a/packages/svelte-table/tests/fixtures/HookHeaderLabel.svelte b/packages/svelte-table/tests/fixtures/HookHeaderLabel.svelte new file mode 100644 index 0000000000..a7fa47923a --- /dev/null +++ b/packages/svelte-table/tests/fixtures/HookHeaderLabel.svelte @@ -0,0 +1,10 @@ + + +header-label:{(header.column.columnDef.meta as { label: string }) + .label} diff --git a/packages/svelte-table/tests/rendering.test.ts b/packages/svelte-table/tests/rendering.test.ts index 85eacc86c1..a636fbd454 100644 --- a/packages/svelte-table/tests/rendering.test.ts +++ b/packages/svelte-table/tests/rendering.test.ts @@ -7,6 +7,7 @@ import { renderComponent } from '../src/render-component' import ContextFailure from './fixtures/ContextFailure.svelte' import FlexRenderHarness from './fixtures/FlexRenderHarness.svelte' import HookContextScope from './fixtures/HookContextScope.svelte' +import HookDataRefresh from './fixtures/HookDataRefresh.svelte' import HookHarness from './fixtures/HookHarness.svelte' import RenderBadge from './fixtures/RenderBadge.svelte' import { hook } from './fixtures/hook-fixture' @@ -220,6 +221,25 @@ describe('createTableHook', () => { expect(outputText('Contexts outside the wrappers')).toBe('') }) + test('keyed wrappers follow new cells and headers with the same id', async () => { + render(HookDataRefresh) + + expect(outputText('Snippet cell')).toBe('First') + expect(screen.getByText('cell-component:First')).toBeTruthy() + expect(screen.getByText('header-label:Before')).toBeTruthy() + + await fireEvent.click(screen.getByRole('button', { name: 'Replace data' })) + + expect(outputText('Snippet cell')).toBe('Second') + expect(screen.getByText('cell-component:Second')).toBeTruthy() + + await fireEvent.click( + screen.getByRole('button', { name: 'Replace columns' }), + ) + + expect(screen.getByText('header-label:After')).toBeTruthy() + }) + test.each([ ['useTableContext', () => hook.useTableContext()], ['useCellContext', () => hook.useCellContext()], diff --git a/packages/vue-table/src/createTableHook.ts b/packages/vue-table/src/createTableHook.ts index 96281a9a78..d220fb6aa6 100644 --- a/packages/vue-table/src/createTableHook.ts +++ b/packages/vue-table/src/createTableHook.ts @@ -1,6 +1,7 @@ import { createColumnHelper as coreCreateColumnHelper } from '@tanstack/table-core' -import { defineComponent, h, inject, provide } from 'vue' +import { computed, defineComponent, h, inject, provide } from 'vue' import { FlexRender } from './FlexRender' +import { createLiveProxy } from './live-proxy' import { mergeProxy } from './merge-proxy' import { useTable } from './useTable' import type { TableOptionsWithReactiveData, VueTable } from './useTable' @@ -439,7 +440,8 @@ export function createTableHook< } // `` Object.assign-es `cellComponents` and `FlexRender` onto - // the same cell instance it provides, so this asserts the runtime shape. + // its cell and provides a live view of the current cell, so this asserts + // the runtime shape. return cell as unknown as Cell & TCellComponents & { FlexRender: Component } } @@ -459,7 +461,7 @@ export function createTableHook< } // `` / `` Object.assign `headerComponents` - // and `FlexRender` onto the same header instance they provide. + // and `FlexRender` onto their header and provide a live view of it. return header as unknown as Header & THeaderComponents & { FlexRender: Component } } @@ -526,18 +528,25 @@ export function createTableHook< }, }, setup(props, { slots }) { - const cell = props.cell as Cell - - provide(CellContext, cell) - - const extendedCell = Object.assign(cell, { - FlexRender: CellFlexRender, - ...(cellComponents ?? {}), - }) as Cell & - TCellComponents & { FlexRender: Component } + // A keyed `v-for` reuses this component when `data` is replaced, passing + // a new cell with the same id. Read the prop reactively and provide a + // live view so consumers never keep the cell from the first render. + const extendedCell = computed( + () => + Object.assign(props.cell as Cell, { + FlexRender: CellFlexRender, + ...(cellComponents ?? {}), + }) as Cell & + TCellComponents & { FlexRender: Component }, + ) + + provide( + CellContext, + createLiveProxy(() => extendedCell.value), + ) return () => { - return slots.default?.({ cell: extendedCell }) + return slots.default?.({ cell: extendedCell.value }) } }, }) @@ -551,18 +560,22 @@ export function createTableHook< }, }, setup(props, { slots }) { - const header = props.header as Header - - provide(HeaderContext, header) - - const extendedHeader = Object.assign(header, { - FlexRender: HeaderFlexRender, - ...(headerComponents ?? {}), - }) as Header & - THeaderComponents & { FlexRender: Component } + const extendedHeader = computed( + () => + Object.assign(props.header as Header, { + FlexRender: HeaderFlexRender, + ...(headerComponents ?? {}), + }) as Header & + THeaderComponents & { FlexRender: Component }, + ) + + provide( + HeaderContext, + createLiveProxy(() => extendedHeader.value), + ) return () => { - return slots.default?.({ header: extendedHeader }) + return slots.default?.({ header: extendedHeader.value }) } }, }) @@ -576,18 +589,22 @@ export function createTableHook< }, }, setup(props, { slots }) { - const header = props.header as Header - - provide(HeaderContext, header) - - const extendedHeader = Object.assign(header, { - FlexRender: FooterFlexRender, - ...(headerComponents ?? {}), - }) as Header & - THeaderComponents & { FlexRender: Component } + const extendedHeader = computed( + () => + Object.assign(props.header as Header, { + FlexRender: FooterFlexRender, + ...(headerComponents ?? {}), + }) as Header & + THeaderComponents & { FlexRender: Component }, + ) + + provide( + HeaderContext, + createLiveProxy(() => extendedHeader.value), + ) return () => { - return slots.default?.({ header: extendedHeader }) + return slots.default?.({ header: extendedHeader.value }) } }, }) diff --git a/packages/vue-table/src/live-proxy.ts b/packages/vue-table/src/live-proxy.ts new file mode 100644 index 0000000000..9a972e15fb --- /dev/null +++ b/packages/vue-table/src/live-proxy.ts @@ -0,0 +1,50 @@ +/** + * Creates a stable object that forwards every read to `getTarget()`. + * + * `App*` wrappers provide a cell or header through context once, while the + * instance behind them can change (e.g. `data` is replaced and a keyed + * `v-for` reuses the component with a new cell). Reading `getTarget()` inside + * a reactive scope also tracks the source prop, so consumers update with it. + * + * Prototype methods are called with the current instance as `this`, so + * per-instance memos never see the proxy. The forwarders are cached per key so + * method identity stays stable across target changes. + */ +export function createLiveProxy(getTarget: () => T): T { + const forwarders = new Map) => any>() + + return new Proxy({} as T, { + get(_, key) { + const target = getTarget() + const value = Reflect.get(target, key, target) + if (typeof value !== 'function' || Object.hasOwn(target, key)) { + return value + } + let forwarder = forwarders.get(key) + if (!forwarder) { + forwarder = function (...args) { + const current = getTarget() as any + return current[key](...args) + } + forwarders.set(key, forwarder) + } + return forwarder + }, + set(_, key, value) { + return Reflect.set(getTarget(), key, value) + }, + has(_, key) { + return key in getTarget() + }, + ownKeys() { + return Reflect.ownKeys(getTarget()) + }, + getOwnPropertyDescriptor(_, key) { + const descriptor = Reflect.getOwnPropertyDescriptor(getTarget(), key) + return descriptor && { ...descriptor, configurable: true } + }, + getPrototypeOf() { + return Reflect.getPrototypeOf(getTarget()) + }, + }) +} diff --git a/packages/vue-table/tests/unit/rendering.test.ts b/packages/vue-table/tests/unit/rendering.test.ts index d90866dd73..6d510b99b8 100644 --- a/packages/vue-table/tests/unit/rendering.test.ts +++ b/packages/vue-table/tests/unit/rendering.test.ts @@ -342,6 +342,92 @@ describe('createTableHook', () => { ) }) + test('keyed wrappers follow new cells and headers with the same id', async () => { + function titleColumns(label: string) { + return columnHelper.columns([ + columnHelper.accessor('title', { header: 'Title', meta: { label } }), + ]) + } + + const CellConsumer = defineComponent({ + setup() { + const cell = hook.useCellContext() + return () => + h('output', { 'aria-label': 'Context cell' }, cell.getValue()) + }, + }) + const HeaderConsumer = defineComponent({ + setup() { + const header = hook.useHeaderContext() + return () => + h( + 'output', + { 'aria-label': 'Context header' }, + (header.column.columnDef.meta as { label: string }).label, + ) + }, + }) + const data = ref>([{ id: '1', title: 'First' }]) + const tableColumns = ref(titleColumns('Before')) + const Root = defineComponent({ + setup() { + const table = hook.useAppTable({ + data, + get columns() { + return tableColumns.value + }, + }) + + return () => [ + ...table + .getHeaderGroups()[0]! + .headers.map((header) => + h( + table.AppHeader, + { header, key: header.id }, + { default: () => h(HeaderConsumer) }, + ), + ), + ...table.getRowModel().rows.flatMap((row) => + row.getAllCells().map((cell) => + h( + table.AppCell, + { cell, key: cell.id }, + { + default: ({ cell: slotCell }: { cell: typeof cell }) => [ + h( + 'output', + { 'aria-label': 'Slot cell' }, + slotCell.getValue() as string, + ), + h(CellConsumer), + ], + }, + ), + ), + ), + ] + }, + }) + + render(Root) + + expect(outputText('Slot cell')).toBe('First') + expect(outputText('Context cell')).toBe('First') + expect(outputText('Context header')).toBe('Before') + + data.value = [{ id: '1', title: 'Second' }] + await nextTick() + + expect(outputText('Slot cell')).toBe('Second') + expect(outputText('Context cell')).toBe('Second') + + tableColumns.value = titleColumns('After') + await nextTick() + + expect(outputText('Context header')).toBe('After') + }) + test('bound cell render helpers preserve aggregate and placeholder modes', async () => { const mode = ref<'aggregate' | 'placeholder'>('aggregate') const CellConsumer = defineComponent({