react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

List State: `collection.getItem()` can return `undefined`

Open cedeber opened this issue 1 year ago โ€ข 6 comments

Provide a general summary of the issue here

Well. I have a very hard to reproduce bug - it's randomly happening - and hard to check in the current repo the returned type. So, I build a Combobox thanks to the hooks and load async some data to fill the items - with or without useAsyncList btw - and sometimes I receive:

Uncaught TypeError: Cannot read properties of undefined (reading 'index')
    at useListState.ts:81:21

so looks like that here https://github.com/adobe/react-spectrum/blob/b5974ad1ccf2abc984c5cc6664b3c2d03de36768/packages/%40react-stately/list/src/useListState.ts#L63 collection.getItem() can return undefined

๐Ÿค” Expected Behavior?

It doesn't crash ^_^

๐Ÿ˜ฏ Current Behavior

It does crash :-/

๐Ÿ’ Possible Solution

No response

๐Ÿ”ฆ Context

No response

๐Ÿ–ฅ๏ธ Steps to Reproduce

It's a very hard issue to reproduce and hard to have an easy example unfortunately. I can't have a setup with the current reop that returns another type that "any", so it's really hard to find out what happens.

The only thing I know it that you need to:

  • open the combobox
  • put some text (not necessarily needed)
  • select an item
  • it reloads the items list because filterText changed (set by the selection)
  • clean the selected Item
  • it reloads the first 50 items (filterText = "")
  • repeat

as fast as you can to trigger it. I feel like checking the possible returned value of getItem would be easier but I was unsuccessful here.

Version

latest

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Windows

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

cedeber avatar Jan 30 '24 15:01 cedeber

Just to make sure, you are just using the hooks in your implementation right? No RAC components? I ask because RAC has a different collections setup than what we had before.

Its hard for me to say what is occurring here without a reproduction I can dig into on my end, are you able to add breakpoints to find out what the collection and cached collection is? We could always add some more resiliency against the collection.getItem() returning a undefined but ideally that section of the code should always have a cached collection in which the previous focused key exists in.

LFDanLu avatar Jan 30 '24 18:01 LFDanLu

i have the same issue

Develekko avatar Feb 06 '24 07:02 Develekko

i think they need add let index = Math.min( ( diff > 1 ? Math.max(startItem?.index - diff + 1, 0) : // Optional chaining startItem?.index // Optional chaining ), itemNodes.length - 1);

Develekko avatar Feb 06 '24 08:02 Develekko

Same problem on me, looks like overwriting package locally, and making all startItem variables optionally chained solves the issue. Not sure if owners of the package will accept that PR, but I can maybe fix that in my free time.

const startItem = cachedCollection.current.getItem(selectionState.focusedKey);

โ˜๐Ÿป collection can be undefined when I spam on useAnycList items - set, delete, change quickly and the error is thrown. No issue when optionally chained

mkrystkowicz avatar May 13 '24 19:05 mkrystkowicz

Adding the optional chaining sounds good to me, but just to double check: where does focus land with the above change? Does it get lost to the document body? Or does it land on the list itself?

LFDanLu avatar May 13 '24 22:05 LFDanLu

In my case listState.selectionManager.focusedKey also returns null if none of the elements were focused

sleonia avatar Jun 13 '24 10:06 sleonia