Autocomplete with freeSolo and custom onChange breaks on Enter key pressed

[oziniak]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

I use Autocomplete with redux, to simulate async search. search is provided from the redux store, when action fired with onSearchCustom gets to the reducer and changes the state. Everything works, I see results populated into Combobox.

        <Autocomplete
            freeSolo
            filterOptions={(option) => option}
            options={search}
            renderInput={(params) => {
                const {
                    inputProps: { onChange: ownOnChange }
                } = params as any;

                return (
                    <TextField
                        {...params}
                        inputProps={{
                            ...params.inputProps,
                            onChange(event: any) {
                                onSearchCustom({ limit: 5, search: event.currentTarget.value });
                                return ownOnChange(event);
                            }
                        }}
                    />
                );
            }}
        />

But when I press Enter, everything breaks with this error.

And I can't use event.keyCode, event.charCode, event.key in my onChange handler to input, because everything is undefined there.

Expected Behavior 🤔

It would be great if I could use a component to fit my needs. I was hoping that if I put any plug on Enter key in my onChange handler to input, but I can't even check if that would work, because I can't tell if Enter is pressed or any other key.

Steps to Reproduce 🕹

https://codesandbox.io/s/create-react-app-with-typescript-1te6e
Steps:

1.Write any text, it works ok
2. Press enter, it breaks with error in console

Context 🔦

Your Environment 🌎

Tech	Version
Material-UI	v4.3.2
Material-UI lab	^4.0.0-alpha.30
React	^16.8.6
Browser	chrome 78.0.3904.70
TypeScript	3.5.2
etc.

[oliviertassinari]

@oziniak Thank you for the report! What do you think of this fix?

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 7f28ddca2..d7394c18b 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -166,6 +166,8 @@ export default function useAutocomplete(props) {
     let newInputValue;
     if (multiple) {
       newInputValue = '';
+    } else if (freeSolo && typeof newValue === 'string') {
+      newInputValue = newValue;
     } else {
       newInputValue = newValue != null ? getOptionLabel(newValue) : '';
     }

Do you want to submit a pull request? :)

[ad-das]

Is PR #18117 related to this? If yes, has it been shipped yet?

[oliviertassinari]

@ad-das #18117 solves a different problem (with the multi-select). You can check the added test case to get a better idea of the problem it solves.

This bug is different, it's about using free solo with rich (object) options, it's a combination I didn't encounter during the initial development of the component.

[oziniak]

@oliviertassinari I'll be able to submit a PR in an hour or so

[oliviertassinari]

@oziniak Awesome! If you struggle with the addition of a test case, I can have a look at it.

[oziniak]

@oliviertassinari not sure why, but I can't make a test to fail.

  describe.only('free solo', () => {
    it('should not crash with custom onChange for input', () => {
      const noop = () => {};
      const { container } = render(
        <Autocomplete freeSolo renderInput={(params) => (
          <TextField
            {...params}
            label="defaultProps"
            inputProps={{
              ...params.inputProps,
              onChange(event) {
                noop(); // simulate custom onChange logic
                return params.inputProps.onChange(event);
              }
            }}
          />
        )} />);
      const input = container.querySelector('input');
      fireEvent.keyPress(input, {key: "Enter", code: 13, charCode: 13});
    });
  });

[eps1lon]

@oziniak Try focusing the input first and then fireEvent.keyDown(document.activeElement)

[oziniak]

@eps1lon @oliviertassinari is it possible that there's some caching in built/compiled files? because I'm facing the near-to-magic issue. I've removed the fix from useAutocomplete.js (and double check that changes are not in browser files). Currently, when I do git status, I get:

	modified:   docs/src/pages/components/autocomplete/FreeSolo.js
	modified:   docs/src/pages/components/autocomplete/FreeSolo.tsx

But the issue is FIXED somehow, and it drives me crazy 😄 Maybe that's why I can't manage to break the tests.

[oliviertassinari]

@oziniak I'm not aware of any caching logic that might impact here. Can you try this reproduction?

  describe.only('free solo', () => {
    it('should accept any value', () => {
      const handleChange = spy();
      const { container } = render(
        <Autocomplete
          freeSolo
          options={[{ name: 'test' }, { name: 'foo' }]}
          onChange={handleChange}
          renderInput={params => <TextField {...params} />}
        />,
      );
      const input = container.querySelector('input');
      fireEvent.change(input, { target: { value: 'a' } });
      fireEvent.keyDown(input, { key: 'Enter' });
      expect(handleChange.callCount).to.equal(1);
      expect(handleChange.args[0][1]).to.equal('a');
    });
  });

[ronaldporch]

I've also ran into a similar issue, but I assume it's rooted from the same problem. You can even reproduce it on this example. Select another choice, then press Enter. The error is different from what was reported on this though.

TypeError: Cannot read property 'title' of undefined

According to the sample of code, the first two AutoCompletes have no custom onChange or freeSolo.

Edit: I see that there are other closed issues related to this, so assume that it is well known.

#18110

[oziniak]

@ronaldporch yeah, in my example I used this hack to intercept onChange for text input.

                        inputProps={{
                            ...params.inputProps,
                            onChange(event: any) {
                                // onSearchCustom(....);
                                return params.inputProps.onChange(event);
                            }
                        }}

@oliviertassinari I've ran into issue where I can no longer reproduce a bug on local dev environment. I'm trying to test it on docs/src/pages/components/autocomplete/FreeSolo.tsx.
I'm running yarn start in one tab and yarn docs:typescript --watch in another and testing on a page http://localhost:3000/components/autocomplete#free-solo. And I can't reproduce the issue from the OP post. This is some kind of magic. I have no changes in packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js. I've even tried to clone the second time. Still can't reproduce. Magic.

[oziniak]

Ok, so I investigated: the issue with breaking of autocomplete with freeSolo on enter pressed, appears only when you have options as an array of objects, for example [{name: "a", id: 0}, {name: "b", id: 1}], and using getOptionLabel={option => option.name}

[oziniak]

OK, nevermind my above comments. I wasn't reading the thread carefully and only now I understood that the issue from the start was in having options as not an array of strings.

[ad-das]

it looks like somehow the demo page of freeSolo component doesn't break now on pressing enter. It was definitely breaking if you enter text -> selected an option, -> press enter at least a couple days ago. Did it somehow get fixed? 🤔

[oliviertassinari]

@ad-das This issue is not visible in the documentation. What you report is different.

[vash500]

I am running into the exact same issue.

If I remove either of freeSolo or onChange custom values, the problem is not visible (but neither is the expected functionality).
This seems related to the this pull request #18161, but I am using @material-ui/core 4.11.0 (which should already have this patch) and I can still reproduce this consistently.

[oliviertassinari]

@vash500 Do you have a reproduction?

[vash500]

Sorry for the delay, I had not worked on this project for a whole week. So, it turns out that I am full of stupid and what I had is not a reproduction of this case.

When tried to construct a minimal working example I found that it was my onInputChange function that had a typo:

    const onInputChange = function(event, value) {
        if (typeof value === "unefined" || value === null) {
            return;
        }
        setMovieTitle(value.title)
    }

It says unefied instead of undefined. This led to setMovieTitle to be triggered with undefined, which ultimately was being queried for length. I caught it only thanks to the linter https://eslint.org/docs/rules/valid-typeof
Entirely my bad.