Skip to content

Fix error with bundling task eval. when no gradle config is provided#27101

Closed
d4vidi wants to merge 1 commit into
facebook:masterfrom
d4vidi:master
Closed

Fix error with bundling task eval. when no gradle config is provided#27101
d4vidi wants to merge 1 commit into
facebook:masterfrom
d4vidi:master

Conversation

@d4vidi
Copy link
Copy Markdown
Contributor

@d4vidi d4vidi commented Nov 4, 2019

Summary

Fix react.gradle's handling of the case where a configuration isn't explicitly specified by the parent project - e.g. before importing to build.gradle files:

apply plugin: 'com.android.application'

// Nothing to define:
// project.ext.react = [
// ]
apply from: "../../node_modules/react-native/react.gradle"

This is a ridiculously subtle change but it is nevertheless important, as, combined with other groovy quirks, it can result in an overall misbehaviour of the build script.

Source of the bug

Currently, when a react build config isn't specified by the user, RN's react.gradle falls back to [] (i.e. in this line). In groovy, [] stands for an empty array (actually, an empty ArrayList instance). The config, however, is expected to have the nature of a Map.

Effects of the bug

As a bottom line, whenever a configuration isn't specified, the evaluation of the condition as to whether the bundling task in question should be enabled, results in the following build-time exception:

FAILURE: Build failed with an exception.

* Where:
Script '/Users/...../node_modules/react-native/react.gradle' line: 179

* What went wrong:
A problem occurred configuring project ':app'.
> Could not create task ':app:bundleDebugJsAndAssets'.
   > Could not find method enabled() for arguments [[]] on task ':app:bundleDebugJsAndAssets' of type org.gradle.api.tasks.Exec.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 9s

I'm not much of a Groovy person, but while digging in, I've learned that it has the following odd attribute to it:
When accessing a non-existing property of an empty ArrayList in a bean-like fashion (i.e. as array.property1 rather than array.getProperty('property1')), a new empty array is returned. This only holds true for empty arrays, as illustrated by the following snippet:

def emptyArr = []
def arr = [40, 2]

def result1 = (emptyArr.uninitializedProp == null)
println "$result1, ${emptyArr.uninitializedProp}" // ==> false, []"

def result2 = (arr.uninitializedProp == null) // ==> MissingPropertyException
println result2 // Never reached

While this whole scheme could be a bug, it's nonetheless effective in both the latest 2.x.x groovy version and in 2.1.0 (which is the oldest one that seems to be available for download nowadays). The point being is that its a behavior that's sticked.

Note that other evaluations of config properties (e.g. lines 10-19) in the script are not effected by this as they are initially only examined in a boolean form, rather than using a null comparison; "Lucky" for us, empty arrays evaluate to false.

Fix

Simply fallback to a groovy map rather than an array whenever config hasn't been specified. Namely, initialize it to [:], which results in a new HashMap instance, rather than [].

Workaround

Until effectively fixed, can be worked-around by explicitly setting config to an empty map before the react gradle script is imported by the user:

apply plugin: 'com.android.application'
project.ext.react = [:]
apply from: "../../node_modules/react-native/react.gradle"

Changelog

[Android] [Fixed] - Fix 'Could not create task ':app:bundleDebugJsAndAssets'.' error in project with no react config

Test Plan

Mostly regression is required here. I've myself ran this over a project with an empty config and the app has launched successfully in both release and debug flavors.

@facebook-github-bot 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 Nov 4, 2019
@react-native-bot react-native-bot added Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used. Bug Platform: Android Android applications. labels Nov 4, 2019
@d4vidi
Copy link
Copy Markdown
Contributor Author

d4vidi commented Nov 4, 2019

Well CI seems to be broken, not sure what to do about that.

Copy link
Copy Markdown
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Thanks for the fix – and for the very detailed summary in the PR!

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @d4vidi in 5df204b.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications. Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants