React: Should React warn when controlled <select> components have duplicate values?

[jbinto]

Consider the following example (live at http://www.webpackbin.com/Vk0Q_FqmZ with React 15.0.1):

const MySelect = (props) => (
  <select value={this.props.value} onChange={this.props.onChange}>
    <option value="fruit">banana</option>
    <option value="vegetable">broccoli</option>
    <option value="fruit">orange</option>
    <option value="vegetable">tomato</option>
  </select>
);

class App extends React.Component {
  constructor() {
    super()
    this.state = {}
    this.onSelectChange = this.onSelectChange.bind(this)
  }

  onSelectChange(e) {
    this.setState({ selectedValue: e.target.value })
  }

  render() {
    return <MySelect 
      value={this.state.selectedValue} 
      onChange={this.onSelectChange}
    />
  }
}

HTML itself doesn't seem to have any problems with duplicate values in the <select> tag. As you can see in the example above, there are semantically valid reasons you might want to do this.

But this controlled component example makes it impossible to select e.g. "orange" or "tomato": the selection will be forced to the first matching value.

I thought about a solution that stores both the selectedValue and the selectedIndex, but react-dom's <select> does not take any sort of index prop, so I'd have to manipulate the internal DOM node, which I'm sure is bad form.

React _could_ be enhanced to take a selectedIndex, but I foresee problems with single source of truth.

The fruit/vegetable example is a bit contrived, and there may not be many real-world scenarios you would want to do this in. My gut feeling is this should be a warning so developers (like me) don't paint themselves into a corner trying to support this case.

[jimfb]

Honestly, this would be pretty low priority for us. But yeah, in an ideal world, we would probably emit a warning.

Feel free to submit a PR.

[nikhilshagri]

@jimfb I could take this up. Hopefully, I should submit a PR within 4-5 days.

[zeel]

@cynicaldevil can I help ? I would also like to contribute :)

[chicoxyzzy]

Why should it warn instead of selecting proper option as DOM does?
http://jsbin.com/mesurekaso/edit?html,output

I think it's better to have consistency

[michelgotta]

I see the problem here but I think it's more a problem with the design of the code itself. Instead of using the value attribute of the <select> and directly binding it to the value, you could use the selected attribute of the <option> to set the initial selected value (or don't set an initial value at all) and somehow detach the binding. React can't determine which of the options is selected just by the value of the value attribute

I tweaked your webpackbin, maybe it helps to understand what I'm trying to explain:
http://www.webpackbin.com/EkXrq3nrW

[SalehHindi]

@cynicaldevil, @zeel, are you still working on this issue? Could you link the PR? I'd like to work on this bug too. Just let me know how I can help.

[nikhilshagri]

@SalehHindi @zeel I am not working on this issue, so I think you two can take it up!

[chicoxyzzy]

This is a bug in React. We shouldn't warn, we should fix <select> behaviour to eliminate inconsistency cc @jimfb

[zeel]

I am taking up this issue .. How can I start ?

[syranide]

@zeel Sadly it's already being worked on #7054

[git-richard]

@zeel I started working on this about 2 weeks ago. #7054. Sorry, I forgot to mention it in here.

[zeel]

@michelgotta then it will become uncontrolled component. @git-richard no problem. will go through your code :)

[git-richard]

7054 shows a warning the first time it finds a select element with a duplicate value. If there is a following select that also has duplicate values it will not display a warning. Is this correct behavior? Should it show a warning for all duplicates or just the first one it finds?

[zeel]

@jimfb @git-richard following up on this pr #7054. Please let us know how we can merge these?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!