Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax ToString consistency guarantees #13508

Open
gaearon opened this issue Aug 29, 2018 · 1 comment
Open

Relax ToString consistency guarantees #13508

gaearon opened this issue Aug 29, 2018 · 1 comment
Labels
Component: DOM React Core Team Opened by a member of the React Core Team Type: Enhancement

Comments

@gaearon
Copy link
Collaborator

gaearon commented Aug 29, 2018

We recently chatted about #13367 and related work (e.g. #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:

  • warning for invalid values
  • ensuring that the output for invalid values is consistent (e.g. functions are always skipped)

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: DOM React Core Team Opened by a member of the React Core Team Type: Enhancement
Projects
None yet
Development

No branches or pull requests

3 participants