-
Notifications
You must be signed in to change notification settings - Fork 792
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 cocoa pod 0.60.0 for rn-fetch-blob #397
Conversation
I think this can be merged |
pls merge and do a version :) |
waiting for merge thanks 🙏 |
Got no clue of this stuff; just noticed that a similar fix references |
The correct fix should be React-Core
Op wo 10 jul. 2019 12:36 schreef geisterfurz007 <[email protected]>:
… Got no clue of this stuff; just noticed that a similar fix references
React-Core instead of React: ***@***.***
<Soni96pl/react-native-sound@c803452>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#397?email_source=notifications&email_token=AAJR2N6VAIDYOX3YSQEADGDP6W3S3A5CNFSM4H7BL4B2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZTBXYA#issuecomment-510008288>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJR2NZJFYYAXWIAFJU4OKLP6W3S3ANCNFSM4H7BL4BQ>
.
|
|
Hello @Traviskn or any maintainer can check & merge some PR requests like cocoa pod warning, storage warning, provider conflict in AndroidManifest file. |
@Traviskn Hello, can we merge this to support react-native @0.60.0 asap please? |
A lot of work has already been done toward this: - adjusting to the newer Flow version (zulip#3827) - updating to AndroidX (zulip#3852) - migrating to using CocoaPods at all (zulip#3987) Also, do what needs to be done atomically with the upgrade. ----- Platform-agnostic -------------------------------------------- After the upgrade, we get this peer dep warning: warning " > [email protected]" has incorrect peer dependency "react-native@>=0.57 <0.60". `[email protected]` is the minimum supported with RN v0.60.0. But if we try to get `[email protected] before the upgrade, we get this warning: warning " > [email protected]" has incorrect peer dependency "react-native@>=0.60 <0.62". So, handle `react-native-webview` in the same commit as `react-native`. ----- iOS ---------------------------------------------------------- As far as I've seen, facebook/react-native@2321b3fd7 is the only cause of iOS changes that must happen atomically with the RN v0.60.0 upgrade. In that commit, all the "subspecs" that were nested under the "React" pod are un-nested so they each become their own pod, with many of these living in their own directory under node_modules/react-native/Libraries [0]. In our own code, this means changing our Podfile to refer to all these new pods, instead of the "React" pod's subspecs. So, do. Our Podfile has a list of "Pods we need that depend on React Native". We must also be sure all the dependencies in this list adapt to facebook/react-native@2321b3fd7. The syntax for pointing explicitly to a subspec is a slash, as in "React/Core" [1]. Knowing this, we check that list for pods that explicitly depended on those subspecs, with "React/[...]": grep -Rl dependency.*React/ --include=\*.podspec --exclude="node_modules/react-native/*" node_modules There are two, and here's how they handled the change in new versions that we take in this commit: - `zmxv/react-native-sound@2f2c25a69`: "React/Core" -> "React" - `joltup/rn-fetch-blob`, `01f10cb10^..0f6c3e3cc`: "React/Core" -> "React-Core" So, those respond to *explicit* dependencies on a subspec. There are quite a few that depended on the "React" pod before, and they still depend on the "React" pod now. So, let's find out if any of these might have been *implicitly* depending on a subspec. For that, first note that React Native was a special case before the upgrade: the React podspec declared a `default_subspec` [1] [2] of "Core". From reading the docs, that seems to mean that a dependency on the "Core" subspec, specifically, could have been spelled in one of two ways: "React/Core", or just "React" [3]. From reading the doc on `default_subspec`, it seems that this flexibility did not apply to *all* of the subspecs, though it would have if `default_subspec` were absent. So, the only remaining worry is that a "React/Core" / "React-Core" dependency can no longer be spelled as "React" after the upgrade. If so, it's possible that some of these "React"-dependent pods were secretly depending on "React/Core" before, and will need to get an `s.dependency "React-Core"` line added to them now. I don't think we have to worry about this. Here's one theory: "React.podspec", after `facebook/react-native@2321b3fd7`, newly declares `s.dependency "React-Core"` [4]. I suspect this accomplishes much the same thing as having "Core" as a `default_subspec`, as far as we're concerned here: namely, that if you want "React-Core", you can still just say "React". I wish it were clear in the CocoaPods docs, or that React Native had cleared this up in the RN v0.60 release notes and said it's fine to just depend on "React" when you want "React-Core". The `zmxv/react-native-sound@2f2c25a69` story (going from "React/Core" to just "React") may support the conclusion even more strongly, in that the "React/Core" specificity may have been intentional (there's very little to say either way, though), and there was demonstrably no harm in losing it. `rn-fetch-blob` showed less certainty about handling the upgrade. In `01f10cb10`, they chose "React". Then, in the same PR [5], they moved to "React-Core" in 0f6c3e3cc. That PR is a little ambiguous to read, but to me, it looks like things were working fine with "React", and they moved to "React-Core" more or less at random. To be sure, test for ourselves. Before and after making this change (and clearing `ios/Pods` and running `pod install`): node_modules/rn-fetch-blob/rn-fetch-blob.podspec ```diff - s.dependency 'React-Core' + s.dependency 'React' ``` there are no build failures, and I can still log `NativeModules` from `react-native` and see `RNFetchBlob` there. I also see `RNSound`. So, it looks like we've done what we need to for facebook/react-native@2321b3fd7. [0]: They do still live in node_modules. It's possible for pods to be hosted online and downloaded on `pod install`, npm-style. For an example, see the 'Toast' dependency in node_modules/react-native-simple-toast/react-native-simple-toast.podspec. But that's not what's happening here, yet. facebook/react-native@2321b3fd7 hints that this will be the way of the future for React Native, "to make the experience nicer for library consumers". [1]: https://guides.cocoapods.org/syntax/podspec.html#subspec [2]: https://guides.cocoapods.org/syntax/podspec.html#default_subspecs [3]: One blog post has an example of this; see http://www.dbotha.com/2014/12/04/optional-cocoapod-dependencies/#default-subspec. [4]: It also newly declares a lot of those formerly-subspec dependencies, like "React-RCTImage", etc. I don't see why this would have been strictly necessary, and it means we newly can't avoid linking these things (which we could before, since only "Core" was listed as a `default_subspec`) and potentially enlarging the iOS app. Ah well, not a big problem. [5]: joltup/rn-fetch-blob#397
Not working in
|
Same Problem here on react native 0.63.3
|
Dear @AppPilots |
Same problem here, can somebody do something? |
worked for me thanks, run |
The above code where i need to add |
in node_modules/react-native-fetch-blob/react-native-fetch-blob.podspec |
worked like a charm |
Please check again & merge it :)
Thanks,
Cong