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

Unexpected string key handling #2612

Closed
rymohr opened this issue Nov 27, 2014 · 3 comments · Fixed by #5536
Closed

Unexpected string key handling #2612

rymohr opened this issue Nov 27, 2014 · 3 comments · Fixed by #5536

Comments

@rymohr
Copy link

rymohr commented Nov 27, 2014

I came across this one because I'd extended String.prototype.key with a custom key generator. React ends up treating the function itself as the child key, so it hides any strings that come after it.

http://jsfiddle.net/u2b6dac8/2/

Object key handling is documented and makes sense. Not sure it makes sense to handle string keys this way.

http://facebook.github.io/react/docs/multiple-components.html

@sophiebits
Copy link
Collaborator

@sebmarkbage what do you want to do here?

nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar,

@rymohr
Copy link
Author

rymohr commented Nov 27, 2014

I think the issue is actually here yeah?

if (component && component.key != null) {

if (component && component.key != null) {

Should probably be changed to

if (component && component.key != null && typeof component !== "string") {

@sebmarkbage
Copy link
Collaborator

Nice catch. I think that we might actually have more of these flaws. We should be more careful as this prevents browsers to add .key to the string prototype.

@spicyj that if branch can be separated and we just use the implicit key for string/number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants