className may be a function

[onufrienko]

hola :)
we use bem-cn that returns function which react casts into string while rendering, but your framework doesn't consider it. So let's cast to string!

[blainekasten]

Hey! Thanks for the PR! couple of things here:

1. I'm not familar with bem-cn, can you explain a bit about how it works? From my understanding, React expects className to be a string at render time. How does bem-cn properly pass a function to that?
2. Your identation is off.
3. We need unit tests or there is a good chance obscure code like this could be regressed.

You can hold off on executing 2 & 3 until we determine the necessity of this PR if you want. Thanks for starting the conversation around this!

[blainekasten]

For reference: https://github.com/albburtsev/bem-cn#proptypes-warnings

I'm going to wait for other maintainers to respond. I personally feel like option #2 can solve this problem in user-land and this PR is unnecassary. Is there any reason you couldn't just call toString() on your side? instead of expecting enzyme to do it?

[ljharb]

If React stringifies it without a warning, then I'm ok with us doing it too (using String(), never .toString). Otherwise, yes, this is the explicit responsibility of the user (i.e. option 2)

[nfcampos]

I agree with @ljharb that we should support whatever react supports, so if react supports this without any warnings we should too. we do however have to confirm that this is indeed supported by react, in all 3 versions that we support

[f0rmat1k]

I'm not familar with bem-cn, can you explain a bit about how it works? From my understanding, React expects className to be a string at render time. How does bem-cn properly pass a function to that?

bem-cn is a helper for building BEM names using carrying. it returns a function, that have own toString method, that return a string containing sequence of BEM css classes.
As i think, it works because react concatenates className or converts to a String. Now i'm finding confirmation of my words in sources.

[f0rmat1k]

Here it is
https://github.com/facebook/react/blob/master/src/renderers/dom/shared/DOMPropertyOperations.js#L189

[ljharb]

Given that, we should definitely be coercing it to a string. However, it must use String(x), as '' + x and x.toString() wrongly invoke valueOf, and do not guarantee a string type, respectively.

[f0rmat1k]

Ok. Shall we edit our PR or you will do you own with tests?

[ljharb]

@f0rmat1k if you're willing to write the tests, it'd be great if you could edit the PR :-)

[blainekasten]

Looks like we can close this in favor of #519