Skip to content

Card - fix missing types when sending useNative (send nativeProps instead) #1631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

M-i-k-e-l
Copy link
Collaborator

Description

Card - fix missing types when sending nativeProps (send nativeProps instead)
Regarding Omit<Exclude<NativeTouchableOpacityProps, TouchableOpacityProps>, 'style'>;, you have to Omit the style because it causes a typing error (they two styles are not the same).

Changelog

Card - add nativeProps prop, deprecate nativeProps, allows better typings for the allowed nativeProps.

@ethanshar
Copy link
Collaborator

@M-i-k-e-l
Im not sure about this solution.
The activeOpacity is part of RN TouchableOpacity props and activeScale is part of our own implementation of Incubator.TouchableOpacity
Also, this typing issue is not relevant only to Card but to everything that inherit from TouchableOpacity (Button as well)

And last I find it kinda of weird that that some of the props we inherit from Touchable and can be passed directly and some should be pass through nativeProps

@M-i-k-e-l
Copy link
Collaborator Author

We expose activeOpacity in our incubator version; but yes, this is not a general solution.
Should I close this?

@ethanshar
Copy link
Collaborator

Yes.
There's a better solution that can be done with TS.
It's called discriminated unions

https://www.typescriptlang.org/docs/handbook/typescript-in-5-minutes-func.html#discriminated-unions

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.

2 participants