Export new createNativeWrapper
#3971
Conversation
There was a problem hiding this comment.
Pull request overview
Exports the RNGH3 (v3) createNativeWrapper as the public createNativeWrapper API while preserving the previous implementation under legacy_createNativeWrapper, and updates migration guidance/docs accordingly.
Changes:
- Re-export v3
createNativeWrapperand rename the legacy export tolegacy_createNativeWrapper. - Add/extend upgrade + migration-skill documentation for the new wrapper, including the new
detectorTypeparameter. - Add a dedicated docs page for
createNativeWrapper.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/gesture-handler-3-migration/SKILL.md | Updates LLM migration instructions to mention the rewritten createNativeWrapper and legacy fallback. |
| packages/react-native-gesture-handler/src/v3/index.ts | Exports the new v3 createNativeWrapper. |
| packages/react-native-gesture-handler/src/index.ts | Renames the legacy export to legacy_createNativeWrapper while continuing to export v3 APIs. |
| packages/docs-gesture-handler/docs/guides/upgrading-to-3.mdx | Adds an upgrade-guide section describing the new createNativeWrapper. |
| packages/docs-gesture-handler/docs/components/create-native-wrapper.mdx | Adds a new API docs page for createNativeWrapper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| config: NativeWrapperProperties, | ||
| detectorType: GestureDetectorType, | ||
| ): React.FC<P> |
There was a problem hiding this comment.
The createNativeWrapper API signature in the docs marks config and detectorType as required and the return type as React.FC<P>, but the implementation provides defaults for both parameters and the returned component also accepts wrapper props (e.g., NativeWrapperProperties). Please update the signature/return type to reflect the actual exported types so users don't lose type-safety or miss available props.
| config: NativeWrapperProperties, | |
| detectorType: GestureDetectorType, | |
| ): React.FC<P> | |
| config?: NativeWrapperProperties, | |
| detectorType?: GestureDetectorType, | |
| ): React.FC<P & NativeWrapperProperties> |
There was a problem hiding this comment.
Updated return type in 4f54383.
I don't think we should add ? to they arguments' types as this is true only from user's perspective (cc @j-piasecki)
There was a problem hiding this comment.
I don't understand "as this is true only from the user's perspective". Isn't the point of the docs to be read by users?
There was a problem hiding this comment.
What I meant by that is that true, users can see it that way, but in reality it is not. If we were to change it, I'd prefer to use default values instead.
Simply put, I don't want to say that it can be undefined if it can't be.
There was a problem hiding this comment.
Default values also work.
Simply put, I don't want to say that it can be undefined if it can't be.
In the same way, users shouldn't think that they need to set that when they don't.
There was a problem hiding this comment.
In the same way, users shouldn't think that they need to set that when they don't.
That's why later on it is described as optional. But I agree that type should also reflect that. Added in 319914c (along with `onGestureUpdate_CAN_CAUSE_INFINITE_RERENDER)
| ### detectorType | ||
|
|
||
| ```ts | ||
| enum GestureDetectorType { | ||
| Native, | ||
| Virtual, | ||
| Intercepting, | ||
| } | ||
| ``` | ||
|
|
||
| Type of the gesture detector that will be used to recognize the `Native` gesture. For more details on available options, see the [Gesture Detectors](/docs/fundamentals/gesture-detectors) documentation. Defaults to `GestureDetectorType.Native` (which is just [`GestureDetector`](/docs/fundamentals/gesture-detectors#gesture-detector)). | ||
|
|
There was a problem hiding this comment.
The docs suggest you can switch to GestureDetectorType.Virtual by just passing detectorType, but the underlying VirtualGestureDetector requires being rendered under an InterceptingGestureDetector (it throws otherwise). Consider adding an explicit note/example showing the required InterceptingGestureDetector wrapper when using GestureDetectorType.Virtual.
There was a problem hiding this comment.
This is described in the gesture detectors section, so I'm not sure if it is required here. If someone doesn't know how it works, they will probably have to look into these docs anyway (cc @akwasniewski)
| ); | ||
| } | ||
| ``` | ||
| In this scenario, the `Switch` from React Native cannot be toggled on because the `tap1` gesture intercepts it. However, when wrapped with `createNativeWrapper`, the `RNGHSwitch` becomes capable of participating in the gesture recognition process. This setup allows the switch to be toggled on while still enabling `tap2` to recognize taps on it. No newline at end of file |
There was a problem hiding this comment.
This works with no relations set?
There was a problem hiding this comment.
On all platforms or is it a one-off?
There was a problem hiding this comment.
Seems like it works differently across platforms. However, I think we can fix this in other PR - I've added entry in our roadmap so we don't forget about it.
2 participants
Description
This PR exports new
createNativeWrapperimplementation and renames old one tolegacy_createNativeWrapper. It also updated documentation, along with skill for LLM migration.Test plan
Check example from new docs.