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 obsolete React podspec dependency #382

Merged
merged 1 commit into from
Mar 26, 2018
Merged

Conversation

lebedev
Copy link
Contributor

@lebedev lebedev commented Mar 26, 2018

I'm using Sentry with a React Native-only app.

react-native link react-native-sentry adds RNSentry as a pod. If I include React as a pod in Podfile, all is ok. But I explicitly do not include React as a pod in my React Native-only app, because it has many downsides and using-react-as-a-pod-issues. In this case Sentry-pod forces CocoaPods to satisfy this (check the changes of this PR) React-dependency by installing old/obsolete/deprecated React 0.11, which breaks even more things.

Resolution is to remove this dependency.

Check corresponding issues in another repos:

invertase/react-native-firebase#325
invertase/react-native-firebase#324

ocetnik/react-native-background-timer#71

@lebedev lebedev requested a review from HazAT as a code owner March 26, 2018 11:11
@HazAT
Copy link
Member

HazAT commented Mar 26, 2018

Thx :)

@HazAT
Copy link
Member

HazAT commented May 16, 2018

@angly-cat Can please elaborate a bit more what's going on here?

Firebase seems to never have merged this and people now run into new issues see:
#344
#395

I need a "fix" on how to set this up without taking any manual steps otherwise I will revert this since it seems it causes more harm than good.

@lebedev
Copy link
Contributor Author

lebedev commented May 16, 2018

Hi! I'm sorry to hear that.

Let me run some detailed tests first and then I would be able to tell you if my premise was wrong after all and if this PR should be reverted, OK?

@HazAT
Copy link
Member

HazAT commented May 16, 2018

@angly-cat OK, thanks for helping with this.

@AndrewJack
Copy link

@HazAT I've just reverted this locally while updating to cocoapods v1.5.2 and it now works after experiencing the same issue as #395.

@@ -17,7 +17,6 @@ Pod::Spec.new do |s|

s.preserve_paths = '*.js'

s.dependency 'React'
Copy link

@AndrewJack AndrewJack May 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If/when you revert. This can be more precise by changing it to:

s.dependency 'React/Core'

Copy link
Member

@HazAT HazAT May 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, but I will just revert it back, other plugins also just have s.dependency 'React'

@lebedev
Copy link
Contributor Author

lebedev commented May 16, 2018

@HazAT Yes, apparently this was a mistake from my side. I'm sorry for that. Please revert this PR.

And it seems like it's not possible to use pod version of react-native-sentry without explicitly including React Native as a pod too.

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.

3 participants