fix: remove key prop from list item children for better performance #71
Conversation
- Remove key prop from View children when using FlatList/FlashList - Keep key prop only for ScrollView children - Improves FlashList performance by allowing proper item reuse - Follows FlashList official performance guidelines Refs: https://shopify.github.io/flash-list/docs/fundamentals/performance#remove-key-prop
🦋 Changeset detectedLatest commit: 163e85f The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis change updates the "react-native-gesture-image-viewer" package to conditionally assign the Changes
Sequence Diagram(s)sequenceDiagram
participant ParentComponent
participant GestureViewer
participant ListComponent (FlatList/FlashList/ScrollView)
participant ChildItem
ParentComponent->>GestureViewer: Render with list component
GestureViewer->>GestureViewer: Memoize isScrollView based on Component
GestureViewer->>ListComponent: Render items using renderItem
alt isScrollView is true (e.g., ScrollView)
ListComponent->>ChildItem: Render with key prop
else isScrollView is false (e.g., FlatList/FlashList)
ListComponent->>ChildItem: Render without key prop
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested labels
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying react-native-gesture-image-viewer with
|
| Latest commit: |
163e85f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://32d5d220.react-native-gesture-image-viewer.pages.dev |
| Branch Preview URL: | https://fix-remove-key-prop-for-list.react-native-gesture-image-viewer.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/tender-cows-flow.md (1)
12-12: Fix markdownlint MD034 (bare URL).Use a markdown link to avoid a bare URL and satisfy linting.
-Refs: https://shopify.github.io/flash-list/docs/fundamentals/performance#remove-key-prop +Refs: [FlashList performance guidelines — Remove key prop](https://shopify.github.io/flash-list/docs/fundamentals/performance#remove-key-prop)src/GestureViewer.tsx (1)
38-39: Minor: consider memoizing isScrollView (and note summary discrepancy).AI summary mentions a “memoized boolean,” but here it’s a plain const. Functionally fine (cheap check), but to align with the summary and keep dependency identity stable:
-const isScrollView = isScrollViewLike(Component); +const isScrollView = useMemo(() => isScrollViewLike(Component), [Component]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/tender-cows-flow.md(1 hunks)src/GestureViewer.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/GestureViewer.tsx (2)
src/utils.ts (1)
isScrollViewLike(5-7)src/useGestureViewer.ts (1)
T(29-517)
🪛 markdownlint-cli2 (0.17.2)
.changeset/tender-cows-flow.md
12-12: Bare URL used
(MD034, no-bare-urls)
⏰ 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: Cloudflare Pages
🔇 Additional comments (4)
.changeset/tender-cows-flow.md (1)
5-11: Changelog content reads well and matches intent.Clear, actionable summary of the behavior change and rationale. LGTM.
src/GestureViewer.tsx (3)
76-76: Correct: only assign key for ScrollView children.This prevents keys on list item wrappers in FlatList/FlashList, enabling proper cell reuse and improved FlashList performance.
91-91: Dependencies look right.Including isScrollView ensures re-creation if ListComponent type changes. keyExtractor covers enableLoop changes, so no extra deps needed.
146-169: Clean conditional rendering with centralized isScrollView check.Using the isScrollView flag simplifies the JSX and keeps key assignment logic consistent across branches. FlatList/FlashList path correctly relies on keyExtractor only.
) - Remove key prop from View children when using FlatList/FlashList - Keep key prop only for ScrollView children - Improves FlashList performance by allowing proper item reuse - Follows FlashList official performance guidelines Refs: https://shopify.github.io/flash-list/docs/fundamentals/performance#remove-key-prop
Refs: https://shopify.github.io/flash-list/docs/fundamentals/performance#remove-key-prop
Summary by CodeRabbit