-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Update hook usage in the NativeDetector #3714
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes React hook usage in the gesture handler components by consolidating duplicate effects and improving lifecycle management.
- Merges two
useEffecthooks with identical dependency arrays inuseGestureto eliminate redundancy - Adds proper
propsRefupdates through a dedicateduseEffectinHostGestureDetector.web.tsx - Replaces standalone cleanup
useEffecthooks with inline cleanup functions for better lifecycle management
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| useGesture.ts | Consolidates two useEffect hooks with same dependencies into one |
| HostGestureDetector.web.tsx | Adds propsRef update effect and replaces cleanup effects with cleanup function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
packages/react-native-gesture-handler/src/v3/NativeDetector/HostGestureDetector.web.tsx
Show resolved
Hide resolved
packages/react-native-gesture-handler/src/v3/NativeDetector/HostGestureDetector.web.tsx
Show resolved
Hide resolved
packages/react-native-gesture-handler/src/v3/NativeDetector/HostGestureDetector.web.tsx
Show resolved
Hide resolved
akwasniewski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Description
useEffectswith the same dependency array into one inuseGestureuseEffectresponsible for updating thepropsRefinHostGestureDetector.web.tsxuseEffectswhich were only callingdetachHandlersand replaces them with a cleanup function inHostGestureDetector.web.tsx