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

Replace React PropTypes with prop-types in RN CLI/examples #1710

Merged
merged 1 commit into from
Aug 22, 2017

Conversation

shilman
Copy link
Member

@shilman shilman commented Aug 22, 2017

Issue: CLI was still using react PropTypes which are deprecated

What I did

Upgraded RN to use prop-types package

How to test

yarn bootstrap && yarn build-packs && yarn bootstrap:react-native-vanilla
cd examples/react-native-vanilla
yarn storybook
./node_modules/.bin/react-native run-ios

@shilman shilman changed the title Use prop-types for RN CLI instead of deprecated react PropTypes Replace deprecated react PropTypes with prop-types in RN CLI/examples Aug 22, 2017
@shilman shilman changed the title Replace deprecated react PropTypes with prop-types in RN CLI/examples Replace React PropTypes with prop-types in RN CLI/examples Aug 22, 2017
@shilman shilman requested a review from a team August 22, 2017 17:03
@shilman shilman merged commit ff86d37 into master Aug 22, 2017
@shilman shilman deleted the upgrade-rn-cli-proptypes branch August 22, 2017 17:48
@ndelangen ndelangen added dependencies maintenance User-facing maintenance tasks labels Aug 22, 2017
@tmeasday
Copy link
Member

What happens if they haven't installed the prop-types package in their app?

@ndelangen
Copy link
Member

They will get an error ?

@tmeasday
Copy link
Member

Sure, but are we OK with that? If they are using an old version of React?

Probably it's OK now..

@tmeasday
Copy link
Member

Or is it OK because our packages depend on prop-types...? Maybe they won't get an error

@ndelangen
Copy link
Member

Our UI depends on it indeed, unless npm decided to be a 🐒 and not install flat, the module should always be there, no error occur.

@tmeasday
Copy link
Member

unless npm decided to be a 🐒

Ok, I'll consider us completely safe then.

@Hypnosphi
Copy link
Member

Hypnosphi commented Aug 22, 2017

What happens if they haven't installed the prop-types package in their app?

Thanks to eslint, we have it as a dependency everywhere it's used, as docs suggest

@Hypnosphi
Copy link
Member

Hypnosphi commented Aug 22, 2017

Generators don't add it though, and I believe we should fix that (as we did with @storybook/addons-* before in #1652)

@ndelangen
Copy link
Member

I agree!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants