Picker fails with Cannot read property 'key' of null

[matej-hron]

🐛 Bug Report

We are using Picker in CEP panel in Adobe Illustrator. We have ~300 Items in the Picker so that they can not fit into single Picker page. After scrolling and selecting an item the Picker sometimes throws an error. It is not possible to write exact steps to reproduce. The problem appear 'randomly'. With small number of Items the problem does not appear as all items fit into single page of Picker - assumably because the Picker does not need to use Virtualizer.

🤔 Expected Behavior

It is possible to select item from the Picker without getting an Error.

😯 Current Behavior

When the problem appears we see this call stack
Uncaught TypeError: Cannot read property 'key' of null at useVirtualizerItem (module.js:299) at VirtualizerItem (module.js:324) at renderWithHooks (react-dom.development.js:14803) at updateFunctionComponent (react-dom.development.js:17034) at beginWork (react-dom.development.js:18610) at HTMLUnknownElement.callCallback (react-dom.development.js:188) at Object.invokeGuardedCallbackDev (react-dom.development.js:237) at invokeGuardedCallback (react-dom.development.js:292) at beginWork$1 (react-dom.development.js:23203) at performUnitOfWork (react-dom.development.js:22157) useVirtualizerItem @ module.js:299 VirtualizerItem @ module.js:324 renderWithHooks @ react-dom.development.js:14803 updateFunctionComponent @ react-dom.development.js:17034 beginWork @ react-dom.development.js:18610 callCallback @ react-dom.development.js:188 invokeGuardedCallbackDev @ react-dom.development.js:237 invokeGuardedCallback @ react-dom.development.js:292 beginWork$1 @ react-dom.development.js:23203 performUnitOfWork @ react-dom.development.js:22157 workLoopSync @ react-dom.development.js:22130 performSyncWorkOnRoot @ react-dom.development.js:21756 (anonymous) @ react-dom.development.js:11089 unstable_runWithPriority @ scheduler.development.js:653 runWithPriority$1 @ react-dom.development.js:11039 flushSyncCallbackQueueImpl @ react-dom.development.js:11084 flushSyncCallbackQueue @ react-dom.development.js:11072 scheduleUpdateOnFiber @ react-dom.development.js:21199 dispatchAction @ react-dom.development.js:15660 (anonymous) @ predefinedTextPanel.tsx:68 step @ predefinedTextPanel.tsx:52 (anonymous) @ predefinedTextPanel.tsx:33 fulfilled @ predefinedTextPanel.tsx:24 Promise resolved (async) step @ predefinedTextPanel.tsx:26 (anonymous) @ predefinedTextPanel.tsx:27 __awaiter @ predefinedTextPanel.tsx:23 updateElement @ predefinedTextPanel.tsx:62 (anonymous) @ predefinedTextPanel.tsx:109 step @ predefinedTextPanel.tsx:52 (anonymous) @ predefinedTextPanel.tsx:33 (anonymous) @ predefinedTextPanel.tsx:27 __awaiter @ predefinedTextPanel.tsx:23 onSelectionChange @ predefinedTextPanel.tsx:106 onChangeCaller @ module.js:31 (anonymous) @ module.js:67 onSelectionChange @ module.js:33 onChangeCaller @ module.js:31 (anonymous) @ module.js:67 replaceSelection @ module.js:338 onSelect @ module.js:399 itemProps.onPressUp.e @ module.js:445 triggerPressUp @ module.js:153 pressProps.onTouchEnd.e @ module.js:486 callCallback @ react-dom.development.js:188 invokeGuardedCallbackDev @ react-dom.development.js:237 invokeGuardedCallback @ react-dom.development.js:292 invokeGuardedCallbackAndCatchFirstError @ react-dom.development.js:306 executeDispatch @ react-dom.development.js:389 executeDispatchesInOrder @ react-dom.development.js:414 executeDispatchesAndRelease @ react-dom.development.js:3278 executeDispatchesAndReleaseTopLevel @ react-dom.development.js:3287 forEachAccumulated @ react-dom.development.js:3259 runEventsInBatch @ react-dom.development.js:3304 runExtractedPluginEventsInBatch @ react-dom.development.js:3514 handleTopLevel @ react-dom.development.js:3558 batchedEventUpdates$1 @ react-dom.development.js:21871 batchedEventUpdates @ react-dom.development.js:795 dispatchEventForLegacyPluginEventSystem @ react-dom.development.js:3568 attemptToDispatchEvent @ react-dom.development.js:4267 dispatchEvent @ react-dom.development.js:4189 unstable_runWithPriority @ scheduler.development.js:653 runWithPriority$1 @ react-dom.development.js:11039 discreteUpdates$1 @ react-dom.development.js:21887 discreteUpdates @ react-dom.development.js:806 dispatchDiscreteEvent @ react-dom.development.js:4168 Show 35 more frames

💁 Possible Solution

🔦 Context

The whole application crashes.

💻 Code Sample

