Is uniqueId in TableHeader for aria-describedby and id necessary?

[tanoargie]

Hi,
First of all, let me apologize if this is neither the place nor the issue type or label for this kind of thing. I tried to place this question in the Discord server but I think this is not available yet.

Whenever you test a snapshot of a mounted component and that component has TableHeader as a child, the test will failed as there is a Math.random() call in TableHeader.js for some props, specifically id and aria-describedby. Obviously you can mock Math.random() and the tests will passed (that's also why I wouldn't consider this a bug), but I wonder if we could add some props for these fields.
The code in question in TableHeader.js is the following:

Thanks in advance!

[joshblack]

Hi there @francoserio! 👋 Thanks so much for taking the time to make this issue.

I went ahead and opened up a PR for this at #7730. Unfortunately, we do need to generate unique ids for this part of the component but the usage of Math.random is definitely something we can avoid. I went ahead and updated the component to use a more stable identifier, while it still will auto-generate ids it should allow snapshots to stay consistent more easily than Math.random which would trigger them to be out-of-date each time the test is run.

Hope this helps!

[daka1510]

Thanks for bringing this up, @francoserio and the quick action, @joshblack. Because the generated ids are not deterministic and cannot be overriden by the consumer, it is also breaking our snapshot tests and thus the adoption of the latest carbon version.

My understanding of the fix in #7730 is that it will guarantee that the unique ids will stay consistent across different test runs. Will try it when the updated version is released.

[tanoargie]

Awesome @joshblack! (what a quick response). Glad I could be of help @daka1510!
I will update my upstream as soon this is merged!
Thanks to both of you!