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

React Native 0.61+ support #1542

Merged
merged 1 commit into from Apr 27, 2020
Merged

Conversation

belemaire
Copy link
Member

@belemaire belemaire commented Apr 10, 2020

This PR adds support for React Native 0.61.x and 0.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 support extraPods 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

    • Update .xconfig file templates
      For 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.
    • Update ElectrodeContainer xcode schemes to set 'buildImplicitDependencies' to true. This is needed for building the container with the pods, and has no impact on the generation of container not using pods, so we just don't wrap this change in mustache template based on rn version.
    • Heavy logic update in container generator to properly autolink native modules. One thing to note here is that new containers post rn61+ will have a 'node_modules' directory that will be part of the container as it contains react native native code sources as well as the sources of all native modules. That being said, the algorithm will skim the node_modules down to only keep -almost- the minimum needed to support the container build (not to commit 1GB of node_modules).
  • 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.

  • 5aa2aa2 Update Android container generator for RN61+ support

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

  1. Checkout the branch (cocoapods) associated to this PR from my fork.
  2. Make sure you are running dev version of electrode native (ern platform use 1000.0.0)
  3. Locally checkout my cocoapods branch of electrode-native-manifest (https://github.com/belemaire/electrode-native-manifest/tree/cocoapods)
  4. Add the following to your ~/.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.
"manifest": {
  "master": {
    "url": "/local/path/to/electode-native-manifest"
  }
}
  1. Make sure you are not connected to any cauldron (ern cauldron repo clear)
  2. Create a new miniapp with ern create-miniapp. When ern 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.
  3. ern run-ios / ern run-android to launch the miniapp

Follow up

  • I'll look into updating this PR so that the workspace will be auto created for miniapps that already have an ios runner created (i.e ios directory present). This would avoid having to trash existing ios directory or manually creating workspace when upgrading miniapps to RN61+.

Copy link
Member

@friederbluemle friederbluemle left a 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-container-gen-ios/package.json Outdated Show resolved Hide resolved
ern-core/src/android.ts Outdated Show resolved Hide resolved
ern-core/src/android.ts Outdated Show resolved Hide resolved
docs/platform-parts/manifest/native-modules.md Outdated Show resolved Hide resolved
reactNativeVersion: string
): string | never {
if (semver.gte(reactNativeVersion, '0.60.0')) {
return '245459'
Copy link
Member

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',

Copy link
Member Author

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.

Copy link
Member

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?

Copy link
Member Author

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 ;)

Copy link
Member

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'
}

Copy link
Member Author

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 👍

Copy link
Member Author

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 ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

done :)

Copy link
Member

Choose a reason for hiding this comment

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

Oops, yes of course 🤦 😄 👍

ern-runner-gen-ios/src/IosRunnerGenerator.ts Outdated Show resolved Hide resolved
@belemaire
Copy link
Member Author

Addressed all comments.

@belemaire belemaire merged commit 941fd46 into electrode-io:master Apr 27, 2020
@belemaire
Copy link
Member Author

Closes #1446

@davidgovea
Copy link
Contributor

awesome! excited to try it out

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.

None yet

3 participants