search

facebook / prop-types

Runtime type checking for React props and similar objects
MIT License
4.48k stars 356 forks source link

Add key to error messages #284

Closed asbjornh closed 4 years ago

asbjornh commented 5 years ago

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 key in 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 the key prop doesn't hold any special significance outside the case of array.map and might be used as a part of the regular data flow. In those cases it might be weird that the key prop is displayed in warnings. Detecting whether a component was rendered with array.map is likely impossible in prop-types so 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?

asbjornh commented 4 years ago

Closing this since there hasn't been any activity for a while!

ljharb commented 4 years ago

@asbjornh that's not a good reason to close things, but if you're no longer interested in the PR, so be it.

asbjornh commented 4 years ago

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 :)