-
Notifications
You must be signed in to change notification settings - Fork 111
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
React Native 0.61+ support #1542
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick first pass with a few minor comments
ern-core/src/android.ts
Outdated
reactNativeVersion: string | ||
): string | never { | ||
if (semver.gte(reactNativeVersion, '0.60.0')) { | ||
return '245459' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q1: Do you want to return the full version number (i.e. 245459.0.0
instead of just the major version), to keep it consistent with what getDefaultHermesVersion
returns?
Q2: It appears starting with RN 0.61, the jsc-android
dependency is declared as ^245459.0.0
(i.e. including the caret). Do you want to account for that here too?
$ yarn info react-native@0.61.0 dependencies | grep 'jsc-android'
'jsc-android': '^245459.0.0',
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's a good call.
Will update version resolution in another PR dedicated to this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. You mean a separate small PR that just adds the new getDefaultX
methods before this PR right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, just updated the code in this PR as it introduced these methods and it wasn't too much work. Got too lazy to extract the code from this PR to another one ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK cool, and what I also meant in the first comment, is to add an additional conditional branch, to keep it consistent with RN:
$ yarn info react-native@0.60.0 dependencies | grep 'jsc-android'
'jsc-android': '245459.0.0',
$ yarn info react-native@0.61.0 dependencies | grep 'jsc-android'
'jsc-android': '^245459.0.0',
if (semver.gte(reactNativeVersion, '0.60.0')) {
return '245459.0.0'
} else if (semver.gte(reactNativeVersion, '0.61.0')) {
return '^245459.0.0'
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok didn't realized they added caret for 61. Will add it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will revert conditional order statement though otherwise it'll pick first one for 61 ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, yes of course 🤦 😄 👍
Addressed all comments. |
92905d2
to
960dbf6
Compare
Closes #1446 |
awesome! excited to try it out |
This PR adds support for React Native
0.61.x
and0.62.x
versions.The majority of changes are located in iOS container/runner generators to support new cocoapods autolinking mecanism and new XCode workspace structure for container and runner compared to single project. It also contains manifest plugin configuration updates to add pod specific supporting directives. Also a small update of android generator to select the correct default hermes/jsc version based on the version of react native used by the project.
Walking through each commit from top to bottom:
137944f Add new manifest plugin config directives
podFile
: Can only be used in react-native plugin configuration. Pointing to the Podfile to be used for a specific react native version. Basically a copy/paste of the official Podfile of react native only modified with a mustache template placeholder so supportextraPods
directive (to inject additional pods needed for some plugins). But in some override manifest, the Podfile could be heavily modified if desired. This directive just give flexibility over the Podfile to be used.extraPods
: Additional pod statements to inject in the Podfile for the ElectrodeContainer target. This can be used for example, by plugins that are relying on third party dependencies that are available as pods and also need to be injected in the container.podspec
: Pointing to a podspec file. Useful for native modules that do not ship a podspec (and are not maintained anymore or take a while to process PRs to add such a podspec), or native modules for which one would like a different podspec than the one shipped with it.9da9c79 Introduce plugin config based on react native version
See the documentation updated part of this commit for more details.
This is basically done because for react native 0.61.0+, on iOS we don't want the generator to process the current plugin configuration (manually injecting native module projects in the container), but we still want the plugin configuration to be this way for support of version of react native less than 0.61.0 ... So a way to specify multiple configurations, inside the same plugin configuration file, based on the react native version, was needed.
This commit add support of such a feature for both ios and android plugin configuration, which could be useful in some cases, not limited to cocoapods.
1755cf1 Update iOS container generator for RN61+ support
.xconfig
file templatesFor cocoapods on versions of react native >= 0.61.0, we need to include (import) the xcconfig generated in the pods project (created on 'pod install') we also don't need some configuration settings that we were setting only for react native versions < 0.61.0. The update to the config templates takes care of that.
3f117d7 Update iOS runner for RN61+ support
Not many changes here. No changes to generated runner sources.
Main change is to accommodate new runner structure as an XCode workspace (v.s XCode project). Indeed, for React Native >= 0.61.0, in addition to the container XCode project, a Pod XCode project is also generated and both are needed to do a runner build. Therefore, instead of having a runner XCode project pointing to the container project as a dependency, we now have a runner XCode worskpace which include the runner project, the container project and the pods project.
The only Android changes.
Just some code to select the proper default JSC and Hermes engine versions based on the version of react native used (rather than hardcoding these default versions).
To try it
cocoapods
) associated to this PR from my fork.ern platform use 1000.0.0
)cocoapods
branch ofelectrode-native-manifest
(https://github.com/belemaire/electrode-native-manifest/tree/cocoapods)~/.ern/.ernrc
(of course update the path and do not forget to remove afterward). This will basically bypass the remote public master manifest and use the local one instead.ern cauldron repo clear
)ern create-miniapp
. Whenern
creates the miniapp double check in the logs if it picked the right version (0.62.2) which will confirm that your setup is correct and that it is using the local master manifest.ern run-ios
/ern run-android
to launch the miniappFollow up
ios
directory present). This would avoid having to trash existingios
directory or manually creating workspace when upgrading miniapps to RN61+.