Repository navigation
fix(virtual-list): keep the ref handle stable across scroll renders - #151
Merged
Merged
Conversation
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
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.
VirtualList's
useImperativeHandlehad no deps, so the handle got rebuilt on every commit, scroll steps included. When React rebuilds a handle it nulls the ref in the mutation phase and only re-attaches it in VirtualList's own layout phase, which runs after the cells' layout effects. So a cell recycled onto a new row in that commit callsgetLayout(index)through the parent's ref, gets null, and its seed never gets written. In the Plex app that cell then keeps the previous row's seed (or none), computes the wrong section expansion and writes a bogus 504px reserve, which shifts the focused Home row ~1000px and makes fast Up skip a row (10 9 8 7 6 4 3 2 1, every run).Fix is one handle for the list's lifetime (
useImperativeHandle(ref, ..., [])). The methods read visibleRange, itemAreaOffset, paddingCross, horizontal and the scroll functions through a ref that's updated in auseInsertionEffect. Insertion effects run before any layout effect in the commit, so a cell asking from its layout effect already gets this render's range and offsets (a layout effect would hand it the previous render's). Public API is unchanged.Also pulled the bare react-reconciler host from the useContentCross spec into
createTestRenderer.tsso the new VirtualList spec can share it.Verified:
Not run on a real TV or on native platforms.