Remove array indexes as keys in JSX #2497
Conversation
Summary by CodeRabbitRelease Notes
WalkthroughReplaced array-index React keys with stable, item-derived composite keys across many frontend components and tests; added NOSONAR comments where index usage is intentional; removed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-07-12T17:36:57.255ZApplied to files:
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/about/page.tsx (1)
205-207: Use stable identifiers for milestone keys and tooltip IDs instead of array indexThe code at lines 207 and 231 uses array index as a fallback for keys and tooltip IDs, which risks unstable reconciliation and DOM ID collisions when list order changes. The
milestone.idfield is available in the GraphQL query response and should be used as the primary key.Apply the suggested changes:
- .map((milestone, index) => ( + .map((milestone) => ( <div - key={milestone.url || milestone.title || index} + key={milestone.id ?? milestone.url ?? `${milestone.title}-${milestone.progress}`} className="flex items-center gap-4 overflow-hidden rounded-lg bg-gray-200 p-6 dark:bg-gray-700" > ... - id={`tooltip-state-${index}`} + id={`tooltip-state-${(milestone.id ?? milestone.url ?? milestone.title).toString().replace(/\W+/g, '-')}`}
🧹 Nitpick comments (4)
frontend/src/components/NavDropDown.tsx (1)
73-73: Consider using href alone if unique within submenu.The composite key
${submenu.href}-${idx}includes the index. Ifsubmenu.hrefvalues are guaranteed to be unique within each dropdown menu (which is typical for navigation), you could use justsubmenu.hrefas the key. However, the current implementation is safe and prevents potential issues with duplicate hrefs.If hrefs are unique, simplify to:
- key={`${submenu.href}-${idx}`} + key={submenu.href}frontend/src/app/members/[memberKey]/page.tsx (1)
85-99: Keys still depend on array index.Both
mention-${username}-${index}andword-${word}-${index}include the array index, which means the keys change when word positions shift. For bio text, this may be acceptable since order is meaningful and the content is relatively static. However, for complete independence from index position, consider using only the content and a unique counter based on the content itself.If duplicate words/mentions are rare in bio text, you could use:
-<React.Fragment key={`mention-${username}-${index}`}> +<React.Fragment key={`mention-${username}-${mentionMatch.index}`}>This uses the regex match index instead of the array index, providing better stability.
frontend/src/app/about/page.tsx (2)
257-258: Make “Our Story” keys more collision‑resistant (optional)Using a 40‑char slice can collide if paragraphs share a prefix. Use a full‑string slug.
- {projectStory.map((text) => ( - <div key={text.slice(0, 40).replace(/\W+/g, '-')} className="mb-4"> + {projectStory.map((text) => ( + <div key={text.toLowerCase().replace(/\W+/g, '-').replace(/-+/g, '-')} className="mb-4">
54-59: Leverage Apollo ‘loading’ flags for clearer state (recommended)Current
isLoadingchecks data presence and errors; usingloadingis simpler and avoids transient false negatives.- const { data: projectMetadataResponse, error: projectMetadataRequestError } = useQuery( + const { data: projectMetadataResponse, loading: projectMetadataLoading, error: projectMetadataRequestError } = useQuery( GetProjectMetadataDocument, { variables: { key: projectKey }, } ) - const { data: topContributorsResponse, error: topContributorsRequestError } = useQuery( + const { data: topContributorsResponse, loading: topContributorsLoading, error: topContributorsRequestError } = useQuery( GetTopContributorsDocument, { variables: { excludedUsernames: Object.keys(leaders), hasFullName: true, key: projectKey, limit: 24, }, } ) @@ - const isLoading = - !projectMetadataResponse || - !topContributorsResponse || - (projectMetadataRequestError && !projectMetadata) || - (topContributorsRequestError && !topContributors) || - leadersLoading + const isLoading = + projectMetadataLoading || + topContributorsLoading || + leadersLoadingThis keeps error handling in effects unchanged while simplifying loading logic.
Also applies to: 61-71, 98-104
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
frontend/__tests__/unit/pages/Header.test.tsx(1 hunks)frontend/src/app/about/page.tsx(3 hunks)frontend/src/app/members/[memberKey]/page.tsx(2 hunks)frontend/src/app/page.tsx(1 hunks)frontend/src/components/CardDetailsPage.tsx(2 hunks)frontend/src/components/Footer.tsx(1 hunks)frontend/src/components/Header.tsx(2 hunks)frontend/src/components/ItemCardList.tsx(1 hunks)frontend/src/components/MultiSearch.tsx(1 hunks)frontend/src/components/NavDropDown.tsx(1 hunks)frontend/src/components/Pagination.tsx(1 hunks)frontend/src/components/RecentReleases.tsx(1 hunks)frontend/src/components/RepositoriesCard.tsx(1 hunks)frontend/src/components/SkeletonsBase.tsx(1 hunks)frontend/src/components/ToggleableList.tsx(1 hunks)frontend/src/components/TopContributorsList.tsx(1 hunks)frontend/src/components/skeletons/ApiKeySkelton.tsx(1 hunks)frontend/src/components/skeletons/Card.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: codic-yeeshu
PR: OWASP/Nest#1444
File: frontend/src/components/NavDropDown.tsx:0-0
Timestamp: 2025-04-30T13:41:20.846Z
Learning: Use React's useId() hook rather than manually generating random strings when creating accessibility identifiers for UI components. This creates stable, unique IDs without causing hydration mismatches.
🧬 Code graph analysis (4)
frontend/src/app/about/page.tsx (2)
frontend/src/types/__generated__/projectQueries.generated.ts (2)
GetProjectMetadataDocument(37-37)GetTopContributorsDocument(38-38)frontend/src/utils/aboutData.ts (1)
projectStory(56-60)
frontend/src/components/RepositoriesCard.tsx (1)
backend/apps/github/api/internal/queries/repository.py (1)
repository(14-35)
frontend/src/components/Header.tsx (2)
frontend/src/components/NavDropDown.tsx (1)
NavDropdown(13-98)frontend/src/types/link.ts (1)
Link(1-7)
frontend/src/components/RecentReleases.tsx (1)
frontend/src/types/release.ts (1)
Release(3-13)
🔇 Additional comments (14)
frontend/src/components/SkeletonsBase.tsx (1)
11-11: Index-based key is acceptable here for skeleton loaders.The template string
user-skeleton-${index}still relies on the array index. For skeleton components with fixed-size generated arrays and no underlying data, using the index is acceptable since the array structure is stable during the loading state. The string prefix doesn't provide additional stability benefits over using the index directly.frontend/src/app/page.tsx (1)
360-361: Excellent use of content-based key.Using
stat.labelas the key is the correct approach. The labels are unique and stable, providing proper React reconciliation benefits.frontend/src/components/RepositoriesCard.tsx (1)
25-26: Perfect implementation of stable, unique keys.Using
repository.keyas the React key is exactly what this PR aims to achieve. The key property is a stable, unique identifier that ensures proper React reconciliation.frontend/src/components/Pagination.tsx (1)
72-72: Composite key is appropriate for pagination with ellipses.The composite key
${number}-${index}is necessary here because thepageNumbersarray can contain multiple'...'string values (ellipses). Using the index as part of the key ensures uniqueness while still incorporating the content value.frontend/src/components/ToggleableList.tsx (1)
39-41: No issues found - code change is correct.The key change from index to string value is appropriate for this use case. The
itemsarrays (languages, topics, tags, domains) represent unique categorical metadata by design. These semantic entities should not contain duplicates—they come from backend APIs representing distinct programming languages, project topics, tags, or domains.Using the string value as a React key is the standard and more correct approach than index-based keying. If duplicates were theoretically possible, they would cause issues regardless of key strategy.
frontend/src/components/MultiSearch.tsx (1)
267-267: The review comment is incorrect and should be disregarded.The data properties (
hit.key,hit.login,hit.url) are not guaranteed to be unique within suggestion groups. Specifically, the Project type has bothkeyandurlas optional fields, meaning multiple Project results could have identical values or both be undefined. ThesubIndexsuffix is necessary to ensure unique keys for React list rendering and is correctly used in the current implementation.Likely an incorrect or invalid review comment.
frontend/src/components/skeletons/ApiKeySkelton.tsx (1)
54-57: LGTM - Skeleton keys are appropriately stable.For skeleton/loading components that render static placeholders, index-based keys are acceptable since the items don't represent real data and won't be reordered. The descriptive prefix improves readability.
frontend/__tests__/unit/pages/Header.test.tsx (1)
84-92: LGTM - Test mock aligns with component implementation.The mock correctly uses composite keys matching the actual NavDropDown component implementation, ensuring tests remain accurate.
frontend/src/components/skeletons/Card.tsx (2)
35-35: LGTM - Skeleton keys are appropriately stable.Index-based keys for skeleton loading components are acceptable since they render static placeholders that won't be dynamically reordered.
60-64: LGTM - Consistent key naming for skeleton contributors.The descriptive prefix maintains consistency with other skeleton keys in this component.
frontend/src/components/CardDetailsPage.tsx (2)
316-327: LGTM - URL is an ideal unique key.Using the URL itself as the key is perfect since URLs are inherently unique. This change eliminates any index-based key issues.
159-169: Use array index as fallback key or add unique identifier to stats.The concern is valid. The
Statstype (frontend/src/types/card.ts) has noidfield, and bothunitandvalueare optional. Two different stats can legitimately have identicalunitandvaluecombinations (e.g., two metrics both withunit="issues"andvalue=0), creating duplicate keys.Replace the key with:
{stats.map((stat, index) => ( <div key={`${stat.unit}-${stat.value}-${index}`}>Or add a unique
idfield to theStatstype definition.frontend/src/components/Header.tsx (1)
196-209: Verify mobile submenu key uniqueness.The mobile submenu uses the same composite key pattern
${sub.text}-${sub.href}. Ensure all submenu items have definedhrefvalues to prevent key collisions.This verification is covered by the script in the previous comment (lines 94-96).
frontend/src/components/RecentReleases.tsx (1)
32-37: Verified: Stable composite key correctly replaces array index in RecentReleases.tsxThe fix using
${item.repositoryName}-${item.tagName}is stable and unique for releases, matching the PR objective. Verification identified several pre-existing index-based or index-combined keys across the codebase (e.g., ItemCardList.tsx, TopContributorsList.tsx, LeadersList.tsx), but these remain outside the scope of this PR's changes to RecentReleases.tsx.
|
@arkid15r @kasya in ApiKeysSkeleton and Card.tsx there is sonarqube warning about using array index as key but here its all static so i think its safe to use index key here but if we still want to remove warning we can generate uuid using useMemo for key let me know what you think better |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/app/about/page.tsx (2)
257-265: Story keys may collide; use the full text (or a hash) as the key.First-40-char slug is not guaranteed unique. Use the full string (stable) or add a hash.
Apply:
- {projectStory.map((text) => ( + {projectStory.map((text) => ( <div - key={text - .slice(0, 40) - .trim() - .replaceAll(' ', '-') - .replaceAll(/[^\w-]/g, '')} + key={text} className="mb-4" >
206-208: Usemilestone.idfor React key and tooltipidin the Roadmap section.Your review is accurate. The
GetProjectMetadataQueryincludesidin therecentMilestonesarray, so the refactoring is valid. Replace both:
- Line 207:
key={milestone.url || milestone.title || index}→key={milestone.id}- Line 232:
id={tooltip-state-${index}}→id={tooltip-state-${milestone.id}}Using stable unique identifiers for React keys and element IDs is a best practice.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/app/about/page.tsx(3 hunks)frontend/src/components/TopContributorsList.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/app/about/page.tsx (2)
frontend/src/types/__generated__/projectQueries.generated.ts (2)
GetProjectMetadataDocument(37-37)GetTopContributorsDocument(38-38)frontend/src/utils/aboutData.ts (1)
projectStory(56-60)
🔇 Additional comments (1)
frontend/src/components/TopContributorsList.tsx (1)
45-49: Remove this review comment—the suggestedidfield does not exist on theContributortype.The
Contributortype definition (frontend/src/types/contributor.ts) contains only:avatarUrl,contributionsCount,login,name,projectKey, andprojectName. There is noidfield. The proposed fixitem.id ?? item.loginwould cause a TypeScript error. The current key usingloginas the primary fallback is appropriate sinceloginis a unique identifier.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Footer.tsx(1 hunks)frontend/src/components/ItemCardList.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/Footer.tsx
@arkid15r Can you please look into this so that i can proceed |
Oh sorry, I missed that. I believe this case can be excluded with # NOSONAR. Please add a comment explaining the decision. Thank you! |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/skeletons/Card.tsx (1)
38-38: Add key prop or remove from array context.This
Skeletonis rendered alongside the mapped skeletons (lines 35-37) as part of an array of children. React requires akeyprop for all children in such contexts to avoid warnings and reconciliation issues.Apply this diff to add a unique key:
- <Skeleton /> + <Skeleton key="extra-icon" className="h-8 w-16" />
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/skeletons/ApiKeySkelton.tsx(1 hunks)frontend/src/components/skeletons/Card.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: codic-yeeshu
Repo: OWASP/Nest PR: 1444
File: frontend/src/components/NavDropDown.tsx:0-0
Timestamp: 2025-04-30T13:41:20.846Z
Learning: Use React's useId() hook rather than manually generating random strings when creating accessibility identifiers for UI components. This creates stable, unique IDs without causing hydration mismatches.
🪛 Biome (2.1.2)
frontend/src/components/skeletons/Card.tsx
[error] 34-34: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
[error] 60-60: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
frontend/src/components/skeletons/ApiKeySkelton.tsx
[error] 53-53: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
anurag2787
force-pushed
the
remove-indexes
branch
from
f4f2696 to
cdee17a
Compare
anurag2787
force-pushed
the
remove-indexes
branch
from
cdee17a to
1811800
Compare
|
hy, in my local running make check-test multiple time it something giving failed but sometime getting passed |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Header.tsx (1)
196-210: Prevent undefined values in mobile submenu keys.Similar to the desktop menu, the mobile submenu uses
${sub.text}-${sub.href}as the key, which will produce keys containing "undefined" whenhrefis absent (sincehrefis optional in theLinktype).Apply this diff:
<div className="ml-4"> - {link.submenu.map((sub) => ( + {link.submenu.map((sub) => ( <Link - key={`${sub.text}-${sub.href}`} + key={`${sub.text}-${sub.href || 'no-href'}`} href={sub.href || '/'}This ensures consistent key generation across desktop and mobile views.
♻️ Duplicate comments (4)
frontend/src/components/Header.tsx (1)
94-96: Prevent undefined values in composite keys.The composite key
${link.text}-${link.href}will produce keys like"text-undefined"whenhrefis absent, since theLinktype defineshrefas optional (href?: string). While the currentheaderLinksdata may havehrefvalues for items with submenus, the key strategy should be defensive against missing values.Note that
NavDropDown.tsxuses a different strategy with an index fallback:${submenu.href}-${idx}.Apply this diff to add a safe fallback:
.map((link) => { return link.submenu ? ( - <NavDropdown link={link} pathname={pathname} key={`${link.text}-${link.href}`} /> + <NavDropdown link={link} pathname={pathname} key={`${link.text}-${link.href || 'no-href'}`} /> ) : (This prevents undefined from appearing in the key string.
frontend/src/components/ItemCardList.tsx (1)
48-51: Risk of undefined keys for items missing all identifier properties.The composite key
item.objectID ||${item.repositoryName}-${item.title || item.name}-${item.url}`` could still evaluate toundefinedif an item lacks all these properties. Based on past review analysis:
- Milestone items may lack
objectID, and the composite could produce keys like"undefined-Title-undefined"ifrepositoryNameorurlare missing- String concatenation with undefined values creates fragile keys
Add a guaranteed final fallback to prevent undefined keys:
{data.map((item, index) => ( <div - key={item.objectID || `${item.repositoryName}-${item.title || item.name}-${item.url}`} + key={ + item.objectID || + `${item.repositoryName || 'unknown'}-${item.title || item.name || 'item'}-${item.url || index}` + } className="mb-4 w-full rounded-lg bg-gray-200 p-4 dark:bg-gray-700" >This ensures all components of the composite key have valid fallbacks, preventing undefined or malformed keys.
frontend/src/components/Footer.tsx (1)
51-52: Risk of duplicate keys across sections.The key
link.href ||span-${link.text}`` could produce duplicate keys when the same link text appears in multiple sections. According to the constants data, text values like "About" and "Contribute" appear in multiple sections.As discussed in the past review comments, the recommended solution is to scope the key by section title:
Apply this diff:
- {section.links.map((link) => ( - <div key={link.href || `span-${link.text}`} className="py-1"> + {section.links.map((link) => ( + <div key={`${section.title}-${link.text}`} className="py-1">This ensures uniqueness by combining the section title (which is unique per section) with the link text, eliminating the possibility of collisions while avoiding reliance on the potentially undefined
hrefproperty.frontend/src/app/about/page.tsx (1)
54-103: Avoid gating loading on data presence—errors now spin forever.On a failed fetch, Apollo sets
errorbut leavesdataundefined, so the newisLoadingcondition never clears and the user is stuck on the spinner (no error screen). BaseisLoadingon the hooks’loadingflags so the component exits the loading branch once the request settles.- const { data: projectMetadataResponse, error: projectMetadataRequestError } = useQuery( + const { + data: projectMetadataResponse, + error: projectMetadataRequestError, + loading: projectMetadataLoading, + } = useQuery( GetProjectMetadataDocument, { variables: { key: projectKey }, } ) - const { data: topContributorsResponse, error: topContributorsRequestError } = useQuery( + const { + data: topContributorsResponse, + error: topContributorsRequestError, + loading: topContributorsLoading, + } = useQuery( GetTopContributorsDocument, { variables: { excludedUsernames: Object.keys(leaders), hasFullName: true, key: projectKey, limit: 24, }, } ) - const isLoading = - !projectMetadataResponse || - !topContributorsResponse || - (projectMetadataRequestError && !projectMetadata) || - (topContributorsRequestError && !topContributors) || - leadersLoading + const isLoading = projectMetadataLoading || topContributorsLoading || leadersLoading
🧹 Nitpick comments (1)
frontend/src/components/ItemCardList.tsx (1)
58-58: Align tooltip ID with the composite key strategy.The tooltip ID still uses
avatar-tooltip-${index}, which is inconsistent with the item's composite key strategy (line 49). As noted in past reviews, this inconsistency can make debugging difficult and doesn't follow the same stability principles applied to the item key.<Tooltip closeDelay={100} content={item?.author?.name || item?.author?.login} - id={`avatar-tooltip-${index}`} + id={`avatar-tooltip-${ + item.objectID || + `${item.repositoryName || 'unknown'}-${item.title || item.name || 'item'}` + }`} delay={100}This aligns the tooltip ID generation with the same fallback logic used for the item key.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
-
frontend/__tests__/unit/components/RecentIssues.test.tsx(1 hunks) -
frontend/__tests__/unit/pages/Header.test.tsx(1 hunks) -
frontend/src/app/about/page.tsx(3 hunks) -
frontend/src/app/members/[memberKey]/page.tsx(2 hunks) -
frontend/src/app/page.tsx(1 hunks) -
frontend/src/components/CardDetailsPage.tsx(2 hunks) -
frontend/src/components/Footer.tsx(1 hunks) -
frontend/src/components/Header.tsx(2 hunks) -
frontend/src/components/ItemCardList.tsx(1 hunks) -
frontend/src/components/MultiSearch.tsx(1 hunks) -
frontend/src/components/NavDropDown.tsx(1 hunks) -
frontend/src/components/Pagination.tsx(1 hunks) -
frontend/src/components/RecentReleases.tsx(1 hunks) -
frontend/src/components/RepositoryCard.tsx(1 hunks) -
frontend/src/components/SkeletonsBase.tsx(1 hunks) -
frontend/src/components/ToggleableList.tsx(1 hunks) -
frontend/src/components/TopContributorsList.tsx(1 hunks) -
frontend/src/components/skeletons/ApiKeySkelton.tsx(1 hunks) -
frontend/src/components/skeletons/Card.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- frontend/tests/unit/pages/Header.test.tsx
- frontend/src/app/members/[memberKey]/page.tsx
- frontend/src/components/Pagination.tsx
- frontend/src/components/NavDropDown.tsx
- frontend/src/app/page.tsx
- frontend/src/components/skeletons/ApiKeySkelton.tsx
- frontend/src/components/CardDetailsPage.tsx
- frontend/src/components/RepositoryCard.tsx
- frontend/src/components/ToggleableList.tsx
- frontend/src/components/SkeletonsBase.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
📚 Learning: 2025-04-30T13:41:20.846Z
Learnt from: codic-yeeshu
Repo: OWASP/Nest PR: 1444
File: frontend/src/components/NavDropDown.tsx:0-0
Timestamp: 2025-04-30T13:41:20.846Z
Learning: Use React's useId() hook rather than manually generating random strings when creating accessibility identifiers for UI components. This creates stable, unique IDs without causing hydration mismatches.
Applied to files:
frontend/src/components/Footer.tsxfrontend/src/components/ItemCardList.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/components/skeletons/Card.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/components/skeletons/Card.tsx
📚 Learning: 2025-09-21T17:04:48.154Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Applied to files:
frontend/src/app/about/page.tsx
📚 Learning: 2025-09-21T11:34:33.377Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
Applied to files:
frontend/src/app/about/page.tsx
🧬 Code graph analysis (3)
frontend/src/components/Header.tsx (2)
frontend/src/components/NavDropDown.tsx (1)
NavDropdown(13-98)frontend/src/types/link.ts (1)
Link(1-7)
frontend/src/components/RecentReleases.tsx (1)
frontend/src/types/release.ts (1)
Release(3-13)
frontend/src/app/about/page.tsx (2)
frontend/src/types/__generated__/projectQueries.generated.ts (2)
GetProjectMetadataDocument(37-37)GetTopContributorsDocument(38-38)frontend/src/utils/aboutData.ts (1)
projectStory(56-60)
🔇 Additional comments (4)
frontend/__tests__/unit/components/RecentIssues.test.tsx (1)
177-183: LGTM! Improved test robustness.Replacing the empty object cast with an explicit minimal
Issueobject is a good improvement. This makes the test data more explicit and type-safe, aligning with better testing practices.frontend/src/components/skeletons/Card.tsx (2)
34-38: LGTM! Appropriate use of NOSONAR directive.The NOSONAR comment correctly documents the intentional use of index-based keys for static skeleton items with fixed length. Since these skeleton arrays never change order or content during render, using the index as a key is safe and performant.
60-64: LGTM! Appropriate use of NOSONAR directive.The NOSONAR comment correctly documents the intentional use of index-based keys for static skeleton items. This aligns with the maintainer guidance to use NOSONAR directives for static lists where index keys are safe.
frontend/src/components/RecentReleases.tsx (1)
32-38: LGTM! Stable composite key with required fields.The composite key
${item.repositoryName}-${item.tagName}is safe and stable because bothrepositoryNameandtagNameare required fields in theReleasetype (as shown in the relevant code snippets). This guarantees unique, stable keys without risk of undefined values. Removing theindexprop from theReleasecomponent is also appropriate.
|
3 participants
Resolves #2486
Description
Fixed React key prop implementation in to address the TypeScript rule S6479 violation.
Checklist
make check-testlocally; all checks and tests passed.