Skip to content

fix(virtual-core): scroll to index should only retry if still targeting the same index#1073

Closed
jb-chief wants to merge 2 commits into
TanStack:mainfrom
jb-chief:main
Closed

fix(virtual-core): scroll to index should only retry if still targeting the same index#1073
jb-chief wants to merge 2 commits into
TanStack:mainfrom
jb-chief:main

Conversation

@jb-chief

Copy link
Copy Markdown

🎯 Changes

When calling scrollToIndex in a tight loop, it no longer retries scrolls for outdated indices.

✅ Checklist

  • I have followed the steps listed in the Contributing guide.
  • I have tested and linted this code locally.
  • I have generated a changeset for this PR, or this PR should not release a new version.

@changeset-bot

changeset-bot Bot commented Oct 30, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0a64098

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@tanstack/virtual-core Patch
@tanstack/angular-virtual Patch
@tanstack/lit-virtual Patch
@tanstack/react-virtual Patch
@tanstack/solid-virtual Patch
@tanstack/svelte-virtual Patch
@tanstack/vue-virtual Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@piecyk piecyk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jb-chief , I’ve added a comment suggesting renaming scrollToIndexTargetIndex to currentScrollToIndex for clarity, and pointed out a missing return in one condition. After this, we should be good to merge.

scrollOffset: number | null = null
scrollDirection: ScrollDirection | null = null
private scrollAdjustments = 0
private scrollToIndexTargetIndex: number | null = null

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private scrollToIndexTargetIndex: number | null = null
private currentScrollToIndex: number | null = null

const scheduleRetry = (align: ScrollAlignment) => {
if (!this.targetWindow) return

if (this.scrollToIndexTargetIndex !== index)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (this.scrollToIndexTargetIndex !== index)
if (this.scrollToIndexTargetIndex !== index) return

@piecyk

piecyk commented Jan 3, 2026

Copy link
Copy Markdown
Collaborator

@jb-chief Thank you for the contribution!
After reviewing this PR, I implemented the same changes directly, when fixing scrollToIndex in #1106. As the changes are now included, I’ll be closing this PR. Much appreciated!

@piecyk piecyk closed this Jan 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants