[Android] Add support for Build Variants to react.gradle #4686

AndrewJack · 2015-12-09T23:13:33Z

This PR adds support for Android Gradle Build Variants when generating the JS bundle.

Before: only supported "bundleDebugJsAndAssets" and "bundleReleaseJsAndAssets"

Now: all variants are supported
Examples: "bundleDevDebugJsAndAssets", "bundleStageAlphaJsAndAssets", or "bundleBetaJsAndAssets"

The Gradle script will automatically create bundle tasks for each build variant found in a project.

facebook-github-bot · 2015-12-09T23:14:34Z

By analyzing the blame information on this pull request, we identified @foghina, @tdzl2003 and @mkonicek to be potential reviewers.

facebook-github-bot · 2015-12-09T23:14:36Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

facebook-github-bot · 2015-12-09T23:25:31Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

mkonicek · 2015-12-12T15:09:07Z

What is stage alpha?

AndrewJack · 2015-12-14T10:07:41Z

@mkonicek it's an example of a build variant that could be used. "stage" would be the flavor and "alpha" would be the build type.

The script finds all the build variants and creates bundle tasks for them, improves upon the previous hardcoded "bundleDebugJsAndAssets" and "bundleReleaseJsAndAssets" tasks

mkonicek · 2015-12-14T14:11:59Z

Thanks for the explanation but still need more context. Where would you use the output of bundleStageAlphaJsAndAssets and bundleBetaJsAndAssets? Would you upload them to the Play store?

What's the difference between bundleDevDebugJsAndAssets and bundleDebugJsAndAssets?

AndrewJack · 2015-12-14T15:30:09Z

Where would you use the output of bundleStageAlphaJsAndAssets and bundleBetaJsAndAssets? Would you upload them to the Play store?

A: The output could be used for testing or uploaded to a tool such as hockey app for internal dogfooding

What's the difference between bundleDevDebugJsAndAssets and bundleDebugJsAndAssets?

A: The difference here is the Gradle config. The first uses flavors and build types, and the second only uses build types.

The tasks will perform the same action however the use of build variants are useful for Android developers who are dogfooding and developing an app. I have explained in greater detail below.

Build types only
To dogfood my app I may want to install the release, alpha, and debug version at the same time. On Android I can only install one app with the same applicationId. To overcome this I can change the applicationId per build type using the Android Gradle build variants.

The config would look like this:

android {
  buildTypes {
    debug {
      applicationIdSuffix ".debug"
    }
    alpha {
      applicationIdSuffix ".alpha"
    }
    release {
      ...
    }
  }
}

This will generate the tasks:

bundleDebugJsAndAssets
bundleAlphaJsAndAssets
bundleReleaseJsAndAssets

Flavor + Build types
Flavors add on to this functionality. They can be used for different server environments (dev, stage, prod) or free/pro versions of an app.

I may want to install the alpha app in the stage environment and install the debug app in the stage environment. I can only achieve this with this configuration.

The config would look like this:

android {
  productFlavors {
    dev {
      applicationId = "com.example.dev"
    }
    stage {
      applicationId = "com.example.stage"
    }
    prod {
      applicationId = "com.example"
    }
  }

  buildTypes {
    debug {
      applicationIdSuffix ".debug"
    }
    alpha {
      applicationIdSuffix ".alpha"
    }
    release {
      ...
    }
  }
}

This will generate the tasks:

bundleDevDebugJsAndAssets
bundleDevAlphaJsAndAssets
bundleDevReleaseJsAndAssets
bundleStageDebugJsAndAssets
bundleStageAlphaJsAndAssets
bundleStageReleaseJsAndAssets
bundleProdDebugJsAndAssets
bundleProdAlphaJsAndAssets
bundleProdReleaseJsAndAssets

AndrewJack · 2015-12-14T21:53:04Z

local-cli/generator-android/templates/src/app/build.gradle

@@ -18,6 +19,10 @@ apply plugin: "com.android.application"
 *   // whether to bundle JS and assets in debug mode
 *   bundleInDebug: false,
 *
+ *   // whether to bundle JS and assets in another build variant
+ *   // bundleInStageAlpha: true,
+ *   // bundleInBeta: true,


@mkonicek I assume you are referring to this? I could change it to something more generic like bundleIn{variantName}

We should add a bit more info here, like explaining the properties, and may be a link on how to configure build variants,

e.g. -

// whether to bundle JS and assets in a build variant (if you have build variants configured, see <link>) // the configuration property is in the format `bundleIn${productFlavor}${buildType` // bundleInStageAlpha: true, // bundleInBeta: true,

facebook-github-bot · 2015-12-15T19:50:00Z

@AndrewJack updated the pull request.

AndrewJack · 2015-12-15T20:01:27Z

Updated react config comments in build.gradle

mkonicek · 2015-12-18T11:57:44Z

OK thanks for the explanation, get the flavors some. However, I feel like this adds a lot of complexity into the template and the code will have to be maintained. Not everyone might necessarily need these features - no one has been asking about these so far.

Do you think it would make more sense to publish this as a blogpost somewhere and people can easily add it to their build files if needed?

satya164 · 2015-12-18T12:05:13Z

@mkonicek Even if we don't want to merge the build flavours features to the core, I like how the file has been refactored to remove duplicate code. For example the bundleDebugJsAndAssets and bundleReleaseJsAndAssets had almost the same code.

AndrewJack · 2015-12-18T12:09:17Z

Ok, that makes sense, but I agree with @satya164.
Adding the build variants are optional, without adding in extra variants the output is exactly the same as before, but with added flexibility.

AndrewJack · 2015-12-18T14:19:10Z

Maybe a separate React Native Gradle plugin would be useful? That would keep the template simple but allow the plugin to be improved separately.

facebook-github-bot · 2016-01-03T18:33:33Z

@AndrewJack updated the pull request.

mkonicek · 2016-01-06T18:25:17Z

OK I played around with Gradle variants today and finally understand variants and flavors somewhat.

I like how this removes code duplication, keeps the existing behavior and adds extensibility.

@foghina Are you happy with this being merged?

mkonicek · 2016-01-06T18:26:00Z

local-cli/generator-android/templates/src/app/build.gradle

@@ -18,6 +19,12 @@ apply plugin: "com.android.application"
 *   // whether to bundle JS and assets in debug mode
 *   bundleInDebug: false,
 *
+ *   // whether to bundle JS and assets in a build variant (if configured).
+ *   // See https://sites.google.com/a/android.com/tools/tech-docs/new-build-system/user-guide#TOC-Build-Variants


Here's a shorter URL: http://tools.android.com/tech-docs/new-build-system/user-guide#TOC-Build-Variants

mkonicek · 2016-01-06T20:11:15Z

Slightly related PR here: #5160

AndrewJack · 2016-01-07T10:33:46Z

I will make the requested changes today.
When #5160 is merged I'll update to include those changes too.

foghina · 2016-01-07T12:02:57Z

local-cli/generator-android/templates/src/app/react.gradle

+                    name: "$bundleJsAndAssetsTaskName",
+                    type: Exec) {
+                group = "react"
+                description = "bundle Js And assets for ${targetName}."


nit: uppercase S, lowercase A ("bundle JS and...")

foghina · 2016-01-07T12:06:09Z

This is pretty amazing, sorry for taking a long time to review. Thanks for the PR! We should merge #5160 today so you can rebase on top of that and get this merged as well.

facebook-github-bot · 2016-01-07T14:59:13Z

@AndrewJack updated the pull request.

facebook-github-bot · 2016-01-07T20:34:22Z

@AndrewJack updated the pull request.

facebook-github-bot · 2016-01-07T20:36:18Z

@AndrewJack updated the pull request.

mkonicek · 2016-01-07T20:36:47Z

Thanks for rebasing this so quickly! I see you're making updates, let me know when we should merge :)

facebook-github-bot · 2016-01-07T22:23:08Z

@AndrewJack updated the pull request.

facebook-github-bot · 2016-01-07T22:24:24Z

@AndrewJack updated the pull request.

AndrewJack · 2016-01-07T22:24:30Z

@mkonicek All done, just had to test it.

satya164 · 2016-01-08T11:08:57Z

local-cli/generator-android/templates/src/app/react.gradle

+                    commandLine "cmd", "/c", "react-native", "bundle", "--platform", "android", "--dev", "true", "--entry-file",
+                            entryFile, "--bundle-output", jsBundleFile, "--assets-dest", resourcesDir
+                } else {
+                    commandLine "react-native", "bundle", "--platform", "android", "--dev", "true", "--entry-file",


Shouldn't --dev be false in release build?

How to we want to handle this?

Another config value

if release then --dev false

I think if release then dev false is sensible.

combination of both isn't too hard

@AndrewJack dunno. can't think of a scenario where someone will want production build to have dev warnings. Also if someone does, he can always manually change it.

@satya164 I would use it for a tester build, to get more logs out of a build.
Its easy to add if someone needs it, so I will just add the "if release option".

@AndrewJack sound good.

facebook-github-bot · 2016-01-08T12:15:17Z

@AndrewJack updated the pull request.

AndrewJack · 2016-01-08T12:17:03Z

local-cli/generator-android/templates/src/app/react.gradle

+                workingDir reactRoot
+
+                // Set up dev mode
+                def devEnabled = !targetName.toLowerCase().contains("release")


dev mode can be made configurable like this:

def devEnabled = config."devMode${targetName}" ?: !targetName.toLowerCase().contains("release")

foghina · 2016-01-08T18:59:49Z

Sick PR!

@facebook-github-bot shipit

facebook-github-bot · 2016-01-08T19:01:54Z

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1655573684725605/int_phab to review.

Summary: This PR adds support for Android Gradle [Build Variants](https://sites.google.com/a/android.com/tools/tech-docs/new-build-system/user-guide#TOC-Build-Variants) when generating the JS bundle. **Before**: only supported "bundleDebugJsAndAssets" and "bundleReleaseJsAndAssets" **Now**: all variants are supported Examples: "bundleDevDebugJsAndAssets", "bundleStageAlphaJsAndAssets", or "bundleBetaJsAndAssets" The Gradle script will automatically create bundle tasks for each build variant found in a project. Closes facebook#4686 Reviewed By: svcscm Differential Revision: D2815856 Pulled By: foghina fb-gh-sync-id: 4518de70d178205bc3e5044d2446b56c40298da2

Summary: This PR adds support for Android Gradle [Build Variants](https://sites.google.com/a/android.com/tools/tech-docs/new-build-system/user-guide#TOC-Build-Variants) when generating the JS bundle. **Before**: only supported "bundleDebugJsAndAssets" and "bundleReleaseJsAndAssets" **Now**: all variants are supported Examples: "bundleDevDebugJsAndAssets", "bundleStageAlphaJsAndAssets", or "bundleBetaJsAndAssets" The Gradle script will automatically create bundle tasks for each build variant found in a project. Closes facebook/react-native#4686 Reviewed By: svcscm Differential Revision: D2815856 Pulled By: foghina fb-gh-sync-id: 4518de70d178205bc3e5044d2446b56c40298da2

facebook-github-bot added the GH Review: review-needed label Dec 9, 2015

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 9, 2015

skevy added Android labels Dec 12, 2015

AndrewJack reviewed Dec 14, 2015
View reviewed changes

astreet assigned foghina Jan 5, 2016

mkonicek reviewed Jan 6, 2016
View reviewed changes

foghina reviewed Jan 7, 2016
View reviewed changes

satya164 reviewed Jan 8, 2016
View reviewed changes

satya164 added GH Review: needs-revision and removed GH Review: review-needed labels Jan 8, 2016

Add support for Android Gradle Build Variants to react.gradle

ad4aa41

AndrewJack reviewed Jan 8, 2016
View reviewed changes

foghina added GH Review: accepted and removed GH Review: needs-revision labels Jan 8, 2016

ghost closed this in bd7d10e Jan 8, 2016

This pull request was closed.

[Android] Add support for Build Variants to react.gradle #4686

[Android] Add support for Build Variants to react.gradle #4686

Conversation

AndrewJack commented Dec 9, 2015

facebook-github-bot commented Dec 9, 2015

facebook-github-bot commented Dec 9, 2015

facebook-github-bot commented Dec 9, 2015

mkonicek commented Dec 12, 2015

AndrewJack commented Dec 14, 2015

mkonicek commented Dec 14, 2015

AndrewJack commented Dec 14, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Dec 15, 2015

AndrewJack commented Dec 15, 2015

mkonicek commented Dec 18, 2015

satya164 commented Dec 18, 2015

AndrewJack commented Dec 18, 2015

AndrewJack commented Dec 18, 2015

facebook-github-bot commented Jan 3, 2016

mkonicek commented Jan 6, 2016

Choose a reason for hiding this comment

mkonicek commented Jan 6, 2016

AndrewJack commented Jan 7, 2016

Choose a reason for hiding this comment

foghina commented Jan 7, 2016

facebook-github-bot commented Jan 7, 2016

facebook-github-bot commented Jan 7, 2016

facebook-github-bot commented Jan 7, 2016

mkonicek commented Jan 7, 2016

facebook-github-bot commented Jan 7, 2016

facebook-github-bot commented Jan 7, 2016

AndrewJack commented Jan 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Jan 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foghina commented Jan 8, 2016

facebook-github-bot commented Jan 8, 2016