T
TanStackโ€ข3y ago
fair-rose

why does the component re-renders on each scroll change

it seems that the hook returns a new virtualizer and new virtual items (deeply equal to the prev ones) each time the scroll offset changes. It forces me to do very weird things to avoid over-rendering the component. the only thing that I can think of that actually changes is the scroll offset, which is now returned on the virtualizer as a prop
26 Replies
genetic-orange
genetic-orangeโ€ข3y ago
virtualizer is same, are you passing getItemKey as inline function? This will invalidate the measurements creating new virtual items each re-render
fair-rose
fair-roseOPโ€ข3y ago
virtualizer is the same but you are force rendering, if isScrolling changed or if range changed. (isScrolling changes quite often, not sure that it's a good reason to render, I can get it from the DOM) the different ref for same items is forcing me to compare them
genetic-orange
genetic-orangeโ€ข3y ago
items ref changed, but they are the same
That is true, new items are created each call of getVirtualItems but the are same, if you compare each one. Overall this will not be always true, as for example if item size will change at index 3, every element will have different ref even if they didn't change.
looking at your code, you are force rendering onChange: (instance) => { rerender() options.onChange?.(instance) },
yes we need to re-render if the range changes, this how react works If you have expensive children, you should memo it on item level, having extra div is better that trying to bend react
{items.map(item => (
<div key={item.key} data-index={item.index} ref={virtualizer.measureElement}>
<MemoItem data={rows[item.index]} />
</div>
))}
{items.map(item => (
<div key={item.key} data-index={item.index} ref={virtualizer.measureElement}>
<MemoItem data={rows[item.index]} />
</div>
))}
fair-rose
fair-roseOPโ€ข3y ago
new children make perfect sense to re-rerender, isScrolling not sure that is worth it.. about the items, if anything changed in the items I expect a new reference. if nothing changed it will be nice to get the same one. about the getItemKey it seems you are using the functions passed in the memo so the user should never pass inline functions - it's important for people to know ๐Ÿ™‚
genetic-orange
genetic-orangeโ€ข3y ago
isScrolling not sure that is worth it..
in the end it's 2 re-renders, for some cases it's important, like disable pointer event's when scrolling.
I can get it from the DOM
no, you can't, without custom code.
if nothing changed it will be nice to get the same one.
ooo checked the code and items should be same if they don't change ๐Ÿ‘
it seems you are using the functions passed in the memo
yes but not for all, getScrollElement, estimateSize are fine as inline functions, but getItemKey will re-create items on every re-render
fair-rose
fair-roseOPโ€ข3y ago
ooo checked the code and items should be same if they don't change
in my sandbox I get different array refs for equal item arrays. did you change anything? it's true I can't get isScrolling without custom code, I think that if you're doing it to be nice add registration to scrollingChanged. in general, it is strange that there is no correlation between renders and virtualizer properties, for example, you added the scrollOffset as a property and you won't re-render each time that changes. but getTotalSize and getVirtualItems are functions and it will re-render rangeExtractor, getItemKey are part of memo anyway, thank you for the detailed answer, I guess I will wrap the virtualizer to have more control over this (to see what I'm talking about scroll slowly, so that items won't really change)
it is strange that there is no correlation between renders and virtualizer properties
if we are on the topic, in react you assume that if the virtualizer is the cause of the re-render you'd get a different virtualizer instance
genetic-orange
genetic-orangeโ€ข3y ago
ooo checked the code and items should be same if they don't change in my sandbox I get different array refs for equal item arrays. did you change anything?
array refs will different each call for getVirtualItems, as we create this array every time https://github.com/TanStack/virtual/blob/beta/packages/virtual-core/src/index.ts#L603-L613 what will be same are items if nothing changed that would re-create the measurements
in general, it is strange that there is no correlation between renders and virtualizer properties, for example, you added the scrollOffset as a property and you won't re-render each time that changes. but getTotalSize and getVirtualItems are functions and it will re-render
calling those function will not trigger re-render, virtualizer is optimiazed limit those, that's why we only re-render when start/stop scrolling, range changes, items size changes or size of virtualizer container changes
fair-rose
fair-roseOPโ€ข3y ago
calling those function will not trigger re-render, virtualizer is optimiazed limit those, that's why we only re-render when start/stop scrolling, range changes, items size changes or size of virtualizer container changes
this wasn't what I meant. I didn't think calling the function will trigger re-render
genetic-orange
genetic-orangeโ€ข3y ago
this wasn't what I meant. I didn't think calling the function will trigger re-render
sorry but now i'm confused ๐Ÿ˜„ as you wrote
but getTotalSize and getVirtualItems are functions and it will re-render
fair-rose
fair-roseOPโ€ข3y ago
what I meant, is that react is generally data-driven. it assumes that : if ref is the same -> value is the same if ref is different -> value is different and it's kind of mixed in the virtualizer. but it's a big change ๐Ÿ™‚
genetic-orange
genetic-orangeโ€ข3y ago
kinda, here you can assume that if ref is the same -> value is the same or different ๐Ÿ˜‰ if ref is different -> value is different
fair-rose
fair-roseOPโ€ข3y ago
if ref is different -> value is different
unless we are talking about the virtualItems array ๐Ÿ™‚ I'm starting to get a slight desire to try and rewrite the code. but, you are not at a stage to change the API that much even if I had the time ๐Ÿ™‚
genetic-orange
genetic-orangeโ€ข3y ago
i think here the biggest key for you is that getVirtualItems will return new ref every time, but items could be same if we didn't rebuild the measurements from scratch point here is to make them stale, we would need to keep prev values and that is not free as with every memoization, and there is no clear win here to keep those in memory
fair-rose
fair-roseOPโ€ข3y ago
I know and the re-render on isScrolling change ๐Ÿ™‚
genetic-orange
genetic-orangeโ€ข3y ago
I'm starting to get a slight desire to try and rewrite the code. but, you are not at a stage to change the API that much even if I had the time ๐Ÿ™‚
feel free to do it, we are always open for discussion ๐Ÿ™‚
fair-rose
fair-roseOPโ€ข3y ago
what I imagine is not a small change to the API, maybe I'll have the time, who knows.. ๐Ÿ™‚
genetic-orange
genetic-orangeโ€ข3y ago
if we are on the topic, in react you assume that if the virtualizer is the cause of the re-render you'd get a different virtualizer instance
No, for example having a wrapper over resize observer that trigger re-render, should we get new resize observer instance every time ๐Ÿ˜‰
fair-rose
fair-roseOPโ€ข3y ago
depends what the wrapper returns, I'd assume it returns the sizes -> then you get new sizes, yes
genetic-orange
genetic-orangeโ€ข3y ago
don't agree, instance is not bound directly with rendering so imho not
fair-rose
fair-roseOPโ€ข3y ago
did not understand that
genetic-orange
genetic-orangeโ€ข3y ago
you don't render instance <div>{instance}<div>
fair-rose
fair-roseOPโ€ข3y ago
who is instance?
genetic-orange
genetic-orangeโ€ข3y ago
Need to run, but this was good ๐Ÿ‘‹
fair-rose
fair-roseOPโ€ข3y ago
have fun ๐Ÿ™‚
fair-rose
fair-roseOPโ€ข3y ago
@piecyk I have created a PR ๐Ÿ™‚ https://github.com/TanStack/virtual/pull/509
GitHub
no force render on change by alissaVrk ยท Pull Request #509 ยท TanSta...
this way we add a real state to the component, and the state is correctly immutable. isScrolling is not directly in the state anymore, because I don't think it's a good reason to render the...

Did you find this page helpful?