Skip to content

mantine-ui: stable AND definitely unique key for /alerts cards#16044

Merged
juliusv merged 4 commits intoprometheus:mainfrom
amanycodes:fix/stable-cardUI-key
Apr 24, 2025
Merged

mantine-ui: stable AND definitely unique key for /alerts cards#16044
juliusv merged 4 commits intoprometheus:mainfrom
amanycodes:fix/stable-cardUI-key

Conversation

@amanycodes
Copy link
Contributor

@amanycodes amanycodes commented Feb 15, 2025

Replaced the index i with g.name-g.file which I think is stable (and unique) enough, I haven't caught any edge cases yet, but was also wondering maybe generate some unique ID (hash, uuid) and assign them. but re-renders when a card is removed might cause some issues. 🤔

@amanycodes amanycodes requested a review from juliusv as a code owner February 15, 2025 14:01
@rexagod
Copy link
Contributor

rexagod commented Feb 16, 2025

I'm not sure why is the current i doesn't work here as is? Is there an issue this can be linked to for more context, or could you explain a bit on any unexpected behaviors that stemmed from this?

@amanycodes
Copy link
Contributor Author

amanycodes commented Feb 17, 2025

Right now the key i is coming from the array index in the map, which is unstable because if one card in the middle isn’t rendered or is removed, the indexes of all following cards shift; for example, a card that used to have index i+1 will now have index i, leading to duplicate keys. This can cause issues such as React mixing up component states (since it relies on keys), mounting issues, and UI bugs.
It's safe to have a stable AND definitely unique key.

juliusv
juliusv reviewed Feb 17, 2025
View reviewed changes
@amanycodes amanycodes requested a review from juliusv February 21, 2025 04:43
@amanycodes
Copy link
Contributor Author

amanycodes commented Mar 3, 2025

I wrote stable AND definitely unique keys for the /rules page as well.

@krajorama
Copy link
Member

krajorama commented Mar 11, 2025

Hello from the bug scrub, PR seems active, leaving open.

@amanycodes
stable and definitely unique card UI key in /alerts
3dc9cbc 
Signed-off-by: amanycodes <amanycodes@gmail.com>
@amanycodes
fix-ci-errors
de5b5c5 
Signed-off-by: amanycodes <amanycodes@gmail.com>
@amanycodes
stable and unique key for /rules page and key fixes
25d2746 
Signed-off-by: amanycodes <amanycodes@gmail.com>
@amanycodes
removed comments
ad84642 
Signed-off-by: amanycodes <amanycodes@gmail.com>
@amanycodes amanycodes force-pushed the fix/stable-cardUI-key branch from 11baddb to ad84642 Compare April 24, 2025 14:39
@amanycodes
Copy link
Contributor Author

amanycodes commented Apr 24, 2025

@juliusv I have rebased this one aswell. Please review the changes!

juliusv
juliusv approved these changes Apr 24, 2025
View reviewed changes
Copy link
Member

@juliusv juliusv left a comment

Choose a reason for hiding this comment

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

👍 Thanks!

@juliusv juliusv enabled auto-merge April 24, 2025 15:34
@juliusv juliusv merged commit 9ab60c5 into prometheus:main Apr 24, 2025
27 checks passed
@charleskorn charleskorn mentioned this pull request May 1, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juliusv juliusv juliusv approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@amanycodes @rexagod @krajorama @juliusv