this is our Picker definition:
<Picker aria-label="Select Product" gridRow="2" gridColumn="2" items={predefinedTextItems} selectedKey={selectedPredefinedTextId} onSelectionChange={async (id: string) => { console.log('selection changed ' + id); setSelectedPredefinedTextId(id); await updateElement(id, culture, contentInt); }} > {(item) => ( <Item key={item.reusableFieldId}> {${item.purposeName}${item.textSelector == 'default' ? '' : '/' + item.textSelector}} </Item> )} </Picker>
` const updateElement = async (predefinedTextId: string, culture: string, cnt: string) => {
const selectedTextItem = findPredefinedTextById(predefinedTextId);

if (culture && selectedTextItem) {
  const localizedText = await getLocalizedText(culture, selectedTextItem.textKey);
  if (localizedText != null) {
    setContentInt(localizedText);
    onChange(culture, selectedTextItem, localizedText);
  }
} else {
  onChange(culture, selectedTextItem, cnt);
}

};
`

🌍 Your Environment

react-spectrum 3.0.0
CEP panel in Ilustrator
Windows 10
System |

🧢 Your Company/Team

Cimpress

🕷 Tracking Issue (optional)

[snowystinger]

Are you able to reproduce it here? https://react-spectrum.adobe.com/react-spectrum/Picker.html#asynchronous-loading this will eventually load well over 300, I've no idea how many pokemon there are, but it's definitely over 900 these days I think.

Any chance you could better format the Picker definition so it's easier to read?

[matej-hron]

Thanks for reaching out! I will look to the example you have sent.
In the meanwhile this is better formated picker:

        <Picker
          aria-label="Select Product"
          gridRow="2"
          gridColumn="2"
          items={predefinedTextItems}
          selectedKey={selectedPredefinedTextId}
          onSelectionChange={async (id: string) => {
            console.log('selection changed ' + id);
            setSelectedPredefinedTextId(id);
            await updateElement(id, culture, contentInt);
          }}
        >
          {(item) => (
            <Item key={item.reusableFieldId}>
              {`${item.purposeName}${item.textSelector == 'default' ? '' : '/' + item.textSelector}`}
            </Item>
          )}
        </Picker>

and the updateElement function which is called in onSelectionChanged:

  const updateElement = async (predefinedTextId: string, culture: string, cnt: string) => {
    const selectedTextItem = findPredefinedTextById(predefinedTextId);

    if (culture && selectedTextItem) {
      const localizedText = await getLocalizedText(culture, selectedTextItem.textKey);
      // const localizedText = selectedTextItem.purposeName;
      if (localizedText != null) {
        setContentInt(localizedText);
        onChange(culture, selectedTextItem, localizedText);
      }
    } else {
      onChange(culture, selectedTextItem, cnt);
    }
  };

[matej-hron]

Actualy I will share source for the whole component
https://gist.github.com/matej-hron/e67a04a88efee9b6ba99a2580d35c864#file-predefinedtextpanel

[snowystinger]

weird question, but why bother with having this async and await?

         onSelectionChange={async (id: string) => {
            console.log('selection changed ' + id);
            setSelectedPredefinedTextId(id);
            await updateElement(id, culture, contentInt);
          }}

nothing is actually waiting for updateElement, the function is done at that point

Without knowing a whole lot, I think you have some races potentially. I'd probably construct this a little more like so
incoming psuedocode

PredefinedTextPanel () {
  let const [selectedCulture, setSelectedCulture] = useState('en-US');
  useEffect(() => {
    let cancelUpdate = updateElement(selectedCulture);
    return () => cancelUpdate();
  }, [selectedCulture]);
  return (
    <Picker onSelectionChange={(id: string) => setSelectedPredefinedTextId(id)}></Picker>
  )
}

The general idea being the Picker just handles its selection and that's it, then in a useEffect you update the async stuff, that way you have cleanup capabilities.
Same kind of pattern with the second Picker, and if both result in a call to updateElement, you can put both selectedCulture and selectedPredefinedTextId into the useEffect dependency array so that it'll get called just once for any combination of those two getting updated

Obviously I've left out a lot of code, but this is to illustrate the general pattern

[matej-hron]

Hello @snowystinger I have put the async logic into useEffect but the problem still appears.

the updated code gist is here
https://gist.github.com/matej-hron/e67a04a88efee9b6ba99a2580d35c864

[snowystinger]

So cultureChanged is not listed as a dep of your useEffect, which means that you won't get a new one if culture changes, so you may have some old values going around, you should memo that cultureChanged function and add it to your useEffect dep list
https://gist.github.com/matej-hron/e67a04a88efee9b6ba99a2580d35c864#file-predefinedtextpanel-L58

I think you're doubling a lot of effort that we've put in to make it easier to manage async lists, have you checked out https://react-spectrum.adobe.com/react-stately/useAsyncList.html ?

otherwise a lot of the useEffect dependency lists don't seem to be complete, which may be totally on purpose, but i don't have enough info/comments to be able to tell

[mischnic]

Looks like this is caused by strict mode (https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects):

diff --git .storybook/config.js .storybook/config.js
index 1a40f7308..b35a2eef0 100644
--- .storybook/config.js
+++ .storybook/config.js
@@ -29,6 +29,12 @@ addDecorator(story => (

 addDecorator(withProviderSwitcher);

+addDecorator(story => (
+  <React.StrictMode>
+    {story()}
+  </React.StrictMode>
+))
+

 function loadStories() {
   let storiesContext = require.context('../packages', true, /^(.*\/stories\/.*?\.(js|jsx|ts|tsx))$/);

Then closing the dropdown menu on http://localhost:9003/?path=/story/combobox--static-items crashes with the same error.

[devongovett]

Closing since strict mode should work in the next release as of #4564. If this is still a problem after the next release/nightly let us know.