overscanning kicks in *after* the user scrolls in either direction?

[zikaari]

(I'm happy to post GIF of Chrome's paint-flashing debug should you not understand the case)

In a nutshell, I specified x overscanCount, and I was expecting to have x rows (pre)rendered top and bottom of List at all times. That way when the user suddenly changes scroll direction from up to down or vice versa, there is literally no flash of unstyled content.

AFAIK, react-window does overscanning in one direction only and that direction is determined once the user starts scrolling in the said direction. First scroll event that is. Due to which, the "repaint" happens in the visible viewport which isn't that pretty.

[bvaughn]

The behavior you're describing is intentional, and is consistent with the long standing behavior of react-virtualized.

My reasoning is that over-rendering has a cost, and it's generally wasteful to overscan in a direction that's "backwards" compared to the direction the user is scrolling. In my experience, there's a high chance for a small flash of empty space when the user quickly begins to scroll, regardless of overscan.

Since this issue seems to be a question, I'm going to close it now that it's been answered. We can chat more here though if you'd like.

[zikaari]

I'd like to propose the following technique and we can see if it makes any sense.

Imagine a user who is scrolling up, and so far has scrolled up 4 times. react-window is ahead of the game by (pre)rendering x items in the top of the <List />. She suddenly scrolls down and we see the ugly FOUC. Not very pretty.

Now as I understand from your comment, having the rowRenderer render stuff for top and bottom simultaneously is expensive. Which is totally true. But how about we preserve the "trail" of rows that were freshly rendered as the user was scrolling up instead of having them "unmount" the moment they leave bottom of visible area? Maybe "preserve" just few of them (x/2)?

That way when user switches scroll direction, there is something to show besides blankness.

[bvaughn]

That's a reasonable suggestion, but it would also add complexity since list and grid would need to now track some extra state to know (a) when to preserve these backwards rows and (b) when to throw them out if e.g. props changed.

It's worth giving some thought.

For what it's worth, we hope to add more support for windowing to React core in the near to medium future. So some behaviors like we're talking about might be moved into the framework.

[zikaari]

To be able to go further, do you mind explaining why react-window (and react-virtualized) ask the rowRenderer to render all the items visible in the viewport when scrolled and not just the new ones that got scrolled in?

For example, if turning the mousewheel up once causes 3 more items to reveal, why doesn't react-window just ask for those 3 new items?

In the gif below, rendering item # ${index} is being logged in each call to rowRenderer. I was expecting no more than 5 of those per scroll (that too is quite a big leeway considering only 2 more items got scrolled in at the bottom).

[bvaughn]

Both libraries pass style objects to item renderers (to position items). These objects are cached while the user is scrolling, but the cache is thrown away once scrolling stops. This means that the next scroll will initially render all items again for the first "scroll" event, but then only render the ones that are newly added to the list.

In most cases, this is good enough behavior, because it keeps scrolling pretty slim while avoiding potential stale style bugs.

Furthermore, a single scroll event isn't very performance sensitive. Many scroll events in a row do though. This approach optimizes for the latter case.

If you'd like to tweak this further though, you should be able to add your own shoudlComponentUpdate method that just ignores the style prop.

[bvaughn]

Dan mentioned an idea that maybe List/Grid could overscan in both directions only when not scrolling. That might be a good compromise.

[bvaughn]

I like the idea of over-scanning in both directions only when scrolling isn't active. There is one potential downside though– the onItemsRendered callback would get called an extra time now after scrolling stops. I'm not sure this is actually a problem. It seems a little awkward though.

[bvaughn]

Change in overscan behavior implemented in #112

[zikaari]

Super awesome Brian! You were quick, thank you!

If you'd like to tweak this further though, you should be able to add your own shoudlComponentUpdate method that just ignores the style prop

Ignoring style prop through shouldComponentUpdate would also not let the updated style prop reach DOM as that specific component update would be skipped over.

I guess the proper way to go with this is to have an extra wrapper div around the main thing:

Before

renderItem({index, style}) {
  // ...
  return <FileEntry style={style} model={fileEntry} onClick={this.handleClick} />
}

After

renderItem({index, style}) {
  // ...
  return <div style={style}
      <FileEntry model={fileEntry} onClick={this.handleClick} />
    </div>
}

I reckon going with the After method, I can have React save some CPU time as it'll skip over <FileEntry /> component, which, with every instance it renders bunch of complex stuff (file icon, label, decorations etc.)

Please let me know if you think there's any better way to do this!

[bvaughn]

Ignoring style prop through shouldComponentUpdate would also not let the

updated style prop reach DOM as that specific component update would be skipped over. I meant ignoring the new style wrapper object. In this case, you would just need to loop over style properties and compare them instead. Although styles are often/usually static in most cases. No maybe you know yours are too? If so, you could just use the style prop entirely.

…

On Sat, Dec 22, 2018, 6:21 AM Neek Sandhu ***@***.*** wrote: Super awesome Brian! You were quick, thank you! If you'd like to tweak this further though, you should be able to add your own shoudlComponentUpdate method that just ignores the style prop Ignoring style prop through shouldComponentUpdate would also not let the updated style prop reach DOM as that specific component update would be skipped over. I guess the proper way to go with this is to have an extra wrapper div around the main thing: Before renderItem({index, style}) { // ... return <FileEntry style={style} model={fileEntry} onClick={this.handleClick} /> } After renderItem({index, style}) { // ... return <div style={style} <FileEntry model={fileEntry} onClick={this.handleClick} /> </div> } I reckon going with the *After* method, I can have React save some CPU time as it'll skip over <FileEntry /> component, which, with every instance it renders bunch of complex stuff (file icon, label, decorations etc.) Please let me know if you think there's any better way to do this! — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#111 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABznQGewU7-Kgjqm-5bLBNLFaMeQx35ks5u7hWdgaJpZM4ZcNiA> .

[bvaughn]

Maybe I could export a custom sCU from react-window for this?

[bvaughn]

I created a Twitter poll for with a few options:
https://twitter.com/brian_d_vaughn/status/1076516431074861056

[bvaughn]

I think this shows promise as an incremental step!

[zikaari]

Hmmmmm 🤔 Looks yummy!

Let's see where this takes us.

I'm not sure if you considered this in past. But here's my take on windowing with no involvement of style props at all!

The demo has (NewImplementaion | ReactWindow | NoWindowing), in that order.

https://codesandbox.io/s/nn4nnm6mnp

At the moment, new implementation works only in one direction, scrolling down that is.

[bvaughn]

I have played a bit with similar things (like using margin/padding to offset items and then allow them to use relative positioning). It has some nice benefits for lazily measured content, but I think it has some other awkward edge cases too, e.g. when scrolling backwards with reflowed content, or with Grids instead of lists, or with complex layouts like react-flame-graph which renders multiple items per "row".

May still be worth playing with more.

[zikaari]

I kinda guessed you'd have tried and ruled them out. In fact, myself after fiddling with it for some time realized how complex this subject matter is.

With the style issue in context:

1. Why react-window uses position: absolute paired with top, left instead of transform: translate(x, y)? I've heard they perform better

2. As we now try to figure out how to reduce expensive udpates associated with style prop, what do you think is probelematic with using wrapper approach as I mentioned before? Like <div style={style}><QuitComplexContent data={data} /></div>, and assuming data is 100% static per item. That way React will (re)render the wrapping div but skip over <QuiteComplexContent /> it being a PureComponent.

[bvaughn]

Why react-window uses position: absolute paired with top, left instead of transform: translate(x, y)? I've heard they perform better

The API change I'm currently leaning toward would enable users to use translate more easily, if they want to. But the short answer is that— in my own testing, this did not make a noticeable improvement.

As we now try to figure out how to reduce expensive udpates associated with style prop, what do you think is probelematic with using wrapper approach as I mentioned before? Like <div style={style}><QuitComplexContent data={data} /></div>, and assuming data is 100% static per item. That way React will (re)render the wrapping div but skip over <QuiteComplexContent /> it being a PureComponent.

