Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running virtual examples with React 18 createRoot causes rendering lag #549

Closed
2 tasks done
bwamsellem opened this issue May 9, 2023 · 13 comments
Closed
2 tasks done

Comments

@bwamsellem
Copy link

Describe the bug

I have tested the examples/react/fixed example with React 18, so changed the init code to use the new createRoot / render API:

ReactDOM.render( , document.getElementById('root'))
=>
ReactDOM.createRoot( document.getElementById('root')).render( )

Before: the scrolling is super smooth,
After: you can notice an important lag ( white flickering) while scrolling through rows or grid.

Is this caused by the concurrent rendering of React 18?

Your minimal, reproducible example

https://react-ts-mfeisb.stackblitz.io

Steps to reproduce

  1. Run the example in dev mode

Expected behavior

Smooth scrolling

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Windows 10 + Chrome 113

tanstack-virtual version

3.0.0-beta.54

TypeScript version

4.4.2

Additional context

No response

Terms & Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@piecyk
Copy link
Collaborator

piecyk commented May 11, 2023

@bwamsellem kinda looks ok for me, can you create small clip showing the issue.

Is this caused by the concurrent rendering of React 18?

no, i don't think so.

@bwamsellem
Copy link
Author

Yes, I will do.

@bwamsellem
Copy link
Author

bwamsellem commented May 11, 2023

Here are two video clips showing the normal behavior (old React API) , and the slow one ( new React 18 API).

normal ( old React API) :

virtual_react17_OK_fast.mp4

slow ( React 18 API) :

virtual_react18_KO_slow.mp4

@igortas
Copy link

igortas commented May 25, 2023

Having same lag issue, table looks like it's not virtualized at all.

@manelio
Copy link

manelio commented May 27, 2023

I faced that problem months ago. My project makes intensive use of virtualization to create Excel-like spreadsheets and I switched from react-virtualized to @tanstack/virtual, and I had to make some adaptations to avoid unnecessary renders.

Anyway, for the problem you mention, my solution involves using a modified version of @tanstack/react-virtual (it's just a file without relative references):

Source code is here:

Instead of

import { useVirtualizer } from '@tanstack/react-virtual';

I use

import { useVirtualizer } from './react-virtual.custom';

And I only make a small change to invoke rerender from react-dom's flushSync:

Original:

const resolvedOptions: VirtualizerOptions<TScrollElement, TItemElement> = {
  ...options,
  onChange: (instance) => {
    rerender()
    options.onChange?.(instance)
  },
}

Modified:

import { flushSync } from 'react-dom'

const resolvedOptions: VirtualizerOptions<TScrollElement, TItemElement> = {
  ...options,
  onChange: (instance) => {
    queueMicrotask(() => {
      flushSync(() => {
        rerender()
      })
    })
    options.onChange?.(instance)
  },
}

I believe that the entire onChange callback or the rerender call should be overridable by the user. I am not an expert in Typescript and I am not clear about the best approach, but I will create a pull request in case any maintainer finds the change interesting to add.

@nongdinhtuyen
Copy link

I faced that problem months ago. My project makes intensive use of virtualization to create Excel-like spreadsheets and I switched from react-virtualized to @tanstack/virtual, and I had to make some adaptations to avoid unnecessary renders.

Anyway, for the problem you mention, my solution involves using a modified version of @tanstack/react-virtual (it's just a file without relative references):

Source code is here:

Instead of

import { useVirtualizer } from '@tanstack/react-virtual';

I use

import { useVirtualizer } from './react-virtual.custom';

And I only make a small change to invoke rerender from react-dom's flushSync:

Original:

const resolvedOptions: VirtualizerOptions<TScrollElement, TItemElement> = {
  ...options,
  onChange: (instance) => {
    rerender()
    options.onChange?.(instance)
  },
}

Modified:

import { flushSync } from 'react-dom'

const resolvedOptions: VirtualizerOptions<TScrollElement, TItemElement> = {
  ...options,
  onChange: (instance) => {
    queueMicrotask(() => {
      flushSync(() => {
        rerender()
      })
    })
    options.onChange?.(instance)
  },
}

I believe that the entire onChange callback or the rerender call should be overridable by the user. I am not an expert in Typescript and I am not clear about the best approach, but I will create a pull request in case any maintainer finds the change interesting to add.

It doesn't work with useWindowVirtualizer, and I still experience white flickering when scrolling the screen

@tannerlinsley
Copy link
Collaborator

If this is still an issue for you, please comment here and tag me and I'll reopen the issue and work on it. Thanks!

@1MaTo
Copy link

1MaTo commented Oct 17, 2023

@tannerlinsley Will you fix it in the future updates?
Current tanstack-virtual examples use React 17, and this issue still present for React 18 as shown on 2 clips by bwamsellem
So the current solution for React 18 is use deprecated render method or make custom changes in package as suggested above

@piecyk
Copy link
Collaborator

piecyk commented Oct 17, 2023

@tannerlinsley Will you fix it in the future updates? Current tanstack-virtual examples use React 17, and this issue still present for React 18 as shown on 2 clips by bwamsellem So the current solution for React 18 is use deprecated render method or make custom changes in package as suggested above

This is interesting issue, like dynamic example is using react 18 and works correct https://tanstack.com/virtual/v3/docs/examples/react/dynamic

let see, planing to update other examples and simplify them a bit.

@1MaTo
Copy link

1MaTo commented Oct 18, 2023

It's really strange, when i wrote my comment, I checked all examples and they were working without any white flickering
But now its not (even the ones that still use react 17)

So I'll just ask about the correct behaviour
Is this white flickering while scroll is correct behaviour? (Current dynamic example from site)

chrome_XiuSfKKFvT.mp4

@piecyk
Copy link
Collaborator

piecyk commented Oct 19, 2023

@1MaTo as mention before this is tricky issue, there are many reasons why it's happening

  • like browse internal implementation ( noticed that currently Firefox doing best job here )
  • interesting caveats with chrome, for dynamic case when we use resize observer to watch items sizes the scroll handle lag behind Increasing cursor offset to scroll handle when scrolling #421
  • hardware performance, when running low on battery seeing more white space as browser can't keep up with rendering

Going back to react 18, as @manelio noticed we can use flushSync, but let's use it only when scrolling, checkout #608

Overall it's pretty hard to test, any help would be greatly appreciated.

@danprat92
Copy link

danprat92 commented Feb 22, 2024

@tannerlinsley @piecyk Hello, i'm unsure whether this is the same react 18 issue but seems quite related. I'm currently using table + virtual in this example here

307093934-1477c3a9-9396-407c-81e6-2550f45a5f08.mov

Screenshot 2024-02-22 at 19 40 25

Can this issue be reopened and investigated? If there's any more info needed please let me know. Or if i should open a new issue

Versions

Table: 8.12.0
Virtual: 3.1.1

@airstrike
Copy link

Source code is here:

Thanks for sharing but this link has expired. Any chance you could point future readers to the example? Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants