Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed using array index as key in lists #2685

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

Made1ra
Copy link
Contributor

@Made1ra Made1ra commented Nov 12, 2024

Description:
Using array indices as keys in list components can cause unexpected behavior, especially when items are reordered or changed. Instead, use a unique ID for each item to help React handle updates more efficiently.

File Affected:
AddProfessionalCategoryModal.tsx

@Made1ra Made1ra added good first issue Good for newcomers sonarFix fix the sonar issues Frontend part labels Nov 12, 2024
@Made1ra Made1ra linked an issue Nov 12, 2024 that may be closed by this pull request
@Made1ra Made1ra self-assigned this Nov 12, 2024
@@ -214,7 +214,7 @@ const AddProfessionalCategoryModal: FC<AddProfessionalCategoryModalProps> = ({
disableOptions={data.subjects as Array<Partial<SubjectInterface>>}
handleChange={handleProfessionalSubjectChange(index)}
handleSubjectDelete={() => handleSubjectDelete(subject._id || '')}
key={index}
key={subject._id || crypto.randomUUID()}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any chance for _id here to be falsable?

Copy link
Contributor Author

@Made1ra Made1ra Nov 12, 2024

Choose a reason for hiding this comment

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

Yes, sometimes it's just an empty string.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove crypto.randomUUID(), since id='' will only be used once, when no category has been selected yet.

@@ -214,7 +214,7 @@ const AddProfessionalCategoryModal: FC<AddProfessionalCategoryModalProps> = ({
disableOptions={data.subjects as Array<Partial<SubjectInterface>>}
handleChange={handleProfessionalSubjectChange(index)}
handleSubjectDelete={() => handleSubjectDelete(subject._id || '')}
key={index}
key={subject._id || crypto.randomUUID()}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove crypto.randomUUID(), since id='' will only be used once, when no category has been selected yet.

@Made1ra Made1ra merged commit 5dfe7c6 into develop Nov 14, 2024
9 checks passed
@Made1ra Made1ra deleted the bugFix/2677/avoid-using-index-as-key branch November 14, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend part good first issue Good for newcomers sonarFix fix the sonar issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(SP: 0.5) Avoid Using Array Index as Key in JSX Lists
5 participants