Duplicate key in Tabs list when label is a node instead of a string

[slafayIGN]

In the Tabs component, the label can be a node instead of a simple string.

But the way the key is computed : https://github.com/codegouvfr/react-dsfr/blob/main/src/Tabs.tsx#L160

  <li key={label + (iconId ?? "")} role="presentation">

may give the <li> a non unique [object Object] key.

I encountered this when putting labels inside <span> .

Wouldn't it be a better option to choose the tabIndex as the key ?

[david-nathanael]

in general, it is better to avoid using indexes as key in react https://react.dev/learn/rendering-lists#why-does-react-need-keys

You might be tempted to use an item’s index in the array as its key. In fact, that’s what React will use if you don’t specify a key at all. But the order in which you render items will change over time if an item is inserted, deleted, or if the array gets reordered. Index as a key often leads to subtle and confusing bugs.

maybe give us the option to provide our own key ?

[garronej]

Hello @slafayIGN,

Thank you for bringing this to my attention. I can't believe I made such a basic error. It appears I changed the label type from string to ReactNode but failed to update the code to reflect that change.

@david-nathanael, although it's generally advisable to avoid using indexes as keys, it's not an absolute rule. The key attribute in React helps to prevent unnecessary rerendering of list items when their order changes. This is crucial for performance in some scenario like for example when you have dynamic filtering on a list of items that gets filtered out as the user types. However, in this case, the performance gain would be negligible.

Firstly, in 99.9% of scenarios, the tab order won't change dynamically. Even if it does, it would only trigger rerender of some native <button />, which would have an insignificant impact on performance.

I've released a new version that includes this patch.