tbleckert / react-select-search

⚡️ Lightweight select component for React
https://react-select-search.vercel.app
MIT License
678 stars 147 forks source link

SelectSearch ignores not changed value after async onChange #169

Open Ronny25 opened 3 years ago

Ronny25 commented 3 years ago

Hi!

It seems that SelectSearch ignores not updated value prop after async onChange.

Let say the value is '1', onChange calls API and (after validation or whatever reason) value didn't actually update then SelectSearch still shows previously clicked option as a selected one. Which is wrong. So it looks like the internal state is not updated and it mismatches with the provided value from now on.

My workaround is to use a key prop but it's masking the actual problem in the end. 🤷🏻

AlexBasile123 commented 3 years ago

@Ronny25, I'm looking at a similar issue.

Seems like value is set when an option is selected, and not when user types into the text box. Then clicking away from SelectSearch component shows the name associated w/ the value. If the user clicked away after typing in the text box but didn't select an option, then the text isn't preserved.

Thus, text box only shows names associated w/ a selected option, and that any typed in value is transitive (doesn't get persisted unless option is selected).
I think I can live w/ that behavior, but want to confirm that it's intended to work that way, AND there's no other option (w/o adding custom code).

I'm looking at an additional issue, that I can't deselect an option. That is, neither option doesn't unhighlight nor text in textbox cleared.

<SelectSearch
    value={selectedValue}
    className="select-search select-search--multiple"
    options={searchEntities}
    renderOption={renderOption}
    search
    closeOnSelect={false}
    placeholder="Search something"
    onChange=
        {(optionSelected) => {
            lock = true; // added my own guard to limit state change race conditions
            setSearchQuery(optionSelected);
            lock = false;
        }
    }
    filterOptions={fuzzySearch} // my own implementation that creates the options and changes state
/>

Hoping the admins can comment/advise :-)

icaroscherma commented 3 years ago

@Ronny25 can you post the block of code like @AlexBasile123 did? It seems that he had a different issue all along and your issue seems to be related to react state management, not the component itself.

You can play around with one example in this URL: https://codesandbox.io/s/select-search-3-test-skjec .

Ronny25 commented 3 years ago

Hi @icaroscherma Yes. Please check this sandbox https://codesandbox.io/s/select-search-3-test-forked-4o65y I also added logs for an easier demonstration of what the issue is.

Maybe you can suggest a better way of handling such a situation. Because in this simple example React is not actually updating the state (I believe). So how then we can change value to the previous one when it's expected behaviour?

icaroscherma commented 3 years ago

I see what you're saying. The snapshot.displayValue it's changed before the actual State is changed.

To prevent this you can either:

[edit]

If you take a look here, onSelect it's what triggers your onChange, BUT if your value changes, it will be triggered again. https://github.com/tbleckert/react-select-search/blob/main/src/useSelect.js#L39-L53

Ronny25 commented 3 years ago

@icaroscherma onSelect will be triggered only when value has changed - it means when it's a primitive string it won't detect change (when it has previous value). So I would maybe stick to using key prop. Seems a bit cleaner as for me instead of using additional state or monkeypatching. Thanks.

tbleckert commented 3 years ago

Sorry for the late reply @Ronny25 . Yes, seems like something has broken with the controllable state. I'll take a look