The defaultValue of <select> may be broken

[StoneCypher]

I would like to report a bug.

In re-filing another outstanding bug that was closed despite being reproducible, I decided to create a repro case.

Since the repro involves setting defaultValue on a <select>, I confronted the human inability to remember whether HTML is zero-indexed or one-indexed.

Since the repro also involves determining whether a given <option> is selectable, I decided to just have a control with four <option>s and to select 3, to see which I'd get - the third or the fourth.

Joke's on me. I got neither. I got the first selectable <option>.

A winner is testcase. If the series is 1-indexed you should see joe; if 0-indexed you should see stan. Instead you see bob.

As such I currently believe that defaultValue is not honored on select.

This is the first time I've noticed this. I have only tested it under 16.0.0.

[jquense]

Can you post the repro in js fiddle so we can read the code and mess with it? Thanks!

[StoneCypher]

@jquense - looks like someone added a fiddle to my three year old version of this on #2803

https://jsfiddle.net/84v837e9/140/

[jquense]

Thanks! Cc @aweary perhaps your reset PR could also cover this if we are saying it should. If it's a pain tho my instinct would be to recommend controlling the select for this case

[aweary]

@jquense looks like it's an issue with the whitespace in option's value. The value property on an option element strips whitespace so if you have " -- Select Eisenhower -- " it will fail to match due to the leading and trailing whitespace.

Here's that same example without the whitespace: https://jsfiddle.net/hkr4f5yr/ you can see it matches the native DOM behavior.

So the solution here would be separate from #11057. We could trim defaultValue when doing the comparison, or try to use something like innerText that doesn't trim the whitespace. I'm not sure yet what the best solution would be.

[StoneCypher]

@aweary - the replication you were given here by me does not have a space in the option's value, and remains broken.

I'm not sure why you were able to repair the other guy's silly and overcomplicated code that way.

The one I gave you is not subject to that form of fix

[StoneCypher]

I hope you'll consider testing against the bug reporter's actual test case

[jquense]

Can you please post your test case in jsfiddle if the one above doesn't cover the bug? Thanks

[StoneCypher]

I already said no to that.

This bug is four years old. I've already written you three productions.

You can cut and paste that if you want to.

[StoneCypher]

Maybe someone else will see this and act

[nhunzaker]

Hey @StoneCypher, I'm sorry that you've had a bad experience with our trouble shooting methods. We work through a ton of issues on the React repo and some times we just need a little help finding stuff.

If we can get a JSFiddle that reproduces this issue, I think that we could:

