devout citrus
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
solar basalt
virtualizer is same, are you passing getItemKey as inline function? This will invalidate the measurements creating new virtual items each re-render
devout citrus
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
solar basalt
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>
))}
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 🙂
solar basalt
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
devout citrus
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)
devout citrus
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
solar basalt
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
devout citrus
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
solar basalt
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
devout citrus
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 🙂
solar basalt
kinda, here you can assume that
if ref is the same -> value is the same or different 😉
if ref is different -> value is different
devout citrus
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 🙂
solar basalt
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
solar basalt
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 🙂
devout citrus
what I imagine is not a small change to the API, maybe I'll have the time, who knows.. 🙂
solar basalt
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 😉
devout citrus
depends what the wrapper returns, I'd assume it returns the sizes -> then you get new sizes, yes
solar basalt
don't agree, instance is not bound directly with rendering so imho not
devout citrus
did not understand that
solar basalt
you don't render instance <div>{instance}<div>
devout citrus
who is instance?
solar basalt
Need to run, but this was good 👋
devout citrus
have fun 🙂