Children of option tags flattened when select has value

[jorrit]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When the following is rendered to string using ReactDOM:

<select value=""><option>a ({`b`})</option></select>

I get

<select data-reactroot=""><option>a (b)</option></select>

When hydrating the above code, I get this warning:

Warning: Text content did not match. Server: "a (b)" Client: "a ("

Fiddle: https://jsfiddle.net/z1q0azjL/1/

What is the expected behavior?

I should get:

<select data-reactroot=""><option>a (<!-- -->b<!-- -->)</option></select>

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

16.1.1

This has something to do with flattenOptionChildren in ReactPartialRenderer.js.

PS:
When browsing ReactPartialRenderer.js, I found this code:

        props = Object.assign(
          {
            selected: undefined,
            children: undefined,
          },
          props,
          {
            selected: selected,
            children: optionChildren,
          },
        );

What is the point of those undefined props when they are overwritten later on?

[gaearon]

Would you mind contributing a (failing) test to ReactDOMServerIntegration-test that verifies this case?

[jorrit]

Sure, hopefully tomorrow (CET)!

[gaearon]

Do you want to work on a fix?

What is the point of those undefined props when they are overwritten later on?

That ensures objects have the same shape which helps performance.

[jorrit]

Of course I tried, but I failed on the following point: currently the code concats the strings and leaves out any other children of <option> tags, in effect babysitting the developer a bit. When the child of an option tag is a single string a special code path is followed that isn't followed when the child is an array of strings. I could not get some client tests in react-dom working because of this.

I can push the changes I've made so far to #11911 and maybe you or another experienced dev can guide me to additional changes necessary?

[jorrit]

BTW: thanks for the performance explanation.

[gaearon]

If you have some work in progress, send a PR and we can try to take it from there.

[jorrit]

Done, thanks!

[t4deu]

Hi, the issue seems to be unassigned, can I take this one, I would love to help!!!

[jorrit]

There is some progress by me in #11911, as far as I am concerned you're welcome to fork it and continue in a PR of your own. I hope you get it working properly!

[t4deu]

Great! I will do it, thanks

[t4deu]

Hello, I just sent a PR with the fix.

Apparently, it is caused by a regression introduced during the integration of Fiber.

The renderDOM method from ReactPartialRenderer was overwriting the children nodes from options, when setting the "selected" option, resulting in the bug.

As can be seen here, the previous version of the same feature only set the selected prop.

Since the bug was not related with flattenChildren, flattenOptionChildren or getHostProps that where removed/altered on the previous PR, I was unable to fork from it.

[jorrit]

Thanks! Are my faling testcases from #11623 (I accidently referenced #11911 twice above when I actually meant #11623) working with your PR?

[t4deu]

Yes, but I needed to update the tests, the clientRenderOnServerString conditional is the same as serverRender and streamRender, so there was no need of an extra block.
The ReactDOMServerIntegrationForms-test seemed to be a better place, since it's where the tests for inputs, textareas and selects are, also updated the clientCleanRender test because only returns one text node.

Besides that also tested using the fiddle you provided and an SSR sample project that I created to reproduce the warning error and confirm that the issue is resolved.

[jorrit]

Thanks a lot!

[gaearon]

This appears fixed by #13261.