[iOS] Remove a stale TODO in State.ts and the unused constants export - #4457
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe native gesture handler module removes legacy state and direction constant exports and their unused direction header import. TurboModule construction remains unchanged. The ChangesGesture constant export removal
Merge Risk: ⚪ Minimal · up to This is a localized cleanup with no intended runtime behavior change; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Pull request overview
Removes stale, unreachable constants-export plumbing and a leftover TODO that referenced it, keeping the JS State values as the single source of truth and trimming dead iOS native module code.
Changes:
- Removed the stale
TODO use State from RNModulecomment fromsrc/State.ts. - Removed the unused iOS
constantsToExportimplementation (State/Direction dictionaries) and its now-unusedRNGestureHandlerDirection.himport.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/react-native-gesture-handler/src/State.ts | Deletes a stale TODO comment above the JS State constant. |
| packages/react-native-gesture-handler/apple/RNGestureHandlerModule.mm | Removes unused constantsToExport and the unused Direction header import. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return @{ | ||
| @"State" : @{ | ||
| @"UNDETERMINED" : @(RNGestureHandlerStateUndetermined), | ||
| @"BEGAN" : @(RNGestureHandlerStateBegan), | ||
| @"ACTIVE" : @(RNGestureHandlerStateActive), | ||
| @"CANCELLED" : @(RNGestureHandlerStateCancelled), | ||
| @"FAILED" : @(RNGestureHandlerStateFailed), | ||
| @"END" : @(RNGestureHandlerStateEnd) | ||
| }, | ||
| @"Direction" : @{ | ||
| @"RIGHT" : @(RNGestureHandlerDirectionRight), | ||
| @"LEFT" : @(RNGestureHandlerDirectionLeft), | ||
| @"UP" : @(RNGestureHandlerDirectionUp), | ||
| @"DOWN" : @(RNGestureHandlerDirectionDown) | ||
| } | ||
| }; |
There was a problem hiding this comment.
Android module didn't export it?
Description
Removes two leftovers that reference each other:
The
TODO use State from RNModulecomment insrc/State.ts, added in the TS rewrite (Rewrite to TS #1327). The idea was to source theStateobject from the native module's exported constants instead of duplicating the values in JS. That's no longer viable: the TurboModule spec has nogetConstants, web and Jest have no native module to ask, andas constliteral types have to exist at compile time anyway.The
constantsToExportmethod inRNGestureHandlerModule.mm(theStateandDirectiondictionaries, exported since 2017) together with the now-unusedRNGestureHandlerDirection.himport. Nothing on the JS side reads these - withoutgetConstantsin the spec they are unreachable on the new architecture.requiresMainQueueSetupstays, as module initialization inserts into the static_managersmap that component views read on the main thread.No behavior change.
Test plan
basic-exampleon the iOS simulator; the app renders and a tap on theVirtualGestureDetectortext logsTapped on first part!.getConstantsinNativeRNGestureHandlerModule.ts, noModule.State/Module.Directionreads insrc/.