Add a regression test for #12200

[GarethSmall]

Problem

When creating a select element and setting multiple the first element would be selected. This was due to the multiple prop being set after the children we're appended.

An non-react example case created by @aweary can been seen here - https://jsfiddle.net/dm6vkq9q/

Tradeoffs / Misc

In my original solution I added a new hook to set props for just the select element before the children we're appended. This worked only because it was setting just the select props first. An alternative idea proposed was to switch the order of appendChildren > setProps to setProps > appendChildren. this will not work in the instance that we have a defualtValue set for our select input. When we change this order a defaultValue is ignored. There we're also many other tests that broke in this process. Although this is bit of a hacky solution this will not require us to make any dangerous changes to the code base.

Solution

In postMountWrapper in ReactDOMFiberSelect.js I set selectedIndex for the select element to -1

Issue

#12200

Related closed pull requests

#12240

@aweary @gaearon

[GarethSmall]

@aweary thoughts?

[ewolfe]

Short and sweet 👍

[ewolfe]

@aweary Would you mind taking a look at this? I just ran into this bug and this seems like a good fix. I tested it locally (after manually copy/pasting to an up-to-date fork) and everything worked as expected :)

[nhunzaker]

Possibly chasing ghosts, but I'm worried about the side-effects of always assigning selectedIndex to -1. Do you think it makes sense to only apply this for the multiple input case?

const multiple =  !!props.multiple;

if (multiple) {
  node.selectedIndex = -1
  node.multiple = true
}

[GarethSmall]

Good suggestion, since this only occurs in the event of multiple this makes sense.

[nhunzaker]

Thanks for sending this in!

Additionally, would it be possible to add a test case to ensure that this behavior is consistent when multiple is set from false to true and back? I want to make sure we cover the update case too.

[GarethSmall]

Sounds good, ill add them this evening.

[gaearon]

We merged a different fix in #13270. I'm taking this for the regression unit test though. Thank you so much!

fix reverted