Upgrade React Native peer dependency to 0.54. #2115
Conversation
The Podfile is updated in line with the required React Native dependency changes.
This commit also makes the following changes to how Ruby gems are installed:
1. Remove the bundler "binstubs" which were previously in `examples/ios`.
Binstubs exist so that Ruby "binaries" can be executed without using
`bundle exec`. See here for an explanation:
https://github.com/rbenv/rbenv/wiki/Understanding-binstubs#project-specific-binstubs
The only place bundles are used currently is in the `build:ios` script in
`package.json`. This already uses `bundle exec`, so the binstubs were
actually never used.
2. Install the Rubygem bundles locally in `example/ios/bundles`.
`bundler` previously installed the dependencies in the global gems directory.
If you are using the default macOS Ruby installation, this requires `sudo`.
Since we already install node_modules & pods within the repo, I believe it
makes sense to also install these locally.
3. Update the `Gemfile.lock`.
The current `Gemfile.lock` specified an outdated version of Cocoapods.
This throws a warning on the current example Podfile.
Cocoapods is now updated to version 1.4.0.
|
Any ETA on merging this? |
|
@wbattel4607 did you test by any chance this PR? |
|
@rborn I haven't had any issues with it yet. |
|
@wbattel4607 @lachenmayer the only issue I see here is backward compat for RN < 0.54 @alvelig thoughts on this? |
|
The README suggests that this repo will only support the latest version of React Native- though I understand that doesn't mean pulling the plug on everyone else is the right thing to do. Perhaps it could get bundled with a new minor release version, where those on RN < 0.54 can stick to 0.20.x and you can publish any other changes as patch versions on 0.20. Speaking of semver, when will the first major version be? Do you have any plans for a 1.0.0 release? I think the lib is ready for it but I don't know if the collaborators have any further plans before then. If you were to release version 1.0.0 as the first major version, you could easily just say that version 1.X.X required RN >= 0.54 and have 0.X.X remain compatible with RN < 0.54. You can still publish more minor and patch versions to 0.X.X as fixes, optimizations, etc, and just go forward with 1.X.X for new features, etc.... This is what I would do. |
|
Hi @rborn, thanks a lot for checking this out! :) |
|
@alvelig 🐽 |
| <uses-permission android:name="android.permission.INTERNET" /> | ||
| <uses-permission android:name="android.permission.ACCESS_COURSE_LOCATION"/> | ||
| <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" /> | ||
|
|
||
| <application | ||
| android:allowBackup="true" | ||
| android:allowBackup="false" |
There was a problem hiding this comment.
I don't know - this change was made in RN 0.54 ncuillery/rn-diff@rn-0.53.3...rn-0.54.0
There was a problem hiding this comment.
Ok, It's not a problem for example
There was a problem hiding this comment.
That's just some code cleanups from RN team, nothing serious.
|
LGTM |
Does any other open PR do the same thing?
No
What issue is this PR fixing?
This library has some regressions when used with React Native 0.54 - we were using the project with 0.54, and were facing #2100 for example.
Further, I was trying set up some iOS integration tests using the built-in RCTTest library, and I was not able to set it up using RN 0.51. (It works fine on 0.54)
It would be great if we could upgrade the RN version so that this work can continue. I feel that native<->JS integration tests (& possibly also native snapshot tests) will make it much easier to maintain backwards compatibility & avoid serious regressions like the current one we're facing.
How did you test this PR?
yarn/npm installyarn build:ios/npm run build:iosexample/ios/AirMapsExplorer.xcworkspace(make sure it's the workspace, not the project)I have not tested this change with Android, as I currently don't have Android Studio installed. I have updated the Android manifest file as per rn-diff, so in theory this should build properly. I would appreciate any help with that - a simple
npm install && npm run:androidshould do the trick.Known issues
It is trivial to trigger GoogleMaps iOS: initialRegion latitudeDelta/longitudeDelta not respected #2100 now - just open the example app, switch to Google Maps and open any of the examples. Most examples should be zoomed into SF, but instead you see all of North America. If this PR is going to be merged, I would merge GoogleMaps iOS: initialRegion latitudeDelta/longitudeDelta not respected #2100 along with it.
The Podfile currently does not work if you are using an old version of RN. This shouldn't be too difficult to fix, I will update this PR with a fix & documentation. The old Podfile was relying on the deprecated
BatchedBridge, I think it should be possible to useCxxBridgein any RN versions we care about.Changes to Bundler
This PR also makes the following changes to how Ruby gems are installed:
Remove the bundler "binstubs" which were previously in
examples/ios.Binstubs exist so that Ruby "binaries" can be executed without using
bundle exec. See here for an explanation.The only place bundles are used currently is in the
build:iosscript inpackage.json. This already usesbundle exec, so the binstubs were actually never used.Install the Rubygem bundles locally in
example/ios/bundles.bundlerpreviously installed the dependencies in the global gems directory. If you are using the default macOS Ruby installation, this requiressudo. Since we already install node_modules & pods within the repo, I believe it makes sense to also install these locally.Update the
Gemfile.lock.The current
Gemfile.lockspecified an outdated version of Cocoapods. This throws a warning on the current example Podfile. Cocoapods is now updated to version 1.4.0.