diff --git a/.changeset/direct-dom-row-positioning.md b/.changeset/direct-dom-row-positioning.md new file mode 100644 index 00000000..55249cff --- /dev/null +++ b/.changeset/direct-dom-row-positioning.md @@ -0,0 +1,7 @@ +--- +'@tanstack/react-virtual': patch +--- + +fix(react-virtual): position `directDomUpdates` rows that mount without the owner re-rendering + +Rows were only positioned by the owner's layout effect or the next `onChange`. A row mounted by a child that re-renders on its own (local state, context, a resolved Suspense boundary) skipped both when it was fixed-size, and stayed unpositioned until the range changed. Rows are now positioned as they register through `measureElement`, and `containerRef` positions the rows that mounted together with the container. diff --git a/packages/react-virtual/src/index.tsx b/packages/react-virtual/src/index.tsx index 0cd07a7d..e4082c0f 100644 --- a/packages/react-virtual/src/index.tsx +++ b/packages/react-virtual/src/index.tsx @@ -9,7 +9,11 @@ import { observeWindowRect, windowScroll, } from '@tanstack/virtual-core' -import type { PartialKeys, VirtualizerOptions } from '@tanstack/virtual-core' +import type { + PartialKeys, + VirtualItem, + VirtualizerOptions, +} from '@tanstack/virtual-core' export * from '@tanstack/virtual-core' @@ -129,6 +133,27 @@ function useVirtualizerBase< } } + // Writes one item's main-axis position to its element. Idempotent — guarded + // by lastPositions. + const applyItemPosition = ( + instance: Virtualizer, + item: VirtualItem, + el: HTMLElement, + ) => { + const state = directRef.current + const horizontal = !!instance.options.horizontal + const next = item.start - instance.options.scrollMargin + if (state.lastPositions.get(el) === next) return + state.lastPositions.set(el, next) + if (state.mode === 'transform') { + el.style.transform = horizontal + ? `translate3d(${next}px, 0, 0)` + : `translate3d(0, ${next}px, 0)` + } else { + el.style[horizontal ? 'left' : 'top'] = `${next}px` + } + } + // Writes container size + item positions to the DOM. Idempotent — guarded // by lastSize / lastPositions. Called from onChange (covers scroll-driven // updates) and from a layout effect (covers post-render commits when refs @@ -141,24 +166,9 @@ function useVirtualizerBase< applyContainerSize(instance) - const horizontal = !!instance.options.horizontal - const useTransform = state.mode === 'transform' - const posAxis = horizontal ? 'left' : 'top' - const scrollMargin = instance.options.scrollMargin - const items = instance.getVirtualItems() - for (const item of items) { - const next = item.start - scrollMargin + for (const item of instance.getVirtualItems()) { const el = instance.elementsCache.get(item.key) as HTMLElement | undefined - if (!el) continue - if (state.lastPositions.get(el) === next) continue - state.lastPositions.set(el, next) - if (useTransform) { - el.style.transform = horizontal - ? `translate3d(${next}px, 0, 0)` - : `translate3d(0, ${next}px, 0)` - } else { - el.style[posAxis] = `${next}px` - } + if (el) applyItemPosition(instance, item, el) } } @@ -220,17 +230,27 @@ function useVirtualizerBase< } finally { measuringFromRef.current = false } + // A row can mount in a commit that does not include this component — a + // child re-rendering on its own state, context or a resolved Suspense + // boundary — which the `applyDirectStyles` effect below never sees. + // Position it as it registers; a row the effect does cover is skipped by + // `lastPositions`. + const state = directRef.current + if (node !== null && state.enabled && state.container) { + const item = v.measurementsCache[v.indexFromElement(node)] + if (item) applyItemPosition(v, item, node as unknown as HTMLElement) + } } return Object.assign(v, { containerRef: (node: HTMLElement | null) => { const state = directRef.current state.container = node state.lastSize = null - if (node && state.enabled) { - const total = v.getTotalSize() - state.lastSize = total - const axis = v.options.horizontal ? 'width' : 'height' - node.style[axis] = `${total}px` + // Sizes the container, and positions the rows that mounted with it: + // React attaches children's refs before their parent's, so they have + // already registered but found no container to be positioned in. + if (node !== null) { + applyDirectStyles(v) } }, }) diff --git a/packages/react-virtual/tests/index.test.tsx b/packages/react-virtual/tests/index.test.tsx index ca54a5db..f729392e 100644 --- a/packages/react-virtual/tests/index.test.tsx +++ b/packages/react-virtual/tests/index.test.tsx @@ -1,8 +1,9 @@ import { beforeEach, test, expect, vi } from 'vitest' import * as React from 'react' -import { render, screen } from '@testing-library/react' +import { act, render, screen } from '@testing-library/react' import { useVirtualizer, Range } from '../src/index' +import type { ReactVirtualizer } from '../src/index' beforeEach(() => { Object.defineProperties(HTMLElement.prototype, { @@ -207,3 +208,136 @@ test('should not flushSync while measuring an item from its ref callback', () => errorSpy.mockRestore() }) + +// `directDomUpdates` rows rendered by a child component that re-renders on its +// own — local state, context, a resolved Suspense boundary — commit without the +// owner, so the owner's layout effect never positions them. With fixed-size +// rows, measuring them does not notify either. +function DirectList({ + children, +}: { + children: ( + virtualizer: ReactVirtualizer, + ) => React.ReactNode +}) { + const parentRef = React.useRef(null) + + const virtualizer = useVirtualizer({ + count: 10, + getScrollElement: () => parentRef.current, + estimateSize: () => 50, + observeElementRect: (_, cb) => cb({ height: 200, width: 200 }), + measureElement: () => 50, + directDomUpdates: true, + }) + + return ( +
+ {children(virtualizer)} +
+ ) +} + +const DirectRows = React.memo(function DirectRows({ + virtualizer, + show, +}: { + virtualizer: ReactVirtualizer + show: (setVisible: (visible: boolean) => void) => void +}) { + const [visible, setVisible] = React.useState(false) + show(setVisible) + + return ( +
+ {virtualizer + .getVirtualItems() + .filter((item) => visible || item.index !== 2) + .map((item) => ( +
+ ))} +
+ ) +}) + +test('directDomUpdates positions a row a child mounts without the owner', () => { + let setVisible: (visible: boolean) => void = () => {} + + render( + + {(virtualizer) => ( + (setVisible = set)} + /> + )} + , + ) + + expect(screen.getByTestId('item-1')).toHaveStyle({ + transform: 'translate3d(0, 50px, 0)', + }) + expect(screen.queryByTestId('item-2')).not.toBeInTheDocument() + + act(() => setVisible(true)) + + expect(screen.getByTestId('item-2')).toHaveStyle({ + transform: 'translate3d(0, 100px, 0)', + }) +}) + +const DirectContainer = React.memo(function DirectContainer({ + virtualizer, + show, +}: { + virtualizer: ReactVirtualizer + show: (setVisible: (visible: boolean) => void) => void +}) { + const [visible, setVisible] = React.useState(false) + show(setVisible) + + if (!visible) return null + + return ( +
+ {virtualizer.getVirtualItems().map((item) => ( +
+ ))} +
+ ) +}) + +test('directDomUpdates positions rows a child mounts together with the container', () => { + let setVisible: (visible: boolean) => void = () => {} + + render( + + {(virtualizer) => ( + (setVisible = set)} + /> + )} + , + ) + + // React attaches the rows' refs before the container's, so they register + // before there is a container to position them in. + act(() => setVisible(true)) + + expect(screen.getByTestId('item-3')).toHaveStyle({ + transform: 'translate3d(0, 150px, 0)', + }) +})