feat: support end-anchored virtualizers#1173
Merged
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds end-anchored virtualization and chat/log controls: new Virtualizer options ( ChangesEnd-Anchored Virtualization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/virtual-core/tests/index.test.tsParsing error: "parserOptions.project" has been provided for Comment |
Contributor
🚀 Changeset Version Preview1 package(s) bumped directly, 7 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 2a80e00
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
examples/react/chat/README.md (1)
1-6: ⚡ Quick winConsider adding a brief description of what this example demonstrates.
The README is functional but could benefit from a sentence or two explaining that this example showcases end-anchored virtualization for chat-style UIs, including prepending history, streaming messages, and auto-follow behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/react/chat/README.md` around lines 1 - 6, Update the README.md for the chat example to include a short descriptive paragraph explaining what the example demonstrates: that it showcases end-anchored virtualization for chat-style UIs, supports prepending history, streaming messages, and automatic follow/autoscroll behavior; place this sentence or two near the top under the title in examples/react/chat/README.md so developers immediately understand the key features shown by the example.packages/virtual-core/src/index.ts (1)
646-654: 💤 Low valueConsider caching key-to-index mapping for large lists.
findIndexByKeyperforms O(n) linear search. While this is only called during anchor restoration (not per frame), it could become noticeable with very large lists (10k+ items) during rapid prepend operations.For now this is acceptable since anchor restoration is infrequent, but if performance becomes an issue, consider maintaining a
Map<Key, number>index during measurement builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/virtual-core/src/index.ts` around lines 646 - 654, findIndexByKey currently does an O(n) scan over this.options.count using this.options.getItemKey which can be slow for very large lists; to fix, introduce a Map<Key, number> (e.g., this.keyToIndex) that is populated during measurement/build phases (the same place where item sizes/indices are computed) and used by findIndexByKey to return index in O(1); ensure the map is maintained/updated on any mutations that change indices (prepend/insert/remove/clear) or invalidated and rebuilt when measurements are recomputed so the mapping stays in sync with count and getItemKey.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@examples/react/chat/README.md`:
- Around line 1-6: Update the README.md for the chat example to include a short
descriptive paragraph explaining what the example demonstrates: that it
showcases end-anchored virtualization for chat-style UIs, supports prepending
history, streaming messages, and automatic follow/autoscroll behavior; place
this sentence or two near the top under the title in
examples/react/chat/README.md so developers immediately understand the key
features shown by the example.
In `@packages/virtual-core/src/index.ts`:
- Around line 646-654: findIndexByKey currently does an O(n) scan over
this.options.count using this.options.getItemKey which can be slow for very
large lists; to fix, introduce a Map<Key, number> (e.g., this.keyToIndex) that
is populated during measurement/build phases (the same place where item
sizes/indices are computed) and used by findIndexByKey to return index in O(1);
ensure the map is maintained/updated on any mutations that change indices
(prepend/insert/remove/clear) or invalidated and rebuilt when measurements are
recomputed so the mapping stays in sync with count and getItemKey.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0382c210-0433-4cc5-9310-0a7845749778
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.changeset/chat-reverse-virtualization.mddocs/api/virtualizer.mdexamples/react/chat/.gitignoreexamples/react/chat/README.mdexamples/react/chat/index.htmlexamples/react/chat/package.jsonexamples/react/chat/src/index.cssexamples/react/chat/src/main.tsxexamples/react/chat/tsconfig.jsonexamples/react/chat/vite.config.jspackages/react-virtual/e2e/app/chat/index.htmlpackages/react-virtual/e2e/app/chat/main.tsxpackages/react-virtual/e2e/app/test/chat.spec.tspackages/react-virtual/e2e/app/vite.config.tspackages/react-virtual/playwright.config.tspackages/virtual-core/src/index.tspackages/virtual-core/tests/index.test.ts
tannerlinsley
force-pushed
the
codex/reverse-chat-virtualization
branch
from
76f1131 to
34bc57b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-virtual/e2e/app/test/chat.spec.ts (1)
53-54: ⚡ Quick winReplace fixed sleeps with condition-based waits.
The hard-coded waits can make this spec flaky under variable CI load. Prefer
expect.poll(...)/locator-based waits tied to scroll/message state transitions instead ofwaitForTimeout(100).♻️ Suggested direction
- await page.waitForTimeout(100) + await expect + .poll(async () => { + const current = await firstVisibleMessage(page) + return current.id + }) + .toBeDefined() ... - await page.waitForTimeout(100) + await expect + .poll(async () => + page.evaluate(() => { + const container = document.querySelector('`#scroll-container`') + if (!container) throw new Error('Container not found') + return container.scrollTop + }), + ) + .toBeCloseTo(before, 0)Also applies to: 58-59, 78-79, 87-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-virtual/e2e/app/test/chat.spec.ts` around lines 53 - 54, Replace the brittle page.waitForTimeout(100) calls with condition-based waits: locate the chat scroll container or message list and wait for the expected state using Playwright's expect/locator APIs (e.g., await expect(locator('selector-for-messages')).toHaveCount(expected) or await expect.poll(() => locator('selector-for-scroll').evaluate(...)).toMatch(...)); in other words, swap each page.waitForTimeout(100) (and the similar occurrences at 58-59, 78-79, 87-88) with a targeted wait that asserts the message/scroll state change (use expect.poll for custom predicates or locator.waitFor/expect.toHaveText/toHaveCount for standard conditions) so the spec waits for actual UI transitions instead of a fixed sleep.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/react-virtual/e2e/app/test/chat.spec.ts`:
- Around line 53-54: Replace the brittle page.waitForTimeout(100) calls with
condition-based waits: locate the chat scroll container or message list and wait
for the expected state using Playwright's expect/locator APIs (e.g., await
expect(locator('selector-for-messages')).toHaveCount(expected) or await
expect.poll(() => locator('selector-for-scroll').evaluate(...)).toMatch(...));
in other words, swap each page.waitForTimeout(100) (and the similar occurrences
at 58-59, 78-79, 87-88) with a targeted wait that asserts the message/scroll
state change (use expect.poll for custom predicates or
locator.waitFor/expect.toHaveText/toHaveCount for standard conditions) so the
spec waits for actual UI transitions instead of a fixed sleep.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92aeb2de-aff1-4b1e-9e84-edc04c0b0d9d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.changeset/chat-reverse-virtualization.mddocs/api/virtualizer.mdexamples/react/chat/.gitignoreexamples/react/chat/README.mdexamples/react/chat/index.htmlexamples/react/chat/package.jsonexamples/react/chat/src/index.cssexamples/react/chat/src/main.tsxexamples/react/chat/tsconfig.jsonexamples/react/chat/vite.config.jspackages/react-virtual/e2e/app/chat/index.htmlpackages/react-virtual/e2e/app/chat/main.tsxpackages/react-virtual/e2e/app/test/chat.spec.tspackages/react-virtual/e2e/app/vite.config.tspackages/react-virtual/playwright.config.tspackages/virtual-core/src/index.tspackages/virtual-core/tests/index.test.ts
✅ Files skipped from review due to trivial changes (3)
- examples/react/chat/package.json
- examples/react/chat/README.md
- .changeset/chat-reverse-virtualization.md
tannerlinsley
force-pushed
the
codex/reverse-chat-virtualization
branch
from
34bc57b to
bc6a315
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/react/chat/src/main.tsx`:
- Around line 123-133: The current useLayoutEffect sets didInitialScroll and
also starts a timeout for setAutoHistoryEnabled, but React will run the cleanup
immediately when didInitialScroll changes so the timeout is cleared before
firing; fix by separating responsibilities: keep the scroll logic in the
existing React.useLayoutEffect that calls virtualizer.scrollToEnd() and
setDidInitialScroll(true) (leave that effect dependent on didInitialScroll and
virtualizer), then add a separate React.useEffect with an empty dependency array
that schedules the window.setTimeout(() => setAutoHistoryEnabled(true), 250) and
returns a cleanup that clears that timeout on unmount; alternatively, delay
calling setDidInitialScroll until after the timeout completes—use setTimeout to
call setAutoHistoryEnabled(true) then setDidInitialScroll(true) if you prefer
the single-effect approach.
- Line 2: Change the default import to the named createRoot from
'react-dom/client' and update the app bootstrap to use
createRoot(...).render(...). In the component using useLayoutEffect, prevent
didInitialScroll from being set synchronously (which cancels the pending
timeout) by moving the setDidInitialScroll call into the timeout callback after
setAutoHistoryEnabled(true) (or call setAutoHistoryEnabled(true) first, then
setDidInitialScroll(true) inside the same timeout), and ensure the cleanup still
clears the timeout; refer to createRoot, useLayoutEffect, didInitialScroll,
setAutoHistoryEnabled and setDidInitialScroll to locate the changes.
In `@packages/react-virtual/e2e/app/test/chat.spec.ts`:
- Line 53: Replace the fragile fixed sleeps (calls to page.waitForTimeout(100))
in chat.spec.ts with state-based waits: after each scroll or click, use
Playwright's expect.poll or expect.toHaveJSProperty against the chat scroll
container (e.g., check its scrollTop/scrollHeight or the relevant element's
visibility) until the expected value/visibility appears; specifically locate the
occurrences of page.waitForTimeout in chat.spec.ts and swap them for
expect.poll(() =>
document.querySelector('<scroll-selector>').scrollTop).toBeGreaterThan(...) or
expect(locator('<item-selector>')).toBeVisible() so the test waits for the
actual UI state instead of a fixed timeout.
In `@packages/virtual-core/src/index.ts`:
- Around line 1527-1536: isAtEnd()/getDistanceFromEnd() use getTotalSize() -
getSize() instead of the actual DOM max scroll offset, so a DOM scroll extent
larger than virtual size can prevent end detection; update getDistanceFromEnd to
compute distance using getMaxScrollOffset() - this.getScrollOffset() (clamped to
>=0) and keep isAtEnd(threshold = this.options.scrollEndThreshold) comparing
that result to threshold; modify references in getDistanceFromEnd, isAtEnd and
any callers relying on the old computation (functions: getDistanceFromEnd,
isAtEnd, getMaxScrollOffset, getScrollOffset, getTotalSize, getSize) to ensure
followOnAppend/end-pinned behavior works with the real max scroll offset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 671aaa81-a34f-42cb-a74f-dfe76b4598c8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.changeset/chat-reverse-virtualization.mddocs/api/virtualizer.mddocs/chat.mddocs/config.jsondocs/introduction.mdexamples/react/chat/.gitignoreexamples/react/chat/README.mdexamples/react/chat/index.htmlexamples/react/chat/package.jsonexamples/react/chat/src/index.cssexamples/react/chat/src/main.tsxexamples/react/chat/tsconfig.jsonexamples/react/chat/vite.config.jspackages/react-virtual/e2e/app/chat/index.htmlpackages/react-virtual/e2e/app/chat/main.tsxpackages/react-virtual/e2e/app/test/chat.spec.tspackages/react-virtual/e2e/app/vite.config.tspackages/react-virtual/playwright.config.tspackages/virtual-core/src/index.tspackages/virtual-core/tests/index.test.ts
✅ Files skipped from review due to trivial changes (7)
- packages/react-virtual/e2e/app/chat/index.html
- examples/react/chat/README.md
- docs/introduction.md
- examples/react/chat/package.json
- examples/react/chat/.gitignore
- packages/react-virtual/playwright.config.ts
- .changeset/chat-reverse-virtualization.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
anchorTo: 'end'for chat-style prepend stability and bottom-pinned resize behaviorexamples/react/chatdemo for streaming, appending, and loading older messagesBundle Size
@tanstack/virtual-coreESM dist: +747 B gzip versusorigin/main@tanstack/react-virtual: 0 B deltaTesting
pnpm --filter @tanstack/virtual-core test:lib -- --runpnpm --filter @tanstack/virtual-core test:typespnpm --filter @tanstack/virtual-core test:eslintpnpm --filter @tanstack/virtual-core buildpnpm --filter @tanstack/virtual-core test:buildpnpm --filter @tanstack/react-virtual test:typespnpm --filter @tanstack/react-virtual test:eslintpnpm --filter @tanstack/react-virtual buildpnpm --filter @tanstack/react-virtual test:buildpnpm --filter tanstack-react-virtual-example-chat buildpnpm run test:docspnpm run test:sherifpnpm run test:knippnpm exec tsc -p packages/react-virtual/e2e/app/tsconfig.json --noEmitVITE_SERVER_PORT=5175 pnpm --dir packages/react-virtual exec playwright test --workers=1VITE_SERVER_PORT=5177 pnpm --dir packages/react-virtual exec playwright test app/test/chat.spec.ts --workers=1Note: default parallel React e2e showed intermittent timeouts in pre-existing smooth-scroll specs under load; the affected specs passed in isolation and the full suite passed serially.
Summary by CodeRabbit
New Features
Documentation
Examples
Tests