Repository navigation
[BUGFIX] stabilize VirtualizedTable component references to prevent remounts - #358
adrianSepiol wants to merge 2 commits into
Conversation
e95b89a to
a2cc56a
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The remount regression needs focused automated coverage to prevent recurrence.
1 open finding
What changed in this PR
Stabilizes VirtualizedTable internals to prevent child remounts and state loss.
Changes:
- Moves Virtuoso components to module scope and supplies dynamic values through context.
- Stabilizes keyboard-navigation callbacks with
useCallback. - Uses row data directly through Virtuoso’s
dataprop.
| File | Description |
|---|---|
components/src/Table/VirtualizedTable.tsx |
Stabilizes Virtuoso component identities and passes render context. |
components/src/Table/hooks/useVirtualizedTableKeyboardNav.tsx |
Memoizes the active-cell callback. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| data={rows} | ||
| components={VirtuosoTableComponents as TableComponents<Row<TableData>, TableContext<TableData>>} | ||
| context={tableContext} |
…emounts Signed-off-by: Adrian Sepiół <a.sepiol@sap.com>
a2cc56a to
07c4473
Compare
| // Defined outside VirtualizedTable so the object reference is stable and | ||
| // TableVirtuoso never remounts its internal components due to a changed prop. | ||
| // Props that vary per-render are passed via the `context` prop instead. | ||
| const VirtuosoTableComponents: TableComponents<Row<unknown>, TableContext<unknown>> = { |
There was a problem hiding this comment.
context is "leaking" on TableHead, TableFoot and TableBody components, because it's passed by default (where on others, it's separated). So the scroller div, thead, tbody and tfoot (when paginated) now render context="[object Object]"
There was a problem hiding this comment.
Once it's fixed, this PR is a nice improvement for the rendering :)
There was a problem hiding this comment.
Good catch :) I've pushed fix
Signed-off-by: Adrian Sepiół <a.sepiol@sap.com>

Description
Fixes: perses/perses#4571
The fix moves
VirtuosoTableComponentsout of the component entirely as a module-level constant, giving it a permanently stable reference. Per-render values (width,density, keyboard handlers, row event handlers) are threaded through via Virtuoso'scontextprop and atableContextmemo instead of closing over them.Additionally,
useVirtualizedTableKeyboardNavnow wrapsonActiveCellChangeinuseCallbackso the returned handler references are stable when its inputs haven't changed, which was the other source of memo churn.This fix also enables stateful components to be used inside the table (e.g. a filter dropdown that tracks its own
openstate) — previously, remounting on every render would silently reset any local state those components held.Screenshots
Checklist
[<catalog_entry>] <commit message>naming convention using one of thefollowing
catalog_entryvalues:FEATURE,ENHANCEMENT,BUGFIX,BREAKINGCHANGE,DOC,IGNORE.UI Changes
See e2e docs for more details. Common issues include: