Skip to content

Conversation

akwasniewski
Copy link
Contributor

@akwasniewski akwasniewski commented Oct 3, 2025

Description

While writing examples I noticed that some types would be useful. This includes types for each gesture and their events. This PR adds those types.

Important: The v3 gesture types take over the names of old v2 types, the v2 types are getting a Legacy prefix. This change will have to be added to the migration guide. Added this as a task to roadmap.

Test plan

yarn lint-js

@akwasniewski akwasniewski marked this pull request as draft October 3, 2025 11:01
@akwasniewski akwasniewski marked this pull request as ready for review October 3, 2025 11:24
Copy link
Contributor

@m-bert m-bert left a comment

Choose a reason for hiding this comment

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

Looks ok. Do we need SingleGestureType exported?

export {
SingleGestureName,
SingleGestureType,
ComposedGesture as ComposedGestureType,
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, what do you think about going in the other direction and exporting SingleGesture, LongPressGesture, ...?

That will conflict with the API v2, so we may want to rename those and keep that in mind for the migration guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that would be prefarable, and was my first choice, but I didn't want to conflict with v2 api. If you think it is Ok I will gladly change it.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think we can rename the old types to something else to free up the "makes sense" names for the future. We would have to include that in the migration guide.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll just point out that we should write it somewhere in case we forget. Not sure if doing TODO comment` is the best, but but we could include it in the roadmap.


export { SingleGestureName } from './v3/types';
export {
SingleGestureName,
Copy link
Member

Choose a reason for hiding this comment

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

Why is that needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SingleGestureName? I'm preety sure it is no longer needed after we added dedicated gesture hooks, am I correct cc @m-bert? I could remove it in this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think it is no longer required. It was with useGesture but now it is only internal (but please make sure that it's true before removing it 😅)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it in c7a25ef

gestureRelations: GestureRelations;
};

export type SingleGestureType = SingleGesture<unknown, unknown>;
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about a union: TapGestureType | PanGestureType | ...

It would have to be defined elsewhere, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea, done in 43bd77d

@akwasniewski
Copy link
Contributor Author

akwasniewski commented Oct 3, 2025

Looks ok. Do we need SingleGestureType exported?

I figured there is a need for a more general type than handlers corresponding to each gesture, so the user doesn't have to do PanGesture | TapGesture | .... Gesture and GestureType were taken so I figured I can stick with SingleGesture name

Comment on lines 1 to 9
import { FlingGestureEvent, FlingGesture } from './useFling';
import { HoverGestureEvent, HoverGesture } from './useHover';
import { LongPressGestureEvent, LongPressGesture } from './useLongPress';
import { ManualGestureEvent, ManualGesture } from './useManual';
import { NativeGestureEvent, NativeGesture } from './useNative';
import { PanGestureEvent, PanGesture } from './usePan';
import { PinchGestureEvent, PinchGesture } from './usePinch';
import { RotationGestureEvent, RotationGesture } from './useRotation';
import { TapGestureEvent, TapGesture } from './useTap';
Copy link
Contributor

Choose a reason for hiding this comment

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

If this imports only types, please use import type instead of only import

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, done in 704d604

export {
SingleGestureName,
SingleGestureType,
ComposedGesture as ComposedGestureType,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll just point out that we should write it somewhere in case we forget. Not sure if doing TODO comment` is the best, but but we could include it in the roadmap.

@m-bert m-bert mentioned this pull request Oct 6, 2025
Comment on lines 59 to 61
export type FlingGestureEvent =
| GestureStateChangeEvent<FlingHandlerData>
| GestureUpdateEvent<FlingHandlerData>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually want to combine these? At the end of the day, those events are accessed in different callbacks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, it is better to separate it. Done in 5113634


export { SingleGestureName } from './v3/types';
export type { ComposedGesture } from './v3/types';
export type { GestureTouchEvent as SingleGestureTouchEvent } from './handlers/gestureHandlerCommon';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this reexport to keep naming consistency, but I'm not sure whether this is the best approach. Let me know what you think

Comment on lines 57 to 70
export type { TapGestureType } from './handlers/gestures/tapGesture';
export type { PanGestureType } from './handlers/gestures/panGesture';
export type { FlingGestureType } from './handlers/gestures/flingGesture';
export type { LongPressGestureType } from './handlers/gestures/longPressGesture';
export type { PinchGestureType } from './handlers/gestures/pinchGesture';
export type { RotationGestureType } from './handlers/gestures/rotationGesture';
export type { ForceTouchGestureType } from './handlers/gestures/forceTouchGesture';
export type { ManualGestureType } from './handlers/gestures/manualGesture';
export type { HoverGestureType } from './handlers/gestures/hoverGesture';
export type {
ComposedGestureType as ComposedGesture,
RaceGestureType as RaceGesture,
SimultaneousGestureType as SimultaneousGesture,
ExclusiveGestureType as ExclusiveGesture,
ComposedGestureType,
RaceGestureType,
SimultaneousGestureType,
ExclusiveGestureType,
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about that, TapGestureType may still be confused with TapGesture. What about OldTapGesture / LegacyTapGesture?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, changed it to LegacyTapGesture, though I'm not sure it is the best name.

Copy link
Contributor

Choose a reason for hiding this comment

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

Legacy*Gesture sounds fine 😅

Comment on lines +39 to +41
export type FlingGestureStateChangeEvent =
GestureStateChangeEvent<FlingHandlerData>;
export type FlingGestureUpdateEvent = GestureUpdateEvent<FlingHandlerData>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could move it (along with FlingHandlerData to FlingProperties file (and probably rename it so it would make sense).

But this one is also fine, if we decide to change it we can do it later 😅

@akwasniewski akwasniewski merged commit 1df7e30 into next Oct 16, 2025
4 checks passed
@akwasniewski akwasniewski deleted the @akwasniewski/add-missing-types branch October 16, 2025 06:10
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.

3 participants