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

Virtualizer.scrollToIndex - Uncaught error when list is empty #696

Open
2 tasks done
k-1ee opened this issue Mar 28, 2024 · 6 comments
Open
2 tasks done

Virtualizer.scrollToIndex - Uncaught error when list is empty #696

k-1ee opened this issue Mar 28, 2024 · 6 comments

Comments

@k-1ee
Copy link

k-1ee commented Mar 28, 2024

Describe the bug

Inside a useEffect, using Virtualizer.scrollToIndex when the list is empty is throwing the following error:

Uncaught Error: Unexpected undefined at notUndefined at Virtualizer.getOffsetForIndex

Admittedly, it is reasonable to have to add a condition in order to avoid calling scrollToIndex when the list is empty. Due to the nature of side effects + asynchronicity, however, there is an edge case in which scrollToIndex may have been called just as the list became empty, thus causing an error.

Is there any insight into why Virtualizer.scrollToIndex throws an error when called upon an empty list? Would it be better to handle this as a warning instead of an error? Or if the list is empty, have scrollToIndex return early instead of trying to execute?

Unfortunately, wrapping scrollToIndex inside a try/catch block within useEffect does not swallow the error.

Your minimal, reproducible example

https://codesandbox.io/p/devbox/cocky-franklin-d79ytd?file=%2Fsrc%2Fmain.tsx%3A1%2C1-113%2C1&layout=%257B%2522sidebarPanel%2522%253A%2522EXPLORER%2522%252C%2522rootPanelGroup%2522%253A%257B%2522direction%2522%253A%2522horizontal%2522%252C%2522contentType%2522%253A%2522UNKNOWN%2522%252C%2522type%2522%253A%2522PANEL_GROUP%2522%252C%2522id%2522%253A%2522ROOT_LAYOUT%2522%252C%2522panels%2522%253A%255B%257B%2522type%2522%253A%2522PANEL_GROUP%2522%252C%2522contentType%2522%253A%2522UNKNOWN%2522%252C%2522direction%2522%253A%2522vertical%2522%252C%2522id%2522%253A%2522cltrkx8q400073b6gw04ftpw8%2522%252C%2522sizes%2522%253A%255B70%252C30%255D%252C%2522panels%2522%253A%255B%257B%2522type%2522%253A%2522PANEL_GROUP%2522%252C%2522contentType%2522%253A%2522EDITOR%2522%252C%2522direction%2522%253A%2522horizontal%2522%252C%2522id%2522%253A%2522EDITOR%2522%252C%2522panels%2522%253A%255B%257B%2522type%2522%253A%2522PANEL%2522%252C%2522contentType%2522%253A%2522EDITOR%2522%252C%2522id%2522%253A%2522cltrkx8q400023b6gr2ory0k1%2522%257D%255D%257D%252C%257B%2522type%2522%253A%2522PANEL_GROUP%2522%252C%2522contentType%2522%253A%2522SHELLS%2522%252C%2522direction%2522%253A%2522horizontal%2522%252C%2522id%2522%253A%2522SHELLS%2522%252C%2522panels%2522%253A%255B%257B%2522type%2522%253A%2522PANEL%2522%252C%2522contentType%2522%253A%2522SHELLS%2522%252C%2522id%2522%253A%2522cltrkx8q400043b6g6oi97apj%2522%257D%255D%252C%2522sizes%2522%253A%255B100%255D%257D%255D%257D%252C%257B%2522type%2522%253A%2522PANEL_GROUP%2522%252C%2522contentType%2522%253A%2522DEVTOOLS%2522%252C%2522direction%2522%253A%2522vertical%2522%252C%2522id%2522%253A%2522DEVTOOLS%2522%252C%2522panels%2522%253A%255B%257B%2522type%2522%253A%2522PANEL%2522%252C%2522contentType%2522%253A%2522DEVTOOLS%2522%252C%2522id%2522%253A%2522cltrkx8q400063b6g1gkwd1g7%2522%257D%255D%252C%2522sizes%2522%253A%255B100%255D%257D%255D%252C%2522sizes%2522%253A%255B50%252C50%255D%257D%252C%2522tabbedPanels%2522%253A%257B%2522cltrkx8q400023b6gr2ory0k1%2522%253A%257B%2522id%2522%253A%2522cltrkx8q400023b6gr2ory0k1%2522%252C%2522tabs%2522%253A%255B%257B%2522id%2522%253A%2522clubufmra00023b6go7v45762%2522%252C%2522mode%2522%253A%2522permanent%2522%252C%2522type%2522%253A%2522FILE%2522%252C%2522initialSelections%2522%253A%255B%257B%2522startLineNumber%2522%253A1%252C%2522startColumn%2522%253A1%252C%2522endLineNumber%2522%253A113%252C%2522endColumn%2522%253A1%257D%255D%252C%2522filepath%2522%253A%2522%252Fsrc%252Fmain.tsx%2522%252C%2522state%2522%253A%2522IDLE%2522%257D%255D%252C%2522activeTabId%2522%253A%2522clubufmra00023b6go7v45762%2522%257D%252C%2522cltrkx8q400063b6g1gkwd1g7%2522%253A%257B%2522tabs%2522%253A%255B%257B%2522id%2522%253A%2522cltrkx8q400053b6gi57lox78%2522%252C%2522mode%2522%253A%2522permanent%2522%252C%2522type%2522%253A%2522TASK_PORT%2522%252C%2522taskId%2522%253A%2522dev%2522%252C%2522port%2522%253A5173%252C%2522path%2522%253A%2522%252F%2522%257D%255D%252C%2522id%2522%253A%2522cltrkx8q400063b6g1gkwd1g7%2522%252C%2522activeTabId%2522%253A%2522cltrkx8q400053b6gi57lox78%2522%257D%252C%2522cltrkx8q400043b6g6oi97apj%2522%253A%257B%2522tabs%2522%253A%255B%257B%2522id%2522%253A%2522cltrkx8q400033b6ge9it073a%2522%252C%2522mode%2522%253A%2522permanent%2522%252C%2522type%2522%253A%2522TASK_LOG%2522%252C%2522taskId%2522%253A%2522dev%2522%257D%255D%252C%2522id%2522%253A%2522cltrkx8q400043b6g6oi97apj%2522%252C%2522activeTabId%2522%253A%2522cltrkx8q400033b6ge9it073a%2522%257D%257D%252C%2522showDevtools%2522%253Atrue%252C%2522showShells%2522%253Atrue%252C%2522showSidebar%2522%253Atrue%252C%2522sidebarPanelSize%2522%253A15%257D

