ReactTextComponent's string props clashes with cloneWithProps

[glenjamin]

The effect of this can be seen in this gist which uses react-bootstrap: http://jsfiddle.net/Hhc8z/1/

Relevant excerpts from the source:
https://github.com/react-bootstrap/react-bootstrap/blob/master/src/Navbar.jsx#L80
https://github.com/react-bootstrap/react-bootstrap/blob/master/src/utils/ValidComponentChildren.js#L19
https://github.com/react-bootstrap/react-bootstrap/blob/master/src/utils/cloneWithProps.js#L149

I'm trying to fix this in the boostrap lib, but it does seem like a bit of a gotcha anyway.

Is there a documented way to decide if cloneWithProps is safe to do, or is this supposed to just work in the current descriptor scheme of things?

[zpao]

Simple example outside of bootstrap: http://jsfiddle.net/8mXLr/

ReactTextComponent is supposed to be an implementation detail and we shortcut to just use a string as props there for perf (since it's internal). But perhaps we need to make it an object now so this work. @sebmarkbage? Alternatively we can make cloneWithProps skip ReactTextComponents?

[syranide]

Interesting, isn't ReactTextComponent supposed to be strictly internal only? I.e, the children should be strings/numbers/whatever you passed in rather than ReactTextComponent I would've thought?

[sebmarkbage]

Yes. @syranide is right. I think it was converted for legacy reasons because of the shared code path with traverseAllChildren.

This will need to be fixed as part of the effort to hide internal implementation detail.

On Jul 31, 2014, at 1:21 AM, Andreas Svensson notifications@github.com wrote:

Interesting, isn't ReactTextComponent supposed to be strictly internal only? I.e, the children should be strings/numbers/whatever you passed in rather than ReactTextComponent I would've thought?

—
Reply to this email directly or view it on GitHub.

[syranide]

#1988 is merged, closing.