material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

Autocomplete race condition / resets state "at random"

Open joshribakoff-sm opened this issue 1 year ago • 4 comments

Duplicates

  • [X] I have searched the existing issues

Latest version

  • [X] I have tested the latest version

Steps to reproduce 🕹

It's very hard to replicate, as it depends on the timing of network requests and React renders, but the logic is easily proven faulty via deduction.

Here, you do a shallow equality check: https://github.com/mui/material-ui/blob/master/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.js#L199

value !== prevValue.current

Regardless of the value of freeSolo, you call resetInputValue as long as the inequality check returns true. In the case freeSolo is not in use, you always reset the input. In the case the user is using objects for the values instead of strings, this check will always return true after a render even if the values are still the same (since it compares by reference) and will reset the input. It needs to do a deep equality check (compare by value)

The result is that after the user (or automated test) enters a keyboard input event and the DOM element updates, MUI internally resets the input and displays "no options" and the user or automated test experiences a defect where an empty input value is observed, and the expected results are hidden.

Current behavior 😯

Autocomplete randomly resets the input and shows "no results" after a keyboard input event that triggers async requests.

Expected behavior 🤔

No random flaky failures in my e2e tests because of MUI non deterministically resetting the input.

Context 🔦

My e2e tests started failing, and I was able to set a conditional breakpoint to confirm my onInputChange is being called with null event. The stack trace made it apparent this is due to a particular line in the MUI codebase and not something I directly have control over (other than rewriting my code to avoid the use of "objects as values" I suppose).

Your environment 🌎

n/a

joshribakoff-sm avatar Dec 13 '22 20:12 joshribakoff-sm

Also, here is proof via screenshot that it's resetting my input. You can see the value and prevValue.current are the same when serialized to JSON, yet it's calling my onInputChange with an empty value.

Screen Shot 2022-12-13 at 12 24 24 PM

joshribakoff-sm avatar Dec 13 '22 20:12 joshribakoff-sm

What I think happened is there is an autocomplete with multiple prop set, and a value prop supplied to the autocomplete which is the result of asynchronous query. When the user makes a selection, we run the query which eventually updates the value prop. If the value prop changes while the user is typing, MUI will internally reset the input even if the user is trying to type.

As a workaround, I was able to manually reset my own input value only when the user has just made a selection. Later, when the user's new selection causes the query to run and update the value prop which triggered the MUI reset logic, I ignore the MUI triggered resets:

onChange={(_event, newValue) => {
          onAdded(newValue);
          // we reset the controlled input ourselves, see https://github.com/mui/material-ui/issues/35467
          setInputValue('');
        }}
onInputChange={(event, newInputValue, reason) => {
          // ignore reset triggered by MUI, see https://github.com/mui/material-ui/issues/35467
          if (reason !== 'reset') {
            setInputValue(newInputValue);
          }
        }}

joshribakoff-sm avatar Dec 14 '22 22:12 joshribakoff-sm

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://material-ui.com/r/issue-template-latest), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem.

Here are some tips for providing a minimal example: https://stackoverflow.com/help/mcve

mnajdova avatar Dec 16 '22 13:12 mnajdova

https://codesandbox.io/s/youthful-raman-hmim2x?file=/demo.tsx to replicate, simply try to type and every second the text you have entered will be cleared.

As mentioned in my previous post, this is specific to when the value is updated asynchronously while the user is typing (causes inputValue to be reset). I also notice any initial state I set for inputValue is cleared almost immediately.

Why do I need to do it this way? What if the user reloads the page with an existing "value" stored in the URL, but I have to do an async query to hydrate the "value"? I don't want to represent my value by fetching async data, copying it into a separate state and then manipulating that separate state to push in hydrated entries synchronously when the onChange fires. Copying the results of a query into a separate React state and then manipulating that state adds a ton of undesired complexity and is not idiomatic React to me.

Instead, I want one single source of truth for the user's "selection", I want onChange to update it and trigger an async query, and I want that async query to resolve the hydrated entries for the updated value. There should only be one source of truth for the hydrated value (the results of an async query). I do not want to fetch a value from the server and then manipulate it after the fact as then we are dealing with derived state

Worded differently, I want one React state that represents the user's "selection" as an array of "movie IDs". I want a separate query that watches the "selection" and resolves the hydrated list of movies which then "eventually" updates the MUI autocomplete value. In my real app there may also be too many "movies" so I don't have the full list fetched on the client, I only fetch suggestions as they type with one query, and I fetch the selected movies with a second query.

Another piece of context I can share is that in my use case, I don't really have any need for MUI to be aware of a value. I basically just wanted a search box with auto suggestions, and I will handle showing the user their current selection on my own. Unfortunately if you remove the value prop then MUI just maintains its own value internally which goes out of sync with my value, introducing even worse bugs. I did you see you have a "search input" example here but that is not my idea of a "search input". My idea of a "search input" has only inputValue and has no concept of value.

joshribakoff-sm avatar Dec 16 '22 18:12 joshribakoff-sm