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

Add context/detail to missing-key warning #6317

Closed
wants to merge 1 commit into from
Closed

Add context/detail to missing-key warning #6317

wants to merge 1 commit into from

Conversation

bpedersen
Copy link

Fix for #6163

Added offendingElement key to addenda object and func ElementToString() to provide more information on the element with the missing key

This is my first pull request, so feedback is appreciated

I have completed the CLA agreement as well :)

Fix for #6163

Added offendingElement key to addenda object and `func ElementToString()` to provide more information on the element with the missing key

*This is my first pull request, so feedback is appreciated*
}).join('');

}
console.log(element);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@jimfb
Copy link
Contributor

jimfb commented Mar 22, 2016

Travis is failing because of lint warnings, you can run npm run lint to find the problems.

More generally, however, I'm worried about several correctness issues here. For example, validateExplicitKey appears to be ignoring addenda.offendingElement. We are going to need a bunch of unit tests to verify the expected/desired behavior here.

@jimfb jimfb changed the title Update ReactElementValidator.js Add context/detail to missing-key warning Mar 22, 2016
@bpedersen
Copy link
Author

Thank you for the feedback!! I did see that I did leave a console.log in and that is the reason for the lint failing.
I will work on creating some tests this evening. As I see that this wouldn't be descriptive enough for a lot of cases.

On Mar 22, 2016, at 5:06 AM, Jim notifications@github.com wrote:

Travis is failing because of lint warnings, you can run npm run lint to find the problems.

More generally, however, I'm worried about several correctness issues here. For example, validateExplicitKey appears to be ignoring addenda.offendingElement. We are going to need a bunch of unit tests to verify the expected/desired behavior here.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@jimfb
Copy link
Contributor

jimfb commented Apr 13, 2016

Ping @PiereDome

@gaearon
Copy link
Collaborator

gaearon commented Jun 26, 2016

I’m closing as this has been inactive for a while.
We also display a stack trace in key warnings now so this is unnecessary.
Thanks for the effort!

@gaearon gaearon closed this Jun 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants