React 18 Support

[petyosi]

Update: the issue is solved. Component should work out of the box.

The latest version of react-virtuoso seems to be working fine with React 18, except for increased "blinking" when scrolling upwards through unevenly sized items. ~~

For now, I have found a remedy (calling flushSync at the right moment), but it triggers dev mode warnings in React 16/17. The code could detect the React version, but it's not reliable, as you might be running React 18 in compatible mode. So the fix is activated using the react18ConcurrentRendering property - see sample.

If you discover any problems in React 18, please comment here. Thanks!

[gaearon]

The naming of the prop seems unfortunate since the issue is caused by automatic batching rather than concurrency. (React 18 does not use concurrency unless you opt into it with a specific concurrent feature like startTransition.) It also seems unfortunate that people using the latest version of React need to add a special prop.

What is the warning you’re seeing in 17 with flushSync?

[petyosi]

Thanks for the feedback @gaearon. I know, adding a prop is not great :(. Here's the sandbox where you can see the error I get in 17. Just scroll up in the list a few times. My theory is that the internal state management kicks in a render cycle that overlaps with the flushSync call which adjusts a CSS margin value.

Warning: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.
    at eval (https://vnbem4.csb.app/node_modules/react-virtuoso/dist/index.m.js:1423:13)
    at div
    at ye (https://vnbem4.csb.app/node_modules/react-virtuoso/dist/index.m.js:1531:13)
    at div
    at eval (https://vnbem4.csb.app/node_modules/react-virtuoso/dist/index.m.js:1502:15)
    at eval (https://vnbem4.csb.app/node_modules/react-virtuoso/dist/index.m.js:1621:11)
    at eval (https://vnbem4.csb.app/node_modules/@virtuoso.dev/react-urx/dist/react-urx.esm.js:118:38)
    at App

The underlying problem is that, under certain conditions, I need to adjust a margin state value AND call scrollBy synchronously to adjust for newly discovered list item dimensions. Let me know if there's some appetite for looking into that deeper. I can probably isolate it better.

On a side note, if I have to keep the prop, would something like react18BatchRendering be more suitable?

[gaearon]

Frankly, I don't think the prop approach makes sense at all. People using the library with 18 would expect it to "just work". If it doesn't work, it shouldn't declare 18 compatibility, and instead can add flushSync in the next major.

I don't think we've seen a case where flushSync is absolutely necessary though yet. There's usually some better solution in each case. It would help a lot if you could create a tiny repro without your library that just showcases the problem you're trying to solve with plain React, setState call, and CSS.

[petyosi]

@gaearon highly appreciate you engaging with this. Here's the problem in a nutshell - https://codesandbox.io/s/react18-blink-without-flush-sync-vqgeky?file=/src/App.tsx. A thing that I could not reproduce in the sandbox was the warning that flushSync triggers in React 17. However, the actual problem which I'm trying to solve is clearly visible.

FWIW, I've tried working around the scrollBy call to be managed by useEffect or useLayoutEffect, but it did not work.

[gaearon]

Can you trim down the repro a bit to show the real use case? i.e. why do you adjust the margin and scroll — how is this used in practice for the list?

[petyosi]

I don't think that I can trim it further or make it more real without pulling the inner calculation guts of the library - it will actually "buff up" the example.

Let me to explain the real use case:

• The list has a "bottom up" scroll mode (useful for chat interface or tail -f like logs). It assumes a default size for each list item, and fills in the space of the unrendered items using a placeholder element or padding.
• As the user scrolls up, the list starts measuring the newly rendered items. some of them might have different sizes then the "assumed" one.
• These unexpected sizes are taken into account, but to prevent "jumping", the list keeps track of the caused deviation. This deviation is "temporarily" applied using marginTop to keep the list position consistent. Initially, I was calling scrollBy immediately, but it did not work on iOS.
• When scrolling stops, the deviation is reset and scrollBy is called to restore the location. This is what the example shows.
• Those two actions (margin + scrollBy) don't blink in React 17 but blink when batch rendering is on.

This is the actual implementation of the above.

Hope this makes sense. Again, huge thanks, love your work.

[gaearon]

Is it possible to just call setMargin but then perform the scrollBy call in a layout effect? And use a ref to keep track of whether it already ran or not.

[github-actions[bot]]

:tada: This issue has been resolved in version 2.10.2 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[petyosi]

Is it possible to just call setMargin but then perform the scrollBy call in a layout effect? And use a ref to keep track of whether it already ran or not.

@gaearon Tried that. The scroll compensation logic works correctly, but the blinking is still there.

To make matters more complicated, the sequence of the margin setting/scrolling by should be tweaked depending on the value (code). As far as I can see, I can't control this through useLayoutEffect.

Ultimately, I think I found a solution through direct dom access, here's the relevant part.

Again, thanks for your input. This react-virtuoso library is dealing with messy browser edge cases, so I'm fine with introducing workarounds so that its users don't have to do so. The flushSync call seems conceptually right to me, and the only problem is its inconsistent behavior across 17 and 18.