Nothing to stop you from doing this if you prefer! But it requires an unnecessary React Element and DOM element per row— which would have an (admittedly small) negative impact on memory and CPU performance. Seems like the other routes I've proposed would not incur this penalty.

[zikaari]

Awesome thank you Brian!

Well, as for the second concern, I'm shipping the FileTree component as a library which is good at doing math (maintaining flattened structures derived from nested directories without traversing). Users will be required to supply an item renderer which must be wrapped in the styled div anyway (to free the users from dealing with "required" styling) so I think in this case I'm on a good track!

[bvaughn]

After more consideration, I'm only going to add the areEqual and shouldComponentUpdate exports. I'll leave the spreadStyleProps prop for a possible v2 change. It makes Flow types too awkward. (I don't know how to correlate the presence of a spreadStyleProps={true} with different props passed to the item renderer component.)

I think the helper functions, or the wrapper object like you described above, are sufficient for this performance optimization anyway.

[zikaari]

I think we might be heading towards a sick gold mine!

While I was dissecting VSCode's file tree implementation, it turned out that they are using a very similar technique as I showed in the linked sandbox (except that I was altering the scrollTop, and they are altering the top style prop).

I get what they are doing, and I'm sure you'll see that too. But if you approve, we can have a take at this. (tbh I'm all in for setting the itemRenderers free of worrying about style prop).

Here's the GIF of what's happening:

[bvaughn]

If you'd like to put together a POC/PR, feel free!

I'm not convinced that removing the explicit style param from item renderers would be an improvement for react-window. I considered it recently as part of working on #6 (and even did a quick little POC) but it didn't seem like it was going to make things any easier for that component, since RV would still need to know (or measure) individual items for an accurate scrolling implementation.

It would also have an impact on some of the more advanced integrations with RV like react-flame-graph. These could probably be managed by adding a wrapper HTMLDivElement, but wrapper elements have a cost.

[zikaari]

ahh...

I guess it's the fact that react-window is meant to be a general one-size-fits-all solution, whereas VSCode is not.

And like you said that you tried it recently then it's clear that this direction is a no go. I just got too ambitious and wanted to get rid of style prop 😅 but dreams can't always come true!

[bvaughn]

Well you're always welcome to put together a PR for further discussion. Ideas are welcome!

…

On Sun, Dec 30, 2018, 9:54 AM Neek Sandhu ***@***.*** wrote: ahh... I guess it's the fact that react-window is meant to be a general one-size-fits-all solution, whereas VSCode is not. And like you said that you tried it recently then it's clear that this direction is a no go. I just got too ambitious and wanted to get rid of style prop 😅 but dreams can't always come true! — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#111 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABznU6bF0Y9-6rwtpLBqcXEmG6dMyaWks5u-P3lgaJpZM4ZcNiA> .

[lambrospetrou]

Hello @bvaughn, thanks a lot for the awesome library. It has great performance and the API is amazing.

However, I have hit this exact issue and reading the above discussion, although I see your points, I believe that there is something to be improved.

In my case I have long list (~10K rows) and each row renders hundreds of items. The problem is that scrolling in a different direction from before will basically discard the just newly created elements, and then when scrolling back they will get re-created.

My tool intuitively suggests users to alternate between scroll up and down while inspecting the rendered rows (the combined rows make up an interactive visualisation), by a small amount, usually 100-200px. I was surprised though that when you scroll into a different direction the opposite overscan is being reset to 1. I found this in this excerpt of the code.

I understand that over-scanning the scroll's direction is logical (similar to prefetching) but it causes many flushes in my case since it's common to have slight scrolling in either direction. My RowRenderer is using the React.memo(..., areEqual) approach which solves most of memoization problems, but since the overscan components will get reset every time there is the slightest scrolling, we basically throw away its usefulness.

What do you think about exposing another oppositeOverscanCount prop which is used instead of the hardcoded 1 value (obviously defaulting to 1)? This would provide the flexibility to choose both parameters depending on the use-case.

I have a hacky work-around to improve this a bit by rendering multiple actual rows of data for each single row being rendered by the FixedSizeList, which basically simulates the above suggestion, but it makes my row components more complex and it feels wrong (will need to parse and modify the passed in style etc.).

I am looking forward to your response. I could open a new ticket but since there is a nice discussion for the exact same thing here, I preferred to reply here.

Thanks again for the amazing work in the library.

[bvaughn]

In my case I have long list (~10K rows) and each row renders hundreds of items.

This sounds unexpected. Why would each row be so "heavy"?

The problem is that scrolling in a different direction from before will basically discard the just newly created elements, and then when scrolling back they will get re-created.

Yes. This is how libraries like this work. What you're describing (quickly scrolling back and forth) isn't a use case this library is optimized for. (I don't think it's very common?)

My RowRenderer is using the React.memo(..., areEqual) approach which solves most of memoization problems, but since the overscan components will get reset every time there is the slightest scrolling, we basically throw away its usefulness.

Correct me if I'm wrong, but the only rows being "thrown away" would be rows overscanned past the minimum of 1– right?

What do you think about exposing another oppositeOverscanCount prop which is used instead of the hardcoded 1 value (obviously defaulting to 1)?

To be honest, I have an initial strongly negative reaction to this idea. It reminds me of some of the APIs I added to react-virtualized that, over time, made it harder to maintain.

Let me give it a little thought to see if I overcome the initial reaction. 😄 It's not an unreasonable request, I'm just very reluctant to add APIs for use cases that I think are edge cases. (They add size and overhead to the library that only a small % of users benefit from.)

[lambrospetrou]

This sounds unexpected. Why would each row be so "heavy"?

Well, depending on the width of the screen, some items inside the row might be a few pixels wide, and others can be way wider. So the worst case could be width/5px items. Although, yes I admit it is not the common case.

Yes. This is how libraries like this work. What you're describing (quickly scrolling back and forth) isn't a use case this library is optimized for. (I don't think it's very common?)

Not very common, yes. But I don't think it's not a use-case for these libraries. If you just display data, like in common tables yes people will probably search for the row they want and focus on that, but in my tool people might wonder around. I am not talking about scrolling up and down like crazy, but the slightest scroll right now triggers the above behavior.

Correct me if I'm wrong, but the only rows being "thrown away" would be rows overscanned past the minimum of 1– right?

Yes, that's right.

To be honest, I have an initial strongly negative reaction to this idea. It reminds me of some of the APIs I added to react-virtualized that, over time, made it harder to maintain.

Let me give it a little thought to see if I overcome the initial reaction. 😄 It's not an unreasonable request, I'm just very reluctant to add APIs for use cases that I think are edge cases. (They add size and overhead to the library that only a small % of users benefit from.)

I totally understand your reaction and the desire to keep the API small. However, I did try the hack mentioned above (the 1 row for the component contains multiple rows from my data) and the performance and feel is really good because there is almost always some content rendered to the direction of the scroll.

I am open to other ideas you might have as well, to be fair even if the same overscanCount is used in both directions, my issue would go away.

Thanks

[bvaughn]

I'll think on it a little. In the past at one point, I did use overscanCount in both directions– but I changed it to only be used in the forward direction so I could reduce the amount of scripting time was spent in perf sensitive "scroll" handlers. Although if the memoization is solid, this shouldn't have much of an impact so maybe it wouldn't hurt too much to change back... I'll want to think on it some more.

[austincondiff]

Any movement on this issue?

I agree that while this isn't a common use-case, it is still a use-case and while it increases the amount of scripts run, as mentioned memoization should negate this concern.

My argument is that in applications where attention UX is critical, things like this stick out like a sore thumb and make things look broken.

I don't see any harm in at least allowing for this behavior either by specifying a reverseOverscanCount amount or setting a birdirectionalOverscan to true.

[kevinjmo]

@bvaughn hoping to bump this issue as it's been over two years. Would love to have the option to toggle bidirectional overscan!

[zackasaurus]

@bvaughn