facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.53k stars 46.41k forks source link

Relax ToString consistency guarantees #13508

Open gaearon opened 6 years ago

gaearon commented 6 years ago

We recently chatted about https://github.com/facebook/react/pull/13367 and related work (e.g. https://github.com/facebook/react/pull/13394) with @sebmarkbage, and he raised a good point.

It seems like overall treating them consistently is adding significant overhead in the implementation readability. And there’s undoubtedly runtime overhead to it too. There are two separate issues here:

The conclusion we came to is that we should keep warning for bad values, but as long as we warn, consistency is not necessary. It's fine if we sometimes stringify a function, and sometimes skip it, as long as we always warn for those cases

Our guiding principle for invalid inputs should be that we handle them with the least amount of overhead (both at runtime, and in terms of code size), not that they’re always handled the same way.

One exception to this is probably Symbols because they throw when stringified. So it seems like skipping them is actually desirable — unless we're okay with errors.

nhunzaker commented 6 years ago

I'm a bit perplexed on the Symbol case:

image

Maybe we could just call value.toString() instead of '' + value. At the very least, that might get rid of the concern about stringifying symbols.

In any case, I think avoiding the need to ensure consistency would let us drop calls to getToStringValue, which would be a nice win.