Skip to content
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

Remove propTypes #86

Closed
ugrky opened this issue Oct 12, 2020 · 12 comments
Closed

Remove propTypes #86

ugrky opened this issue Oct 12, 2020 · 12 comments

Comments

@ugrky
Copy link

ugrky commented Oct 12, 2020

Is your feature request related to a problem? Please describe.
I am currently trying to run the library on react-native-web. I know that the library is not officially supported on web, but would be nice to make it work. I will be doing some changes on my own fork, but wanted to use this thread as a platform to discuss a few things.

react-native-web has unspoorted ViewPropTypes as of 0.12. Therefore, I get the following error:

Popover.tsx?f681:1 Uncaught TypeError: Cannot read property 'style' of undefined

Caused by the following lines:

popoverStyle: ViewPropTypes.style,
arrowStyle: ViewPropTypes.style,
backgroundStyle: ViewPropTypes.style,

Describe the solution you'd like
First I want to ask you why is PropTypes used in this repo while there is already great TypeScript implementation. If we get rid of PropTypes, it will work on react-native-web too. I will work on further improvements to make this library work on web, but I think it would be the first step.

I would be happy if you could inform me why you choose to opt in for PropTypes.

Thanks in advance!

@SteffeyDev
Copy link
Owner

Great question. For people who use typescript, then the types are great. However, any developers who use this library in a non-typescript project need PropTypes to ensure that React stops them from passing in invalid props. I'm not sure how libraries that support web handle this use case, maybe you could see how they do type enforcement for non-typescript projects and let me know?

@nandorojo
Copy link

nandorojo commented Oct 13, 2020

@SteffeyDev asked me to weigh in here. I only use TypeScript, so I’m not sure, but this might be relevant to necolas/react-native-web#1684

@nandorojo
Copy link

Just updated my comment with the correct link.

@SteffeyDev
Copy link
Owner

SteffeyDev commented Oct 13, 2020

Not sure if I missed it, does that issue relate to PropTypes at all?

@nandorojo
Copy link

Oh I see, this only relates to proptypes, not flow types, sorry got them mixed up since I don’t use either.

@nandorojo
Copy link

@SteffeyDev I ran into the same problem as this issue with react-native-calendars. I figured out a solution there.

All you have to do is conditionally import ViewPropTypes from react-native, and not import it on web. RNW removed it from the package since React Native plans on deprecating it.

This was the solution in react-native-calendars:

import { View, Platform } from 'react-native'

const viewPropTypes =
  typeof document !== 'undefined' || Platform.OS === 'web'
    ? PropTypes.shape({ style: PropTypes.object })
    : require('react-native').ViewPropTypes || View.propTypes

@SteffeyDev
Copy link
Owner

Ah very cool, I'll add this in and see if it works

@nandorojo
Copy link

To please TS, you might need to make the web type as any.

@SteffeyDev
Copy link
Owner

Actually TS has a problem with document and the fact that View does not have a member propTypes. I think I can exclude the document !== 'undefined' check, where did you get View.propTypes from?

@nandorojo
Copy link

Wix included that in their calendar library. I've never used prop types so I just copied that part over.

@SteffeyDev
Copy link
Owner

Interesting. Don't think I need that either.

SteffeyDev added a commit that referenced this issue Oct 24, 2020
@SteffeyDev
Copy link
Owner

Ok, this is done in the latest release, thanks for finding a good solution!

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

No branches or pull requests

3 participants