Repository navigation
fix: replace index-based React keys with stable identifiers - #209
Conversation
- Rating component: Use `star-${index}` for star rating items
- Carousel component: Use `indicator-${index}` for carousel indicators
- Slider component: Use `thumb-${index}` for slider thumbs
- Skeleton component: Use `skeleton-text-${index}` for text lines
- Navigation: Use slugified title as key instead of index
- Product info: Use `badge-${variant}-${idx}` for badges
- Stats section: Use `stat-${year}` for stat cards
- Contact page: Use item label as key
- Orders skeleton: Add prefixed keys for mobile/desktop skeletons
- Product grid skeleton: Use `product-skeleton-${i}` prefix
Closes #64
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Changed Files
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request systematically replaces index-based React keys with stable identifiers across multiple components in the frontend demo and UI library. Keys now use slugified labels, semantic names (e.g., Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Mesa DescriptionTL;DRReplaced index-based React keys with stable, meaningful identifiers across UI library and frontend demo app components to improve rendering performance and reliability. What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Performed full review of 796c3ba...5a0a91a
Analysis
-
Badge Key Implementation Issue: In product-info.tsx, the key pattern
badge-${badge.variant}-${idx}still includes array indices, potentially causing React reconciliation issues. The keys may not be unique if badges share the same variant. -
Navigation Slugify Dependency Risk: Using
slugify(item.title)for keys assumes titles remain unique after slugification. If two navigation items have titles that slugify to the same value, React will issue key warnings. -
Skeleton Loader Keys Unnecessary Complexity: While not harmful, adding prefixes like
skeleton-text-${index}provides minimal value for static skeleton loaders that never reorder, introducing complexity without significant benefit.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
0 files reviewed | 0 comments | Edit Agent Settings • Read Docs
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
apps/frontend-demo/src/app/contact/page.tsx(5 hunks)apps/frontend-demo/src/components/account/orders-skeleton.tsx(2 hunks)apps/frontend-demo/src/components/molecules/navigation.tsx(1 hunks)apps/frontend-demo/src/components/molecules/product-grid-skeleton.tsx(1 hunks)apps/frontend-demo/src/components/organisms/product-info.tsx(1 hunks)apps/frontend-demo/src/components/stats-section.tsx(1 hunks)apps/frontend-demo/src/data/contact-content.ts(2 hunks)libs/ui/src/atoms/rating.tsx(1 hunks)libs/ui/src/atoms/skeleton.tsx(1 hunks)libs/ui/src/molecules/carousel.tsx(1 hunks)libs/ui/src/molecules/slider.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
libs/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement UI library components using Zag.js for React and Tailwind CSS for styling
Files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
libs/ui/src/{atoms,molecules}/**/*.tsx
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/{atoms,molecules}/**/*.tsx: React 19 components must not use forwardRef; pass ref as a prop instead
Do not use useCallback in components
Component implementations should use tv() with token-based classes (no raw Tailwind arbitrary values)
Use data attributes for state-based styling (e.g., data-[state=open], data-[disabled])
Do not use arbitrary Tailwind values in class names (e.g., bg-[#ff0000], bg-[var(--...)]); use token classes instead
Do not use direct semantic tokens like bg-primary; use component-specific token classes (e.g., bg-button-bg-primary)
Exported React components should be PascalCase (e.g., ComponentName)
Do not use abbreviations in component names (e.g., btn, pc)
When using Zag.js, derive styles from data attributes (e.g., data-state) rather than imperative styling
Files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
libs/ui/src/{atoms,molecules}/[a-z0-9-]*.tsx
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
Component files must be placed under src/atoms or src/molecules and named in kebab-case (component-name.tsx)
Files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx
📄 CodeRabbit inference engine (apps/frontend-demo/CLAUDE.md)
apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx: Place app-specific components in apps/frontend-demo/src/components/[atoms|molecules|organisms]/component-name.tsx
In each component TSX file, import its token CSS (e.g., import './_component-name.css')
Define component props with TypeScript interfaces
Use functional components and hooks for implementation
Use tv() with slots for component styles; only add variants/sizes when necessary
Use Tailwind classes mapped to tokens (e.g., bg-button-bg, p-button-sm); never use arbitrary CSS variable values like bg-[var(--...)] or p-[var(--...)]
Do not hardcode colors or spacing in JSX/TSX; use design tokens and/or tv()
Files:
apps/frontend-demo/src/components/organisms/product-info.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsx
libs/ui/src/molecules/**/*.tsx
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
Interactive components must use Zag.js patterns (machine, useMachine, normalizeProps, connect)
Files:
libs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Use data attributes for state-based styling (e.g., data-[state=open], data-[disabled])
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Use functional components and hooks for implementation
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Do not use direct semantic tokens like bg-primary; use component-specific token classes (e.g., bg-button-bg-primary)
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Use functional components and hooks for implementation
Applied to files:
apps/frontend-demo/src/components/account/orders-skeleton.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsxapps/frontend-demo/src/app/contact/page.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : In each component TSX file, import its token CSS (e.g., import './_component-name.css')
Applied to files:
apps/frontend-demo/src/components/account/orders-skeleton.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxapps/frontend-demo/src/app/contact/page.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Do not hardcode colors or spacing in JSX/TSX; use design tokens and/or tv()
Applied to files:
apps/frontend-demo/src/components/account/orders-skeleton.tsxlibs/ui/src/atoms/rating.tsxapps/frontend-demo/src/components/stats-section.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Use tv() with slots for component styles; only add variants/sizes when necessary
Applied to files:
apps/frontend-demo/src/components/account/orders-skeleton.tsxlibs/ui/src/atoms/skeleton.tsxapps/frontend-demo/src/components/stats-section.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Place app-specific components in apps/frontend-demo/src/components/[atoms|molecules|organisms]/component-name.tsx
Applied to files:
apps/frontend-demo/src/components/account/orders-skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsx
📚 Learning: 2025-12-16T19:45:17.746Z
Learnt from: BleedingDev
Repo: NMIT-WR/new-engine PR: 207
File: libs/ui/src/molecules/select.tsx:50-50
Timestamp: 2025-12-16T19:45:17.746Z
Learning: When reviewing Tailwind classes in TSX/TS files, prefer using square brackets for arbitrary CSS values and complex expressions. Specifically: - Do not use the parentheses syntax (z-(--z-index)) for anything beyond simple CSS variable references; this syntax auto-wraps in var() and cannot handle calc or complex functions. - Use the square brackets syntax (e.g., h-[calc(var(--available-height)-var(--spacing-content))]) for calc expressions, var with calc, and any complex CSS expressions. This rule applies broadly to Tailwind v4 usage in TSX code across the project.
Applied to files:
apps/frontend-demo/src/components/account/orders-skeleton.tsxlibs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxapps/frontend-demo/src/components/stats-section.tsxapps/frontend-demo/src/components/organisms/product-info.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsxapps/frontend-demo/src/app/contact/page.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Use data attributes for state-based styling (e.g., data-[state=open], data-[disabled])
Applied to files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxapps/frontend-demo/src/components/stats-section.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Do not use direct semantic tokens like bg-primary; use component-specific token classes (e.g., bg-button-bg-primary)
Applied to files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Component implementations should use tv() with token-based classes (no raw Tailwind arbitrary values)
Applied to files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxapps/frontend-demo/src/components/stats-section.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Exported React components should be PascalCase (e.g., ComponentName)
Applied to files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxapps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxlibs/ui/src/molecules/slider.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Do not use abbreviations in component names (e.g., btn, pc)
Applied to files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxlibs/ui/src/molecules/slider.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : When using Zag.js, derive styles from data attributes (e.g., data-state) rather than imperative styling
Applied to files:
libs/ui/src/atoms/rating.tsxapps/frontend-demo/src/components/stats-section.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Do not use arbitrary Tailwind values in class names (e.g., bg-[#ff0000], bg-[var(--...)]); use token classes instead
Applied to files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/[a-z0-9-]*.tsx : Component files must be placed under src/atoms or src/molecules and named in kebab-case (component-name.tsx)
Applied to files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsx
📚 Learning: 2025-09-07T21:28:54.320Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T21:28:54.320Z
Learning: Applies to apps/{medusa-fe,medusa-demo}/**/*.{ts,tsx} : Import UI components via the alias paths (e.g., `libs/ui/atoms/button`, `libs/ui/molecules/dialog`) rather than relative paths
Applied to files:
libs/ui/src/atoms/rating.tsxlibs/ui/src/atoms/skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsxapps/frontend-demo/src/app/contact/page.tsxlibs/ui/src/molecules/slider.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : React 19 components must not use forwardRef; pass ref as a prop instead
Applied to files:
libs/ui/src/atoms/skeleton.tsxlibs/ui/src/molecules/carousel.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Define component props with TypeScript interfaces
Applied to files:
apps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/molecules/**/*.tsx : Interactive components must use Zag.js patterns (machine, useMachine, normalizeProps, connect)
Applied to files:
apps/frontend-demo/src/components/molecules/product-grid-skeleton.tsxapps/frontend-demo/src/components/molecules/navigation.tsxlibs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/stories/**/*.stories.tsx : Use Storybook title hierarchy matching the layer (e.g., Atoms/ComponentName)
Applied to files:
apps/frontend-demo/src/components/molecules/navigation.tsx
📚 Learning: 2025-10-24T13:43:25.253Z
Learnt from: KaiUweCZE
Repo: NMIT-WR/new-engine PR: 167
File: libs/ui/src/molecules/tabs.tsx:41-45
Timestamp: 2025-10-24T13:43:25.253Z
Learning: In libs/ui/src/molecules/tabs.tsx, using Tailwind arbitrary value syntax to reference Zag.js CSS variables (e.g., w-(--width), h-(--height)) for the indicator element is the correct pattern. These CSS variables are dynamically set by getIndicatorProps() based on active tab dimensions and should not be removed.
Applied to files:
libs/ui/src/molecules/slider.tsxlibs/ui/src/molecules/carousel.tsx
🧬 Code graph analysis (3)
apps/frontend-demo/src/components/organisms/product-info.tsx (1)
libs/ui/src/atoms/badge.tsx (1)
Badge(70-100)
apps/frontend-demo/src/components/molecules/navigation.tsx (1)
libs/ui/src/utils.ts (1)
slugify(19-28)
apps/frontend-demo/src/app/contact/page.tsx (5)
apps/frontend-demo/src/hooks/use-contact-form.ts (1)
useContactForm(36-100)libs/ui/src/atoms/image.tsx (1)
Image(28-38)libs/ui/src/molecules/form-input.tsx (1)
FormInput(57-85)libs/ui/src/utils.ts (1)
slugify(19-28)libs/ui/src/atoms/icon.tsx (1)
Icon(41-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Mesa Description
🔇 Additional comments (14)
apps/frontend-demo/src/components/stats-section.tsx (1)
62-62: Excellent use of stable identifier!Using
stat.yearas the key is a genuine improvement over index-based keys. This ensures stable identity even if the stats array is reordered or filtered, which aligns perfectly with the PR objectives.libs/ui/src/molecules/carousel.tsx (1)
434-434: Key change meets PR objectives.The conversion to a string-based key aligns with the PR goal of replacing index-based keys. For carousel indicators derived from
api.pageSnapPoints, this provides consistent key formatting across the codebase.libs/ui/src/atoms/rating.tsx (1)
129-129: String-based key applied correctly.The change to
star-${index}maintains unique keys for each rating item and aligns with the PR pattern of using string-based keys throughout the codebase.libs/ui/src/atoms/skeleton.tsx (1)
236-236: Consistent key formatting applied.The string-based key
skeleton-text-${index}follows the established pattern in this PR and ensures consistent key formatting across skeleton components.libs/ui/src/molecules/slider.tsx (1)
271-271: Key change applied correctly.Using
thumb-${index}for slider thumbs maintains unique identifiers and follows the string-based key pattern established in this PR.apps/frontend-demo/src/components/organisms/product-info.tsx (1)
58-58: Composite key provides better stability.Using
badge-${badge.variant}-${idx}incorporates semantic information (variant) along with the index, which improves key stability compared to index alone. This is particularly useful if badges are filtered or reordered based on variant.apps/frontend-demo/src/components/account/orders-skeleton.tsx (2)
32-32: Context-specific key applied correctly.Using
mobile-skeleton-${i}clearly identifies these skeleton items as mobile-specific and maintains consistency with the PR's string-based key pattern.
70-70: Desktop skeleton key applied correctly.The
desktop-skeleton-${i}key appropriately distinguishes desktop skeleton items and follows the established naming pattern.apps/frontend-demo/src/components/molecules/product-grid-skeleton.tsx (1)
9-9: String-based key applied correctly.The
product-skeleton-${i}key follows the established pattern in this PR and ensures consistent key formatting across skeleton components.apps/frontend-demo/src/data/contact-content.ts (1)
1-117: LGTM – Quote normalization aligns with project conventions.The conversion from single to double quotes is purely stylistic and improves consistency across the codebase. No functional changes introduced.
apps/frontend-demo/src/components/molecules/navigation.tsx (2)
4-4: Good addition of the slugify utility.Importing
slugifysupports the stable key generation pattern used in this component.
36-46: Consistent use of slugified keys in submenu children.The child navigation items correctly use
slugify(child.title)for keys, maintaining consistency with the parent list implementation.apps/frontend-demo/src/app/contact/page.tsx (2)
8-8: Good addition of the slugify utility.Importing
slugifysupports the stable key generation used for contact information items.
143-170: Contact item labels are unique and key approach is sound.The replacement of index-based keys with
slugify(item.label)correctly addresses the PR objective. Verification confirms that all contact item labels in the data structure are unique, with no duplicate slugs produced after stringification. The current implementation usingslugify(item.label)is safe and appropriate for React key usage.
| "use client" | ||
| import { Button } from "@techsio/ui-kit/atoms/button" | ||
| import { Icon, type IconType } from "@techsio/ui-kit/atoms/icon" | ||
| import { LinkButton } from "@techsio/ui-kit/atoms/link-button" | ||
| import { FormInput } from "@techsio/ui-kit/molecules/form-input" | ||
| import { FormTextarea } from "@techsio/ui-kit/molecules/form-textarea" | ||
| import { Select } from "@techsio/ui-kit/molecules/select" | ||
| import { slugify } from "@techsio/ui-kit/utils" | ||
| import Image from "next/image" | ||
| import { contactContent } from "@/data/contact-content" | ||
| import { useContactForm } from "@/hooks/use-contact-form" | ||
| import contactImage from "/assets/hero/contact.webp" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Scope concern: Multiple unrelated changes included in this PR.
This PR includes several changes that are unrelated to the stated objective of replacing index-based React keys:
- Image priority prop (line 28-29): Adding
priority={true}is an optimisation change - Form field props (lines 64-65, 74-75, 85-87, etc.): Adding explicit
size,value,rows, andtypeprops appears to be form structure refinement - Icon type casting (line 150): The
as IconTypecast is a TypeScript typing change - LinkButton hover styles (line 183): Adding
hover:bg-transparentis a styling change - Quote normalization (lines 61, 71, 81, etc.): Converting single to double quotes is a style change
Including multiple concerns in a single PR makes it harder to:
- Review each change on its merits
- Understand the purpose from the commit history
- Roll back specific changes if needed
Consider splitting these into separate commits or PRs aligned with the conventional commit guidelines (e.g., fix: for keys, style: for quotes, refactor: for form props, perf: for image priority).
Also applies to: 28-29, 57-132, 150-150, 183-183
🤖 Prompt for AI Agents
In apps/frontend-demo/src/app/contact/page.tsx (lines 1-12, 28-29, 57-132, 150,
183) this PR mixes the intended key-fix with unrelated changes (image priority,
form prop additions, icon type cast, style tweak, and quote normalization);
revert or remove all unrelated edits from this branch so the PR contains only
the index-key fix, then create separate commits/PRs for: the perf change (add
priority on Image), form refinements (size/value/type/rows), TypeScript cast for
Icon, styling change to LinkButton, and quote/style-only changes—use focused
branch per concern and clear conventional commit messages (fix:, perf:,
refactor:, style:) before reopening the PR.
| {items.map((item) => ( | ||
| <NavigationItem key={slugify(item.title)} item={item} /> | ||
| ))} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find navigation-related files and data definitions
find apps/frontend-demo -type f -name "*.ts" -o -name "*.tsx" -o -name "*.json" | \
xargs grep -l "navigation\|nav.*item" -i | head -20Repository: NMIT-WR/new-engine
Length of output: 769
🏁 Script executed:
#!/bin/bash
# Look for the navigation.tsx file and check its structure
if [ -f "apps/frontend-demo/src/components/molecules/navigation.tsx" ]; then
echo "=== Navigation.tsx file found ==="
cat "apps/frontend-demo/src/components/molecules/navigation.tsx"
else
echo "File not found. Searching for navigation files..."
find . -name "navigation.tsx" -o -name "navigation.ts" | grep -v node_modules
fiRepository: NMIT-WR/new-engine
Length of output: 2806
🏁 Script executed:
#!/bin/bash
# Search for where navigation items/data are defined
rg "export.*nav.*items|navigation.*:.*\[" --type ts --type tsx -A 10 | head -100Repository: NMIT-WR/new-engine
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Find files that import/use Navigation component
rg "import.*Navigation|<Navigation" --type ts --type tsx -B 2 -A 5 | head -150Repository: NMIT-WR/new-engine
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Find files that import or use the Navigation component
grep -r "import.*Navigation\|<Navigation" apps/frontend-demo/src --include="*.ts" --include="*.tsx" -B 2 -A 5 | head -200Repository: NMIT-WR/new-engine
Length of output: 2103
🏁 Script executed:
#!/bin/bash
# Find where navigationItems is defined in header.tsx
grep -n "navigationItems" apps/frontend-demo/src/components/header.tsx -B 10 -A 2 | head -100Repository: NMIT-WR/new-engine
Length of output: 1576
🏁 Script executed:
#!/bin/bash
# Find where Header component is used and where navigationItems come from
grep -r "<Header\|navigationItems:" apps/frontend-demo/src --include="*.ts" --include="*.tsx" -B 3 -A 10 | head -250Repository: NMIT-WR/new-engine
Length of output: 4722
🏁 Script executed:
#!/bin/bash
# Find the full definition of categoryItems
grep -n "categoryItems" apps/frontend-demo/src/components/header-wrapper.tsx -B 30 | head -50Repository: NMIT-WR/new-engine
Length of output: 1454
🏁 Script executed:
#!/bin/bash
# Check the full header-wrapper.tsx file to see the complete context
cat apps/frontend-demo/src/components/header-wrapper.tsxRepository: NMIT-WR/new-engine
Length of output: 1607
🏁 Script executed:
#!/bin/bash
# Check if there are any other places where Navigation items might be used dynamically
grep -r "navigationItems\|NavItem\[\]" apps/frontend-demo/src --include="*.ts" --include="*.tsx" | grep -v "node_modules"Repository: NMIT-WR/new-engine
Length of output: 1250
🏁 Script executed:
#!/bin/bash
# Extract all navigation titles to verify uniqueness
cat > /tmp/check_slugs.js << 'EOF'
const titles = [
'Domů',
'Produkty',
'Oblíbené',
'O nás',
'Kontakt',
'Město',
'Zimní',
'Obuv',
'Sport'
];
// Simple slugify approximation
function slugify(str) {
return str
.toLowerCase()
.normalize('NFD')
.replace(/[\u0300-\u036f]/g, '')
.replace(/[^a-z0-9\s-]/g, '')
.replace(/\s+/g, '-')
.replace(/-+/g, '-')
.trim();
}
const slugs = titles.map(t => ({ title: t, slug: slugify(t) }));
console.log('Title -> Slug mapping:');
slugs.forEach(s => console.log(` ${s.title} -> ${s.slug}`));
const uniqueSlugs = new Set(slugs.map(s => s.slug));
console.log(`\nTotal titles: ${titles.length}`);
console.log(`Unique slugs: ${uniqueSlugs.size}`);
if (titles.length !== uniqueSlugs.size) {
console.log('⚠️ DUPLICATES FOUND!');
slugs.forEach(s => {
const count = slugs.filter(x => x.slug === s.slug).length;
if (count > 1) {
console.log(` "${s.title}" produces slug "${s.slug}" (${count} items)`);
}
});
}
EOF
node /tmp/check_slugs.jsRepository: NMIT-WR/new-engine
Length of output: 282
Consider augmenting the key with additional context to prevent potential collisions.
The current implementation with slugify(item.title) as a key works correctly for the existing navigation data (all titles are unique). However, to guard against future issues if duplicate titles are added, consider using a composite key. Since each NavItem has an href property, you could strengthen the key: key={slugify(item.title)}-${item.href} or similar. This follows React's requirement that keys be unique among siblings more defensively and would prevent unexpected behaviour if the navigation data changes.
Note: This pattern also applies to the submenu children at line 43.
🤖 Prompt for AI Agents
In apps/frontend-demo/src/components/molecules/navigation.tsx around lines 80-82
(and apply similarly to submenu children near line 43), the key uses only
slugify(item.title) which can collide if titles repeat; change the key to a
composite value that includes another stable field such as item.href (for
example combine slugified title and href) so each sibling key is unique and
resistant to future duplicate titles; update both the top-level NavigationItem
and submenu child keys accordingly to use the composite key.
Summary
key={index}antipattern with stable, meaningful keys across UI library and app componentsstar-${index},stat-${year}, slugified titles)Changes Made
UI Library (
libs/ui)star-${index}for rating itemsindicator-${index}for carousel indicatorsthumb-${index}for slider thumbsskeleton-text-${index}for skeleton text linesFrontend Demo App (
apps/frontend-demo)badge-${variant}-${idx}for badgesstat-${year}for stat cards (unique year identifier)mobile-skeleton-${i},desktop-skeleton-${i})product-skeleton-${i}prefixTest Plan
Related Issues
Closes #64
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.