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

[BUG] initialTopMostItemIndex={undefined} causes runtime error #777

Closed
ondrejsevcik opened this issue Oct 20, 2022 · 6 comments
Closed

[BUG] initialTopMostItemIndex={undefined} causes runtime error #777

ondrejsevcik opened this issue Oct 20, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@ondrejsevcik
Copy link

ondrejsevcik commented Oct 20, 2022

Describe the bug

According to the typescript typing, the property initialTopMostItemIndex is optional. But when undefined is passed into it, it causes runtime error.

image

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://codesandbox.io/s/boring-lamarr-eoks7x?file=/src/App.js
  2. Change initialTopMostItemIndex from undefined to 0 and it will work.

Expected behavior

The TypeScript definiton should be updated or it should be possible to pass undefined into this property.

@ondrejsevcik ondrejsevcik added the bug Something isn't working label Oct 20, 2022
@petyosi
Copy link
Owner

petyosi commented Oct 20, 2022

This has been discussed on several occasions (here: #532). I don't plan on supporting passing undefined to properties. The component has a complex internal state, so many of the properties actually work like calling a method - and undefined is ambiguous.

@petyosi petyosi closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2022
@ondrejsevcik
Copy link
Author

I understand that allowing undefined might not make sense, but shouldn't then the TypeScript typings be updated? Currently it's misleading. I think that's where a lot of confusion comes from.

@petyosi
Copy link
Owner

petyosi commented Oct 20, 2022

I have not given this enough attention, but I am not sure if the typings themselves are to be changed or you need to do this on the project side: microsoft/TypeScript#44421. Happy to accept a PR if you have the time to dive into the issue.

@ondrejsevcik
Copy link
Author

The List component explicitly allows to pass undefined according to it's typings. I suppose this property is optional on purpose.

initialTopMostItemIndex?: number | IndexLocationWithAlign

I'm not sure why it works when you omit the property altogether, but when you pass undefined to it it breaks. Practically speaking, those two should be identical.

<Virtuoso data={data} />
// vs. 
<Virtuoso data={data} initialTopMostItemIndex={undefined} />

@petyosi
Copy link
Owner

petyosi commented Oct 21, 2022

Check the link I sent and this one. There is a difference between

property: number | undefined

and

property?: number

I believe that the typings are correct.

@ondrejsevcik
Copy link
Author

ondrejsevcik commented Nov 21, 2022

@petyosi thank you for the link. I see now how these two are different.

Though, I think this will keep coming back to you - especially since this setting is not enabled by default in TS.

Anyway, I appreciate you looking into this. Thank you for your time 🙏🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants