fix(virtual-core): ignore stale connected measurements - #1246
Merged
piecyk merged 4 commits intoJul 31, 2026
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughVirtualizer measurement paths now ignore indexes outside the current item count. A regression test covers stale refs and delayed ResizeObserver callbacks after reducing ChangesStale measurement handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
piecyk
reviewed
Jul 30, 2026
Co-authored-by: Damian Pieczynski <piecyk@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/virtual-core/src/index.ts`:
- Around line 492-495: Update the conditional in the surrounding index-handling
method to wrap the isIndexInRange(index) expression in parentheses, making the
if statement valid TypeScript while preserving its existing early-return
behavior.
🪄 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 Plus
Run ID: 81b0b608-79f1-4e25-a759-8d899a35c5e8
📒 Files selected for processing (1)
packages/virtual-core/src/index.ts
|
View your CI Pipeline Execution ↗ for commit 3160b99
☁️ Nx Cloud last updated this comment at |
Collaborator
|
Thanks @LeoLB-Wang |
Merged
geosan11
added a commit
to geosan11/EHI-MULTISYSTEMS
that referenced
this pull request
Aug 24, 2026
…h source The shiftsToMark (9f5d16f) and fetchInitial reference-churn (4553763) fixes were both correct and worthwhile, but the Master Ledger crash ("Maximum update depth exceeded" looping through measureElement -> resizeItem -> notify -> onChange) kept recurring in production after both shipped. Root cause was upstream of this app entirely: @tanstack/react-virtual 3.14.3 (virtual-core 3.17.1) has a bug in its ResizeObserver callback. Read node_modules/@tanstack/virtual-core/dist/esm/index.js directly -- the callback's connected-node branch computed `index = this.indexFromElement(node)` and immediately called measureElement/ resizeItem with no check that the index was still within the virtualizer's CURRENT item count. When a legitimate data update shrinks displayEntries while an old DOM node is still connected and mid- measurement, this measures/resizes against a stale index, producing a wrong delta that triggers another notify -> re-render -> another wrong measurement, in a tight synchronous loop -- matching "Maximum update depth exceeded" far better than ordinary scrolling, and firing on LEGITIMATE data changes, not just spurious re-fetches. That's exactly why the previous two fixes (both about eliminating spurious churn) reduced but never eliminated the crash: a busy multi-hub admin session has some row change somewhere within most 5-minute windows regardless. This exact gap was closed upstream in TanStack/virtual#1246 ("ignore connected measurement nodes outside current item count"), shipped in 3.14.10 (virtual-core 3.17.8) -- already within this project's existing ^3.14.3 semver range. Bumped package.json's floor to match. Verified the fix actually landed in what got installed: the ResizeObserver callback now has `if (!this.isIndexInRange(index)) return;` right before the measureElement/resizeItem calls that were previously unguarded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
2 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
Why
#1148 safely handles disconnected stale nodes. A node can also remain connected while React applies a count shrink; its old data-index can then reach getItemKey, which commonly indexes application data and may throw.
Testing
Related
getItemKeyis called with invalid index #580getItemKeywith stale index for removed node #1147Checklist
Summary by CodeRabbit
Bug Fixes
Tests