1. Diagnose the issue
2. Create a test case for it (or a manual DOM test fixture if we can't reproduce this)
3. Do our best to resolve the issue

Inputs and selects are a hot topic for the team right now, and we want to make them work as seamlessly as possible. We just need a little help.

[StoneCypher]

@nhunzaker - This is the third time I am declining to copy the working reproduction that you were already given into jsfiddle.

The last time someone asked I closed the ticket

[aweary]

that you were already given into jsfiddle.

@StoneCypher sorry, the only JSFiddle you provided was https://jsfiddle.net/84v837e9/140/ and I already identified what the issue was there.

If there's another JSFiddle in a previous issue it would really help us if you could just link us to it, we have hundreds and hundreds of closed issues so sometimes it's hard to go back and find one specific one.

[StoneCypher]

I've opened eight tickets about this so far. It's been four years.

Your team closed an active ticket about this without even investigating it five days ago.

You've gotten a lot more than a little help.

@nhunzaker - Open the link, open your console, observe the clear instructions that show the problem is real. Move forwards as you see fit.

I am not interested in copy and pasting the reproduction case in this ticket into jsfiddle. Please take quality more seriously. Please stop asking me to copy repro cases for you.

@aweary - The repro case you're ignoring shows that the thing you found is not correct. Please don't involve yourself on this ticket anymore. Thanks

[StoneCypher]

@aweary - and if you really can't find it?

ctrl+f testcase

it's in the issue. just read the issue

[aweary]

@StoneCypher you've only opened three issues with us, including this one and #11100. In the issue that was closed (#2803) you didn't provide a reproducible test case, but thankfully @nhunzaker spent the time putting together the JSFiddle you linked in this issue.

it's in the issue. just read the issue

There's a reason we're asking for a JSFiddle: we can't easily read the source code for your test case, and we can't make modifications to it to debug. Providing a hosted example without the source is not helpful to us.

Please understand that every single person here trying to help you is a volunteer doing this in our free time; making it easier for us to help you goes a long way. Acting rude only makes it more difficult for us to work together and resolve your issue, so please take that into consideration.

[StoneCypher]

@aweary - please disinvolve yourself from this ticket

it would be less work to make the fiddle yourself than to keep arguing with me. all you're doing is convincing me that i should no longer contribute to react.

i've opened a lot more than three issues with you. you just don't know who i am.

i'm not interested in getting the "we're just volunteers" speech. so am i. i don't get the credit for being a collaborator, i already did the work you want, and you're just too lazy to open it.

if you comment on this ticket again i'm closing it again and leaving it closed. you guys haven't been able to find this for four years, despite that it's one of the basic w3 test cases.

i'd be happy to leave react broken for another four years if the collaborators aren't interested enough in quality to cut and paste a single stretch of text a single time.

[nhunzaker]

JSFiddle:
https://jsfiddle.net/gqb0227f/

Semi-related codepen of native behavior:
https://codepen.io/nhunzaker/pen/LzeOPb

I am not sure if directly assigning value is accurate, but it does result in the select showing "blank", at least in chrome.

As far as I know, our internal methods involve finding the matching option and selecting it. Should we assign value directly? Does that work in every browser?

[StoneCypher]

@nhunzaker - it is not correct, but browsers will honor it. the correct thing is explicitly disallowed in react.

[StoneCypher]

@nhunzaker - as far as what you should do, there's sort of an open discussion there, because react's idea of <option> and <select> is fairly different than html's

you guys don't allow selected on <option>s. this makes multiselection impossible, which should not be, and causes some weird edge problems that aren't important.

you require instead that someone set defaultValue on the <select>, which i assume is a strategy to get around dealing with how controlled and uncontrolled components would turn into a mess if you had to involve their container this way

there is a good argument in my imagination that what React should do is to look at defaultValue, count out the appropriate number of nodes, and set the selected attribute on the relevant <option>

i don't know enough about your internals, but it's not clear to me why matching would be involved. this is a distance count

[aweary]

@StoneCypher I know this is frustrating for you, I understand your old issue was open for a long time and we're sorry about that. We want to work to resolve this, but this kind of behavior is not acceptable and if it continues we'll have to take action. Please keep it civil and focused on the technical discussion. Thanks.

[nhunzaker]

you guys don't allow selected on s. this makes multiselection impossible, which should not be, and causes some weird edge problems that aren't important.

For my own understanding, would you be willing to enumerate these weird edge cases? As far as I understood it, multiple selection is possible by providing an array, as demonstrated in this JSFiddle: https://jsfiddle.net/x29b9kd8/

there is a good argument in my imagination that what React should do is to look at defaultValue, count out the appropriate number of nodes

Could you explain this approach in more detail? Here is the function that runs to select options inside of React:

https://github.com/facebook/react/blob/master/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js#L75-L116

In short, it enumerates over the select's options and adds selected to options with matching values. It bails early for singular selects, however processes all options when multiple is set on the select.

Maybe we can get away with assigning value in the single-select case. I wonder what the browser support on that is. I think it's worth exploring.

[aweary]

@nhunzaker it seems like the reported issue is that you can't use the index of an option element as the value or defaultValue for a select element. I wouldn't expect this to work in the first place, as that's not the behavior that's described in the spec. It sounds like @StoneCypher is looking to set selectedIndex

We shouldn't support using indices for value or defaultValue

[StoneCypher]

@nhunzaker - ... oh jesus, i'm supposed to use the value ?

The documentaton just says "use defaultValue." It doesn't mention that you're supposed to use the value. I had assumed it would be the index because that's what the HTML does.

Okay. Maybe the edge cases don't exist, then. By memory when I tested this, one of the values was the index by coincidence, and the other was not; it may be that when I tried arrays, it gave an unexpected result because half of my dataset was coincidentally correct and the other half was not. (Also, this was four years ago, at a job at a company that doesn't exist anymore, so, I don't actually have the ability to dig up the old code, and I don't remember it very clearly. I'll make an effort to reproduce locally, but in that amount of time things may have been fixed, and with the misunderstanding that I had about the API, it may be that there's nothing to repro.)

That would be embarrassing for me, if I had left a bug open for four years because of misunderstanding the API.

The relevant docs are here. The relevant stretch is

Likewise, <input type="checkbox"> and <input type="radio"> support defaultChecked, and <select> and <textarea> supports defaultValue.

Because the documentation page addresses multiple input types, non-exhaustively in examples, there's no actual example of the use of a <select>, and thus no indicator

[nhunzaker]

@aweary Thank you, I neglected to identify that. I think on on the same page now.

I agree that we should not use indices to select options using the value or defaultValue prop. It should always be a direct comparison.

So this is a question of whether or not we want to support selectedIndex as a way to select options.

@StoneCypher What use case requires you to select an option from an index? Could you maintain the index in state and pass the value in by extracting an element from an array at that index? Something like:

https://jsfiddle.net/x29b9kd8/2/

[StoneCypher]

I am allergic to state. Also, that would not be appropriate for serverside rendering.

I don't really care if it's selectedIndex, defaultValue, or whatever. I think I just misunderstood what I was supposed to do.

I would appreciate being told how to do this in a way that could be rendered to static html without props or state (that is, something like

const example = <select><option>foo</option><option>bar</option></select>;

). I would also appreciate being told how I can contribute to the documentation.

I am worried that this is just a misunderstanding on my part how the API is expected to work.

[StoneCypher]

To be clearer, if someone would modify that one-liner such that a disabled option could be the initially selected one, then I can shut up and go away, and there's no bug

[StoneCypher]

(As far as use cases, it was to satisfy the design requirements for a UI at work. I haven't cared about this for years; I just came back because the original ticket got closed with a request to re-open if it was still broken, and at that time I thought it was still broken)

[StoneCypher]

@nhunzaker - also, just as a matter of being able to do what html does, i feel that supporting selectedIndex would be a pleasant choice

[lindslev]

+1 to pushing out a long overdue fix for this issue

[jquense]

Gonna close this out since there is no bug. We can talk about supporting selectedIndex in a specific feature request issue but at the moment I don't see a super compelling reason to add it. It's easy enough to work around with defaultValue and it's not clear how an additional controllable prop would interact with the existing ones. In terms of parity with html, I think that's a fair point however inputs in React are already pretty specialized and divergent from the normal behavior it may not be worth the added complecity

[StoneCypher]

@jquense - i will renew my request to be told how to contribute to the docs, which give in my opinion a strong signal to do this wrongly

[vndevil]

https://jsfiddle.net/hqpwxfu2/