react-autocomplete icon indicating copy to clipboard operation
react-autocomplete copied to clipboard

Uncontrolled component?

Open mikelambert opened this issue 8 years ago • 3 comments

It appears this component cannot be used in uncontrolled fashion, and requires that you specify value= and control it from outside state. I'm curious if this has to be the case, or if you'd be open to changes to using it in an uncontrolled fashion?

Given that the state is also stored inside the form, I'm happy to have my code submit that form, or extract data from that form directly.

In particular, I'm noticing my webapp being really laggy on my mobile device, and am exploring causes. One potential cause being the overhead on each onChange event (that causes my form to re-render just to pass down a new value=).

I saw this was briefly brought up in https://github.com/reactjs/react-autocomplete/issues/113 , but was then quickly discarded.

mikelambert avatar Aug 24 '17 21:08 mikelambert

Hi Mike! In the very beginning this component was in fact uncontrolled. The main problem we had at that point was that it was in effect impossible to update the component's value without directly referencing the (undocumented) internal ref state. We decided to make the component controlled with the logic that if it's controlled, at least you have the option of wrapping it with another component to make it uncontrolled.

That's typically the advice I've given to people who need an uncontrolled component: create another component which stores value in its own state, then expose defaultValue and whichever other prop you need to get it bootstrapped.

Now I realise that that's slightly more work, and people are going to end up writing very similar code to achieve this. In my experience I almost always end up writing a bespoke wrapper which takes care of app-specific logic, layout, and styling, so adding the code to handle value is only 3-4 extra lines.

When it comes to performance: Even if we maintained value within Autocomplete, chances are we would have to re-render the entire component whenever value changes (due to the nature of how the items are filtered and displayed), so I don't know if there would be much of a gain by doing this. I would rather try to see if you can wrap Autocomplete in PureComponent to see if you can avoid unnecessary tree creation and diffing.

This feature request keeps coming up every few months, and it's definitely something we have in the back of our minds. It will probably be on the v2 roadmap somewhere, if the grand design allows it.

CMTegner avatar Aug 29 '17 18:08 CMTegner

Very good point about performance, that even in uncontrolled mode, it stills requires a re-render each time the value changes anyway. Doh!

I suspect it will be a bit faster since only renderMenu needs to be re-rendered, not the <input>, and not the surrounding React component that might otherwise manage state. And it avoids the need to do additional callbacks. But not too much.

(Naively, my state was stored in a FormComponent, which caused multiple AutoCompletes to needlessly re-renderer. I've since split up my state into multiple components, though PureComponent would have worked too.)

However, I don't think it's possible to wrap this as an uncontrolled component. Adding that support, would also be a valid solution that would work for me.

To do this, react-autocomplete would need to:

  • Support passing defaultValue instead of value to <input .../>. This is now possible as of a week ago in 1.6 with renderInput (I didn't realize it at the time I filed this, since I was running 1.5.x), with this:
          renderInput={args => {
            const { value, ...otherArgs } = args;
            return <input defaultValue={this.props.value} {...otherArgs} />;
          }}
  • Not depend on props.value. Replacing such references with an accessor that returns props.value !== undefined ? props.value || this.refs.input.value would work better. (Or maybe an props.valueAccessorFunction I can override, that does the same thing?)

I have also checked a second iPhone, and noticed that my newer iPhone appears to have serious performance issues overall. And so it might be unfair for my to judge the performance here off my possibly-broken device.

mikelambert avatar Aug 30 '17 04:08 mikelambert

Wether or not the component should be allowed uncontrolled or not aside. The documentation does suggest this is the case because it states that the value attribute is optional.

When I do omit the value attribute te component does not render and throws an error.

midgethoen avatar Mar 06 '19 14:03 midgethoen