Skip to content

Update Swipeable types#4175

Open
m-bert wants to merge 15 commits into
mainfrom
@mbert/update-types
Open

Update Swipeable types#4175
m-bert wants to merge 15 commits into
mainfrom
@mbert/update-types

Conversation

@m-bert
Copy link
Copy Markdown
Collaborator

@m-bert m-bert commented May 13, 2026

Description

Some of the props passed to Swipeable can be SharedValue. However, we do not allow that in types, so TypeScript throws errors.

Thank you @coado with help ❤️

Test plan

Static checks

Copilot AI review requested due to automatic review settings May 13, 2026 14:27
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 updates the public TypeScript API for ReanimatedSwipeable to reflect that certain numeric configuration props may be provided as Reanimated SharedValue<number>, aligning the typings/docs with intended usage.

Changes:

  • Expand SwipeableProps.dragOffsetFromLeft and SwipeableProps.dragOffsetFromRight to accept SharedValue<number> in addition to number.
  • Update the Reanimated Swipeable docs to reflect the updated prop types.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/react-native-gesture-handler/src/components/ReanimatedSwipeable/ReanimatedSwipeableProps.ts Widens dragOffsetFromLeft/dragOffsetFromRight prop types to include SharedValue<number>.
packages/docs-gesture-handler/docs/components/reanimated_swipeable.mdx Updates the documented TypeScript signatures for the same props.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

packages/react-native-gesture-handler/src/components/ReanimatedSwipeable/ReanimatedSwipeable.tsx:489

  • activeOffsetX requires the first tuple element to be <= 0 and the second to be >= 0 (validated by usePanGesture in DEV). Switching to [dragOffsetFromRight, dragOffsetFromLeft] changes the sign contract for dragOffsetFromRight compared to the documented/default behavior ("Defaults to 10"), and will break existing callers passing a positive number. Prefer preserving the existing API (positive distance) and handling the negation internally in a way that supports SharedValue, or update the default value + docs to explicitly require a non-positive dragOffsetFromRight.
  const panGesture = usePanGesture({
    enabled: enabled !== false,
    enableTrackpadTwoFingerGesture: enableTrackpadTwoFingerGesture,
    activeOffsetX: [dragOffsetFromRight, dragOffsetFromLeft],
    simultaneousWith,
    requireToFail,

Comment thread packages/docs-gesture-handler/docs/components/reanimated_swipeable.mdx Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread packages/docs-gesture-handler/docs/guides/upgrading-to-3.mdx Outdated
Comment thread skills/gesture-handler-3-migration/SKILL.md
m-bert and others added 3 commits May 14, 2026 10:34
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Comment thread packages/docs-gesture-handler/docs/components/reanimated_swipeable.mdx Outdated
Comment thread packages/docs-gesture-handler/docs/components/reanimated_swipeable.mdx Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@m-bert m-bert marked this pull request as ready for review May 14, 2026 10:01
@m-bert m-bert requested a review from j-piasecki May 14, 2026 10:02
@m-bert m-bert requested review from coado and relextm19 May 14, 2026 10:02
Comment on lines +69 to +90
if (Reanimated?.isSharedValue<number>(dragOffsetFromRight)) {
// eslint-disable-next-line react-hooks/rules-of-hooks
Reanimated?.useDerivedValue(() => {
'worklet';
if (dragOffsetFromRight.value > 0) {
throw new Error(
tagMessage('dragOffsetFromRight should be non-positive.')
);
}
return dragOffsetFromRight.value;
});
} else {
// eslint-disable-next-line react-hooks/rules-of-hooks
useEffect(() => {
if ((dragOffsetFromRight as number) > 0) {
throw new Error(
tagMessage('dragOffsetFromRight should be non-positive.')
);
}
}, [dragOffsetFromRight]);
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct. You can switch between a constant value and a constant.

This should be a single useEffect and if the value is a shared value, add a subscriber and remove it on cleanup.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can switch between a constant value and a constant.

You mean alternating between SharedValue and standard value? Isn't it weird use-case?

This should be a single useEffect and if the value is a shared value, add a subscriber and remove it on cleanup.

useDerivedValue was discussed with Reanimated team. But if you prefer manually adding listener than ok. wdyt?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it weird use-case?

Yes, but it's valid and would crash now.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think b2d4e5a

Comment on lines +77 to +80
useDerivedValue<T>(
updater: () => T,
dependencies?: readonly unknown[]
): SharedValue<T>; // Not exactly, but we don't need that anyway
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not be needed after that change.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m-bert added a commit that referenced this pull request May 15, 2026
## Description

Following #4175 I've decided to update types that can be passed as
`SharedValue` also in `Drawer`

## Test plan

Static checks
@m-bert m-bert requested a review from j-piasecki May 15, 2026 09:46
Copy link
Copy Markdown
Contributor

@coado coado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

'worklet';
if (maybeUnpackValue<number>(value) > 0) {
throw new Error(
tagMessage('dragOffsetFromRight should be non-negative.')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not a contradiction? First check if the value is more than 0 and then throw an error that says its less than 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants