search

RealOrangeOne / react-native-busy-indicator

A simple, event controlled, busy indicator for react-native
MIT License
94 stars 41 forks source link

new prop-type paradigm for react ^16.0 #24

Closed temitope closed 6 years ago

temitope commented 6 years ago

subtle but critical changes for this package to work with newer versions of react-native. should probably update readme about version compatibility moving forward (not sure if its backwards compat)

RealOrangeOne commented 6 years ago

Pinning a dependency like this shouldnt be necessary. What version of react-native are you using?

temitope commented 6 years ago

@RealOrangeOne I'm using react 16.0.0. The changes were introduced with a warning after react 15.5.0 and turned into breaking changes in 16.0.0

RealOrangeOne commented 6 years ago

Yes, but all you've done in this PR is made the version number more specific. I fail to see how that solves the issue of requiring prop-types, unless there it a breaking change in newer versions of it which is causing issues?

temitope commented 6 years ago

@RealOrangeOne Hmmm. So the PropTypes object has been removed from the React package in version 16.0.0 requiring you to get it directly from the prop-types package. Thus I not only updated the json to retrieve that new required package, but also updated the code to no longer reference prop types as React.PropTypes.number but rather PropTypes.number. If you try to use the former, it will break since React no longer holds this object type.

I do not believe I changed any existing version numbers, but simply added a new one (prop-types) or at least that was my intention...maybe I missed something as I forked this quite a while ago.

Does that make sense?

temitope commented 6 years ago

Ahh okay, I see that you had already made these changes, I am sorry, didnt realize that. I had forked this and didn't get the changes you already made. this PR can be entirely ignored in that case. pardon the trouble

temitope commented 6 years ago

@RealOrangeOne when i run yarn or npm I still get the old version of the code where the prop-types is not updated. any ideas?

RealOrangeOne commented 6 years ago

Looks like the code that fixed it hasn't been pushed to NPM. If you need it urgently, you can pull it from GitHub, else i'll try and get it deployed soon!