Add missing types #3731
Add missing types #3731
Conversation
| export { | ||
| SingleGestureName, | ||
| SingleGestureType, | ||
| ComposedGesture as ComposedGestureType, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 😅)
| gestureRelations: GestureRelations; | ||
| }; | ||
|
|
||
| export type SingleGestureType = SingleGesture<unknown, unknown>; |
There was a problem hiding this comment.
What do you think about a union: TapGestureType | PanGestureType | ...
It would have to be defined elsewhere, though.
There was a problem hiding this comment.
Great idea, done in 43bd77d
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 |
| 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'; |
There was a problem hiding this comment.
If this imports only types, please use import type instead of only import
There was a problem hiding this comment.
Ok, done in 704d604
| export { | ||
| SingleGestureName, | ||
| SingleGestureType, | ||
| ComposedGesture as ComposedGestureType, |
There was a problem hiding this comment.
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 type FlingGestureEvent = | ||
| | GestureStateChangeEvent<FlingHandlerData> | ||
| | GestureUpdateEvent<FlingHandlerData>; |
There was a problem hiding this comment.
Do we actually want to combine these? At the end of the day, those events are accessed in different callbacks
Description
While writing examples I noticed that some types would be useful. This includes types for each gesture and their HandlerData. This PR adds those types.
Important: The v3 gesture types take over the names of old v2 types, the v2 types are getting a type suffix. This change will have to be added to the migration guide. Added this as a task to roadmap.
Test plan
yarn lint-js