Steps to reproduce

  1. Inside useEffect, call scrollToIndex(0) when the list is empty
  2. See that an uncaught error is thrown

Expected behavior

As a user, I expected scrollToIndex to not throw an error when the virtual list is empty.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

macOS - Chrome v.122.0.6261.129

tanstack-virtual version

v3.2.0

TypeScript version

No response

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.
@KevinVandy
Copy link
Member

Yeah, this is annoying, but also how it's designed.

@k-1ee
Copy link
Author

k-1ee commented Mar 29, 2024

@KevinVandy - Would it be a breaking change if scrollToIndex returns early (and logs a warning) instead of throwing?

I'll have to come up with a workaround for now, but this scenario seems like a common use case IMO.

@piecyk
Copy link
Collaborator

piecyk commented Mar 29, 2024

@KevinVandy @k-1ee yeah we can change the implementation if it feels like common use case.

@k-1ee
Copy link
Author

k-1ee commented Mar 29, 2024

@piecyk - Would be great, if possible!

In many cases, scrollToIndex would be called as a side effect -- and if the virtualizer could be used on empty lists, then it should be unexpected behavior for its methods to potentially crash the app through an uncaught error.

@galangel
Copy link

We just encountered this as well. We are filtering, sorting, and scrolling to index on user input, the list is never empty, and still, there is this error.

Wrapping the scroll to index with try/catch did not catch the error.
Debouncing the logic helped, but I don't like it.

@rasgo-cc
Copy link

I'm not sure how this is happening but I also get an occasional "Unexpected undefined" error. Some users report some rows not being rendered at times and I believe that might be related with this, although I wasn't able to reproduce it yet unfortunately.

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

5 participants