petyosi / react-virtuoso

The most powerful virtual list component for React
https://virtuoso.dev
MIT License
5.13k stars 299 forks source link

[BUG] `restoreStateFrom` appears to have a race condition when restoring state on component mount #1076

Closed gerdemb closed 4 months ago

gerdemb commented 4 months ago

Describe the bug restoreStateFrom appears to have a race condition when restoring state on component mount.

Reproduction https://codesandbox.io/p/sandbox/eager-maxwell-446wdy

Unfortunately, this issue is not reproducible in CodeSandbox, as reloading the page using location.reload() clears the history.state in CodeSandbox.

To Reproduce Steps to reproduce the behavior:

  1. Scroll down the list and click "Log State". In the console you can see a log of the snapshot saved to history.state
  2. Click "Reload" repeatedly and notice in the log that the snapshot is correctly retrieved from the history.state, however sometimes the scroll position of the list is restored and sometime it is not.
  3. Click "Increment Key" to force a remount of the component and the scroll position will correctly be restored.

Expected behavior restoreFromState should always restore the scroll position when passed a valid snapshot.

Screenshots https://github.com/petyosi/react-virtuoso/assets/218609/16ad4753-1687-41e1-8034-b2e2e4039c94

Desktop (please complete the following information):

Additional context

petyosi commented 4 months ago

I can't seem to reproduce the problem when I detached the sandbox. However, what you describe might happen and is unavoidable. Restore state from is an asynchronous process; it needs to load the sizes, and, once rendered, scroll to the saved location. It is possible to sneak in a prop update in between if you click the button very fast.

What is the practical problem that you encounter with this?

gerdemb commented 4 months ago

Thank you for your quick response to this report and for your efforts in maintaining the react-virtuoso library.

Regarding this issue, my use case involves preserving the list state when the component is unmounted and then remounted, during navigation between pages using react-router. Before navigating away from a page that includes the Virtuoso component, I capture a snapshot of the component's state and store it in history.state. Upon returning to the page, I retrieve this snapshot from history.state to restore the component's state.

I appreciate your explanation that restoreStateFrom is an asynchronous process which could be affected by rapid interactions. However, I am unsure about the "prop update" you mentioned might be sneaking in. In my implementation, as far as I understand, the component props should be consistent and unchanged at the time of loading. Anyway, what is the recommended way to use restoreStateFrom on component mount? Do I always need to force a remount by changing the key property like in the example?

petyosi commented 4 months ago

To clarify, your example seems correct. if the component is unmounted, you don't need to deal with keys. However, I think there could be something else going on. I took your code and put it here - https://github.com/virtuoso-dev/1076. On my side, the restore works as expected when I follow your actions from the video (mac m1, latest chrome). Can you clone the repo and add your router setup? Perhaps this will reveal the issue.

gerdemb commented 4 months ago

As a workaround, adding a timeout like this seems to solve the problem:

const VirtuosoTimeout = React.forwardRef<VirtuosoHandle, VirtuosoProps>(
  (props, ref) => {
    const [initialized, setInitialized] = useState(false);

    useEffect(() => {
      const timer = setTimeout(() => {
        setInitialized(true);
      }, 0);

      return () => clearTimeout(timer);
    }, []);

    if (!initialized) return null;

    return <Virtuoso ref={ref} {...props} />;
  }
);

I'm going to take a look at the repo that you shared and see if I can reproduce the problem there.

gerdemb commented 4 months ago

Thank you for sharing your repo. I was also unable to reproduce the problem with your repo.

I'm actually using Remix which is based on react-router. I couldn't figure out how to easily retro-fit Remix onto your existing repository, so I created a fork with a new Remix project template and was able to reproduce the problem immediately.

https://github.com/gerdemb/1076/tree/remix

I hope this helps you reproduce the problem.

github-actions[bot] commented 4 months ago

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

The release is available on:

Your semantic-release bot :package::rocket:

petyosi commented 4 months ago

I think I fixed that. Check the latest state of https://github.com/virtuoso-dev/1076. That was a weird problem, to be honest, I think the react router does something extra there.

gerdemb commented 4 months ago

I can confirm that upgrading to version 4.7.10 resolves the problem. Thank you very much for your support.