Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Conversation

lukebarnard1
Copy link
Contributor

  • If the list contains two users twice, react would warn about duplicate keys. Use index instead.
  • Check if unmounted before setting state after fetching members.
 - If the list contains two users twice, react would warn about duplicate keys. Use `index` instead. - Check if unmounted before setting state after fetching members.
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, using list index as a key is a surefire way to confuse React and React doesn't like you doing it (because it gives it no more information than it would have otherwise). Why would we ever have duplicate users in the list?

@lukebarnard1
Copy link
Contributor Author

Hmm, using list index as a key is a surefire way to confuse React and React doesn't like you doing it (because it gives it no more information than it would have otherwise)

Ah. I had no idea about this but that makes a lot of sense.

Why would we ever have duplicate users in the list?

The API allows it (currently). E.g. Amandine is +matrix twice.

@dbkr
Copy link
Member

dbkr commented Oct 24, 2017

It's presumably not meaningful for the same user to be in the list twice though? I would confer with Erik, but I think I would vote for ignoring the dupes until synapse gets fixed to not send dupes?

@lukebarnard1
Copy link
Contributor Author

ignoring the dupes until synapse gets fixed to not send dupes?

This is fair. And then userId would be an OK key.

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES6 has Set() ftr :)

@dbkr dbkr merged commit ba46faf into develop Oct 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

2 participants