plotly / dash-table

OBSOLETE: now part of https://github.com/plotly/dash
https://dash.plotly.com
MIT License
420 stars 72 forks source link

Fix 'child in a list should have a unique key prop' #690

Open chabb opened 4 years ago

chabb commented 4 years ago
alexcjohnson commented 4 years ago

Thanks for the contribution @chabb - this is certainly annoying. But I'd like to figure out exactly where the problem is and just fix that rather than sprinkling more complex keys all over. I've only ever seen this message (from the table) coming from <th> elements, and I think the way cells are keyed just by column makes sense, because they're inside a <tr> for each row, so only column is needed to disambiguate.

Also what can we do to test this fix? Use debug mode for some of the tests and verify that we don't see any console warnings, is that enough? Perhaps this should happen in some of the tests @Marc-Andre-Rivet is porting to Selenium in https://github.com/plotly/dash-table/pull/696.

chabb commented 4 years ago

Thanks @alexcjohnson for taking care of it. I've tried to fix this issue, but I realized that it was not as straightforward as I thought it would be, especially if we want to have a good/consistent fix.

I think using the dev version of react for your tests, and failing tests on console errors would be the right good thing to do ( though it can potentially makes your tests slower and generates a lot of noise); but in the end, you do not want developers to see errors in their chrome/safari/firefox/(IE) dev console

simonhammes commented 4 years ago

Thanks for the contribution @chabb - this is certainly annoying. But I'd like to figure out exactly where the problem is and just fix that rather than sprinkling more complex keys all over. I've only ever seen this message (from the table) coming from <th> elements.

I am currently trying to fix this exact problem; what is the suggested solution? The weird thing is that I don't even see the th components in the React Dev Tools, only the actual table rows.

Thanks in advance!

blanning19 commented 3 years ago

Please let me know when this has been fixed so I can pull. Thanks