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

fix: import ViewPropTypes #133

Closed
wants to merge 1 commit into from
Closed

Conversation

hytStart
Copy link

I think it's better to use import { ViewPropTypes } from 'react-native' here.

@SteffeyDev
Copy link
Owner

Have you tested this on web? Pretty sure we did it the other way because that workaround was needed for web support. Might be fixed by now, haven't looked at RN web support for a while.

@hytStart
Copy link
Author

@SteffeyDev Hi, here is my opinion.
I mean

// new
import { ViewPropTypes } from 'react-native'

const stylePropType =
  isWeb
    ? PropTypes.object
    : ViewPropTypes.style

not

// old
const stylePropType =
  isWeb
    ? PropTypes.object
    : require('react-native').ViewPropTypes.style

Still have the condition of isWeb

It can be seen in this picture.

image

Actually, I don't understand the reason for require('react-native'), instead of using import { ViewPropTypes } from 'react-native'

@SteffeyDev
Copy link
Owner

Thanks for the details. Please review #86 for a discussion of why we went with the current approach. A lot of things should work in theory, but in practice things are messier.

@SteffeyDev
Copy link
Owner

I'm fine with this change as long as you boot up a react native web project with your change and show that your change doesn't break web (and ideally share that environment as well so that I can confirm).

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

Successfully merging this pull request may close these issues.

2 participants