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

useCombobox: crash when using itemToString, and selectedItem is reset to null #975

Closed
rlue opened this issue Mar 17, 2020 · 3 comments
Closed
Labels

Comments

@rlue
Copy link
Contributor

rlue commented Mar 17, 2020

  • downshift version: 5.0.5
  • node version: 10.17.0
  • yarn version: berry (2.0.0-rc.30)

Relevant code

See reproduction repro below, or view the JS file in question.

What you did

Video demo here

  1. Use non-String values for item list;

    // not this
    ['Alabama', 'Alaska', 'Arizona', 'Arkansas', 'California', 'Colorado', 'Connecticut']
    
    // but this
    [
      { name: 'Alabama' },
      { name: 'Alaska' },
      { name: 'Arizona' },
      { name: 'Arkansas' },
      { name: 'California' },
      { name: 'Colorado' },
      { name: 'Connecticut' }
    ]
  2. which requires use of the itemToString prop;

    itemToString: ({ name }) => name,
  3. then, use the combobox in the browser;

  4. enter a value, make a selection, repeat this as much as you like without any issue;

  5. enter a value, highlight an item, hit escape

What happened

Kaboom, React crashes (with message TypeError: _ref is null).

Reproduction repository

https://github.com/rlue/downshift-debug

Problem description

I don't know enough about downshift internals to say for sure what's wrong here. I wonder if perhaps it's related to #719?

In any case, it appears that the following combination of conditions causes React to crash:

  1. using a downshift combobox,
  2. using items which are not plain strings,
  3. thus the using itemToString prop,
  4. after the selectedItem state has already been set on the combobox,
  5. when resetting selectedItem to null (by highlighting a search result and pressing <Esc>).

Suggested solution

No idea! Happy to take guidance on this point, though.

@silviuaavram
Copy link
Collaborator

well you did point out that selectedItem beocomes null, and you cannot destructure null into anything. so try itemToString = item => item ? item.name || ''

Oh and not sure if it's intentional, but in your use case try to use all combobox components (input, menu, items + combobox and maybe toggleButton and label) just to make the experience 100% accessible.

Let me know how it goes.

@rlue
Copy link
Contributor Author

rlue commented Mar 17, 2020

well you did point out that selectedItem beocomes null, and you cannot destructure null into anything. so try itemToString = item => item ? item.name || ''

I am embarrassed to say that this was absolutely it. Spent my whole day scratching my head over this.

Would you accept a PR from me that adds a note to the documentation about this? As a first-time user, I was not aware that the itemToString method had to accommodate null values.

(Or better yet, would there be a way to modify wherever downshift uses itemToString to defensively convert null values to ""?)

@rlue rlue closed this as completed Mar 17, 2020
@silviuaavram
Copy link
Collaborator

sure @rlue you can create a PR but make sure you address both hooks and Downshift

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants