2

I'm having some issues when using shortid or any other unique uid generator. The moment I use shortid.generate() as key in a table, the anchor point of my Material UI Popover is thrown to its default position rather than appearing where the button is.

Here's a sandbox! - try removing/adding back shortid.generate() from line 72.

I even tried uniqueId from lodash and the same thing happens - not using a key does render the dialog on the right place though. I even changed versions of Material UI/React and nothing happened.

Any ideas?

Thanks!

EDIT - I usually use item.uid as key since I always fetch items from a service, but if I just created the object, item.uid is undefined - what I did until now was to set item.uid = shortid.generate() (a temporary uid) when I create the object and then just leave <TableRow key={item.uid}> as is. But then I have to remove the temporary uid before I save the object.

Ori Drori
  • 183,571
  • 29
  • 224
  • 209
Claim
  • 775
  • 3
  • 12
  • 32

1 Answers1

3

You should never use random ids as keys (at least not random ids generated during render).

When your state changes (e.g. due to handleClick) your table rows will all be re-rendered with new keys. This means that instead of a simple re-render, all of the table rows will be unmounted and new DOM elements mounted. The anchorEl in your state will point at an element that has been removed from the DOM, so its position cannot be determined.

If you don’t have unique keys related to your data, then just use an index as a key so that it is at least stable across renders (so long as you aren’t adding/removing rows). Another option would be to generate the unique ids when creating your data instead of during rendering.

Ryan Cogswell
  • 75,046
  • 9
  • 218
  • 198
  • "Another option would be to generate the unique ids when creating your data instead of during rendering." - so is it good practice if - when adding a new object to a list I render out, which is **NOT** read only - I just add a temporary uid for react's sake and then remove it before persisting? "The anchorEl in your state will point at an element that has been removed from the DOM, so its position cannot be determined." - okay this just clicked with me – Claim May 21 '19 at 12:10
  • As long as you have a reliable way to manage it outside of rendering, I think that is fine. It isn’t a common practice because usually there is something in the data that uniquely identifies each row. If you can remove rows in a way that is persisted, how do you communicate to the backend which row should be removed? Generally, the answer to that question is what should be used as the key. – Ryan Cogswell May 21 '19 at 12:18
  • When persisting a new object I usually add (server side) a few default fields like a `date` for example - so in React I usually loop through a list and check if the object has a `date` field. If it does, then I know that the `date` field has been set by the service and it is not a brand new object, so I skip it. This happens when I need a page where I can just press cancel and nothing is persisted. So I don't persist each object on its own, but a whole list at once. If I persisted each object on its own, I wouldn't have this issue though. – Claim May 21 '19 at 12:22