fix option update. Fixes #2620

[yiminghe]

Option update will fail if option's content is mixed with variable and literal text. for example:

https://jsfiddle.net/yiminghe/jw73ccu5/1/

[jimfb]

please use if(__DEV__) instead of "production" !== process.env.NODE_ENV

[jimfb]

@yiminghe Overall looks good. I'm excited to see this fix! A couple of comments. Also, the linter is unhappy (you can execute npm run lint and fix anything of error severity).

[jimfb]

Tracking issue in #2620

[zpao]

nit: abc sort the requires (there's no lint rule for this)

[sophiebits]

This logic would actually be better in ReactDOMOption especially since we already have that wrapper, not in ReactDOMComponent.

[sophiebits]

These should be in a beforeEach so that the container is reinitialized before each spec.

[yiminghe]

This is not specific for option (maybe in the future), that's why i add onlyNestTextTags map.

[syranide]

I would be far less concerned if we simply threw on multiple children, or no longer made <option /> accept children like we did with <textarea /> (but that's probably too late now?). As for this specific implementation, if we ever support components returning a string then <option>Hi <Name /></option> wouldn't work as expected, my point being that this fix is basically a hack. It's current idea of what is printable is not identical to Reacts either. EDIT: Also fragments/arrays isn't accepted right now I think?

[sophiebits]

@yiminghe I understand. I'd prefer to have the logic in ReactDOMOption because I don't anticipate us adding it for other tags. If we choose to do that in the future, we can look at abstracting it out – but ReactDOMComponent is already very complicated and so we'd like to avoid complicating it unnecessarily. Can you move the code to ReactDOMOption's render function?

[sophiebits]

This if statement would be clearer with the branches swapped.

[sophiebits]

Actually, #3847 looks closer so I'll follow up with that one instead. Thanks for sending this in.