fix(virtual-core): guard against null targetWindow in scrollToIndex rAF callback#1129
Merged
piecyk merged 2 commits intoTanStack:mainfrom Feb 23, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: c17d70b The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
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 |
ansmonjol
force-pushed
the
fix/scrollToIndex-null-targetWindow
branch
from
8271ba8 to
db3c5ad
Compare
…AF callback When a component unmounts while scrollToIndex is in progress, the cleanup method sets targetWindow to null. However, the already-scheduled requestAnimationFrame callback in tryScroll still fires and accesses this.targetWindow without a null check, causing "Cannot read properties of null (reading 'requestAnimationFrame')". Add an early return guard at the top of the rAF callback to bail out if targetWindow has been cleaned up.
ansmonjol
force-pushed
the
fix/scrollToIndex-null-targetWindow
branch
from
db3c5ad to
ff6be66
Compare
stephenlago99
approved these changes
Feb 20, 2026
piecyk
requested changes
Feb 23, 2026
Collaborator
piecyk
left a comment
piecyk
left a comment
There was a problem hiding this comment.
Looks good, let's remove the unsafe exclamation mark
4 tasks
|
View your CI Pipeline Execution ↗ for commit c17d70b
☁️ Nx Cloud last updated this comment at |
Co-authored-by: Damian Pieczynski <piecyk@gmail.com>
Contributor
Author
|
Make sense, change is applied @piecyk |
piecyk
approved these changes
Feb 23, 2026
Merged
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
3 participants
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.
🎯 Changes
When a component unmounts while
scrollToIndexis in progress, thecleanupmethod setstargetWindowtonull. However, the already-scheduledrequestAnimationFramecallback insidetryScrollstill fires and accessesthis.targetWindowwithout a null check, causing:The
scheduleRetryfunction already guards against this (if (!this.targetWindow) return), but the rAF callback scheduled intryScrolldoes not — and it uses a non-null assertion (this.targetWindow!) to schedule a second frame in dynamic mode.Fix: Add an early return at the top of the rAF callback to bail out if
targetWindowhas been cleaned up between frames.Test: Added a unit test that reproduces the crash by triggering
scrollToIndexin dynamic mode, then simulating unmount before flushing the rAF callbacks. Confirms the test fails without the fix and passes with it.✅ Checklist
pnpm run test:pr.🚀 Release Impact