no force render on change

[alissaVrk]

1. this way we add a real state to the component, and the state is correctly immutable.
2. it solves the "tearing" issue (I saw it happen in my project already) - article explaining
3. isScrolling is not directly in the state anymore, because I don't think it's a good reason to render the component -> I have added the ability to register to isScrolling change.
4. I return a stable reference for the virtualItems array

it probably would be better to make some of these changes in the core, but

1. there are no examples or tests for Veu, Solid, and Svelte. so there is no way to test it.
2. it is more work, so let's see what you think about it first :)

I left the onChange option, but I would remove it - can't see why would anyone use it

Will happily update the docs if the idea will pass :)

** I would have also subtracted the scrollMargin from the virtualItems start and end

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
react-virtual	❌ Failed (Inspect)			Feb 19, 2023 at 7:15AM (UTC)

[piecyk]

Thanks @alissaVrk will check it latter. One of advantages having a method to get virtual items is that we can get updated items in same render. Like in case of infinite reversed virtual list, the trick is to update scroll offset in same time when new data are prepend, calling get items will return correct items for new offset, more at discussion #195 (comment)

Will this example work also so smooth when scrolling up? https://codesandbox.io/s/beautiful-meninsky-fr6csu?file=/pages/index.js

it solves the "tearing" issue (I saw it happen in my project already)

Ooo that is interesting, didn't see that before. Can you create codesandbox example?

[alissaVrk]

about the reverse scroll, the only problematic line that I see there is
virtualizerRef.current.scrollOffset = nextOffset;
and if I remove it I see no difference..

I will try it with my version.

about the "tearing" I couldn't recreate in a sandbox yet

[alissaVrk]

ah, now I see the issue.
we need virtualizerRef.current.scrollOffset = nextOffset;
I can add a setScrollOffset for this case

[alissaVrk]

the reverse scroll works ok, would you like me to add it to the infinite scroll example?

is there a way to publish the package somewhere? it's very hard to link, I get duplicate react / react-dom.
I really would like to test it with my project

btw, do we need to support react 17?

[piecyk]

the reverse scroll works ok,

Great! It make sense as useSyncExternalStore basic use same logic by forcing re-render in layout effect if snapshot change.

it solves the "tearing" issue (I saw it happen in my project already)

That's why this is bin suprpsing 🤔

Overall still not sold by this, what would be real benefit to make this change that would justify the complexity? Other than

• items will have same ref, this would only have a difference when passing it to effect deps in react 🤔 Let's think if there are practical uses cases to keep the prev items.
• isScrolling will not trigger re-render. Now it's adding two re-renders, if that creates and issue, then it would mean that something is wrong with the app code.

is there a way to publish the package somewhere? it's very hard to link, I get duplicate react / react-dom.
I really would like to test it with my project

I think not, but i think we can create custom react-virtual and mimic the subscribeToChanges as basic it's just calling onChange.

[piecyk]

@alissaVrk you can use this in your project for test ( imho will work the same ) https://codesandbox.io/p/sandbox/crazy-cookies-brjtc0?file=%2Fsrc%2Freact-virtual%2Findex.tsx

[alissaVrk]

I think not, but i think we can create custom react-virtual and mimic the subscribeToChanges as basic it's just calling onChange.

did not understand this.
you are suggesting adding another package and publishing new beta for this?
isn't it possible to publish it under some random name?

benefits:

• isScrolling is not 2 renders if you scroll slowly, it's much more (mostly confusing)
• I can see why things rendered
• I have a use case for stable virtualItems - I pass them to memoized components.
• I have a timing bug, which I think it will solve - but can't test (linking issues). will write more details about it when I'll be sure what's going on there. in general I get mixed virtualItems - some with prev data keys some with new data keys
• it makes the virtualizer play nicely in react flow, like any other hook. ref changes when properties change. things that aren't really part of the state, like scroll offset aren't in the state.
  this way the user can, for example, wrap it in a custom hook and derive only the state that he cares about with the help of useMemo(() => {},[virtualizer])

I think it's simpler for the user

[alissaVrk]

another question, can we add use-sync-external-store as a real dependency?

[alissaVrk]

ah, you mean import only the core and implement the react part locally - ok

but temporarily, I don't think it's a good idea in the long run :)

[piecyk]

did not understand this.
you are suggesting adding another package and publishing new beta for this?
isn't it possible to publish it under some random name?

no, basic just create a file with your impl and update the import's paths, don't need to publish.

isScrolling is not 2 renders if you scroll slowly, it's much more (mostly confusing)

no, it's not :) it's tow re-renders, one for start and one for end, of course if range didn't change.

I can see why things rendered

hmm items are not triggering the re-render, i think mention that before.

I have a use case for stable virtualItems - I pass them to memoized components.

That most cases is wrong, as it's optimized to fast. For example when you got a list with count 100, and visible range is from 0-10, getting new items, and updating the list to count of 150, will also re-render items form 0-10 even they didn't change. Further more position of item should not trigger it re-render, that's why best is to memo rows item not the virtualization.

it makes the virtualizer play nicely in react flow, like any other hook. ref changes when properties change. things that aren't really part of the state, like scroll offset aren't in the state.
this way the user can, for example, wrap it in a custom hook and derive only the state that he cares about with the help of useMemo(() => {},[virtualizer])

hmm, i agree that you meed to have some knowledge, but i think simpler would to work on docs 🤔

[alissaVrk]

I tested, it does solve my bug... I will try to reproduce in a sandbox, but it won't be easy.. since it's timing

[alissaVrk]

no, it's not :) it's tow re-renders, one for start and one for end, of course if range didn't change.
it is one for start and one for end, it just happens more if you scroll slowly

hmm items are not triggering the re-render, i think mention that before.
don't think I understand what you mean..

with the force-render the state is basically the same (in parts) so react doesn't really know what changed
with the useSyncExternalStore the state changes.

about the complexity, I think most of it should move inside the core, and then there is no additional code, just a bit different.

and using useSyncExternalStore with whatever you want to pass inside is still nicer than the force-render.

I think it's always better to make a more intuitive lib for the people using it, most people don't really read the docs :)
if this was the API of the library + item.start = item.start - scrollMargin
I wouldn't wrap it so much for other people working on the codebase to understand (they didn't read the docs and debug the lib). and it's hard to wrap because references don't change

[alissaVrk]

btw, I think the library is really great :)
otherwise, I wouldn't spend so much time arguing :)

[piecyk]

@alissaVrk yeah, this is great, thanks for all your work! Will talk with others at @TanStack and get back to you.

[alissaVrk]

@piecyk I guess it didn't pass? :)

[tannerlinsley]

Triaging as out of date. Please reopen a fresh PR if still valid.