Add key to error messages #284
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Closing this since there hasn't been any activity for a while! |
|
@asbjornh that's not a good reason to close things, but if you're no longer interested in the PR, so be it. |
|
You're right I should've been more clear. I have sort of lost interest because like I wrote in the description I don't think this is such a good idea anymore :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #279
This PR aims to solve the problem of having many failed propTypes checks when rendering a huge list of components. In this case, it can be really hard to determine which warning belongs to which component instance, making debugging tedious and frustrating.
Displaying the
keyin warnings could help to quickly identify which item(s) in a list of components have type warnings:Warning: Failed prop type: The prop 'blabla' is marked as required in 'SomeComponent' with key 'abc', but its value is 'null'.While this would mitigate the issue, I'm not a 100% sure that this is a good idea (see below), but I'm opening this PR in hopes of at least getting some discussion around it.
This solution should work decently when doing
array.map, but it comes with at least one side effect that might not be desirable. As far as I know thekeyprop doesn't hold any special significance outside the case ofarray.mapand might be used as a part of the regular data flow. In those cases it might be weird that thekeyprop is displayed in warnings. Detecting whether a component was rendered witharray.mapis likely impossible inprop-typesso I don't see a way of avoiding this.Also, there might be a need for truncating very long keys to avoid destroying the readability of the message?