adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.9k stars 1.12k forks source link

ListBox fails when refreshing items because internal state is somehow broken #977

Closed tomsontom closed 2 years ago

tomsontom commented 4 years ago

๐Ÿ› Bug Report

In my real world application I can get ListBox into a state to display the following error:

Bildschirmfoto 2020-08-17 um 14 32 13

I've tried to create a reproducer but failed to do so until now. This happens when I try to update the items after a server request returned the items i want tu populate the list with.

๐Ÿค” Expected Behavior

The list updates the items

๐Ÿ˜ฏ Current Behavior

Above stack trace where somehow the internal state is somehow broken

๐Ÿ’ Possible Solution

I've not yet foudn any workaround and as I've been unable yet to publish a custom version with some debug information I'm unable to findout what is going on (and why my use of ListBox causes this error)

๐Ÿ”ฆ Context

๐Ÿ’ป Code Sample

๐ŸŒ Your Environment

Software Version(s)
react-spectrum 3.1.0
Browser Firefox 79.0
Operating System OS-X

๐Ÿงข Your Company/Team

๐Ÿ•ท Tracking Issue (optional)

LFDanLu commented 4 years ago

@tomsontom This feels tantalizingly familiar, feel like I've encountered this previously when working on an earlier iteration of ComboBox. Would you mind sharing a code snippet of how you set up your ListBox? Would you happen to be using useAsyncList alongside your ListBox for updating its items (https://react-spectrum.adobe.com/react-stately/useAsyncList.html)?

tomsontom commented 4 years ago

Well it is hard come up with something you can work with but there are some data points:

My current workaround is to not refill the list but by changing the "key" to make sure I always attach a new one.

christian-baumann commented 4 years ago

Here is a simple example to reproduce the bug. It happens when clicking on the button to replace the list items.

import React from 'react';
import { Provider, defaultTheme, Button, Flex, ListBox, Item, Text } from '@adobe/react-spectrum';
import './App.css';

function App() {
  const options1 = [
    { id: 1, name: 'Aardvark', description: 'Aardvark Description' },
    { id: 2, name: 'Cat', description: 'Cat Description' },
    { id: 3, name: 'Dog', description: 'Dog Description' },
    { id: 4, name: 'Kangaroo', description: 'Kangaroo Description' },
    { id: 5, name: 'Koala', description: 'Koala Description' }
  ];
  const options2 = [
    { id: 6, name: 'Penguin', description: 'Penguin Description' },
    { id: 7, name: 'Snake', description: 'Snake Description' },
    { id: 8, name: 'Turtle', description: 'Turtle Description' },
    { id: 9, name: 'Wombat', description: 'Wombat Description' }
  ];
  const [ items, setItems ] = React.useState(options1);
  const changeOptions = () => {
    setItems(options2)
  }

  return (
    <Provider theme={defaultTheme}>
      <Flex direction="column">
        <ListBox items={items}>{(item) => (
          <Item key={'item' + item.id} textValue={item.name}>
            <Text>{item.name}</Text>
            <Text slot="description">{item.description}</Text>
          </Item>)}
        </ListBox>
        <Button variant="cta" onPress={changeOptions}>Change options</Button>
      </Flex>
    </Provider>
  );
}
export default App;
LFDanLu commented 4 years ago

@christian-baumann Thanks for the code snippet, but I'm having difficulties reproducing the bug with it. Clicking the button properly replaces the items, at least with adobe/react-spectrum: 3.2.0 and adobe/react-spectrum: 3.1.0 (https://codesandbox.io/s/eloquent-https-n7fh5?file=/src/App.js).

tomsontom commented 4 years ago

I can reproduce the problem using react-spectrum and i instrumented the useOption-code js-code a bit and I see the following logged:

Bildschirmfoto 2020-09-10 um 14 17 04

The code I added is:

console.debug("Update: ", "key:", key, "collection:", state.collection, "item: ", state.collection.getItem(key))

when the list is replaced it still sends "item1" as the key to the useOptions() who is not there any more hence it fails.

tomsontom commented 4 years ago

another interesting observation is - if I fix the index access by changing the if

if (isVirtualized && state.collection.getItem(key)) {

the list is not refilled until i resize the window

tomsontom commented 4 years ago

@LFDanLu I can only reproduce the problem if I run it in a standalone react application. I'm quite sure this is connected to #1064 who eg works inside the storybook but fails in a standalone application

christian-baumann commented 4 years ago

@LFDanLu Originally I have tested it with version 3.3.0 but I have also tested it with 3.2.0 and 3.1.0. The error happens with both, Firefox and Chrome. I have pushed a small sample project to reproduce the problem: https://github.com/christian-baumann/spectrum-list-box-test

LFDanLu commented 4 years ago

@tomsontom @christian-baumann Ahh, thanks, I can see the error now. Interesting that it doesn't happen in my codesandbox but it does in the react-create-app, will have to dig

LFDanLu commented 4 years ago

Seems to only happen when a wrapping <React.StrictMode> is applied, will need to see how the useOption call behavior differs between the two.

mischnic commented 4 years ago

Umbrella issue for strict mode: https://github.com/adobe/react-spectrum/issues/779