fix #3933: Duplicated keys in PropTypes table #4095
Conversation
gombek
requested review from
dangreenisrael,
danielduan,
Hypnosphi,
theinterned,
usulpro and
z4o4z
as code owners
gombek
changed the title
gombek
changed the title
| @@ -19,7 +22,9 @@ export const multiLineText = input => { | |||
| lineOfText, | |||
| i // note: lineOfText is the closest we will get to a unique key | |||
There was a problem hiding this comment.
Comment is misleading. Please remove
| @@ -6,6 +6,9 @@ import { Table, Td, Th } from '@storybook/components'; | |||
| import PropVal from './PropVal'; | |||
| import PrettyPropType from './types/PrettyPropType'; | |||
|
|
|||
| const willItOccurAgain = (collection, item, itemIndex = 0) => | |||
There was a problem hiding this comment.
Any particular reason that there is a default? Seems like its always specified
There was a problem hiding this comment.
Variable name could also be more descriptive and declarative.
| @@ -6,6 +6,9 @@ import { Table, Td, Th } from '@storybook/components'; | |||
| import PropVal from './PropVal'; | |||
| import PrettyPropType from './types/PrettyPropType'; | |||
|
|
|||
| const willItOccurAgain = (collection, item, itemIndex = 0) => | |||
| collection.indexOf(item, itemIndex + 1) !== -1; | |||
There was a problem hiding this comment.
Also a bit hesitant to use this to generate a key, seems a bit unperformant and a bit of an overkill
There was a problem hiding this comment.
so maybe to simplify things, let's change key to ${lineOfText}.${i} without any check for duplicates? It won't be perfect but since this component is not heavy and not very dynamic this should be enough in this case. What do you think?
There was a problem hiding this comment.
I think that would be good! It'll simplify the logic which is good
gombek
changed the title
Codecov Report
@@ Coverage Diff @@
## master #4095 +/- ##
==========================================
+ Coverage 40.57% 40.59% +0.02%
==========================================
Files 469 469
Lines 5641 5643 +2
Branches 748 749 +1
==========================================
+ Hits 2289 2291 +2
Misses 2984 2984
Partials 368 368
Continue to review full report at Codecov.
|
Issue: #3933
What I did
I've added check for duplicated lines, if line of text occur multiple times, index value will be added (as postfix) to the key.
How to test
Appropriate test is included in this PR.