Skip to content

Commit 5f14088

Browse files
Support additional dependency declaration format
Summary: Thanks for submitting a PR! Please read these instructions carefully: - [x] Explain the **motivation** for making this change. - [x] Provide a **test plan** demonstrating that the code is solid. - [x] Match the **code formatting** of the rest of the codebase. - [x] Target the `master` branch, NOT a "stable" branch. Previously, `isInstalled` was somewhat naively checking for the presence of a string in the `build.gradle` file to determine whether or not that dependency was already linked. I.e.: ``` compile project(':${name}')\n ``` …where `name` is replaced with the name of the dependency being checked. This was inflexible as it only supported that particular format of `compile` definition. Another, valid `compile` definition follows: ``` compile(project(':example') { … } ``` However, this failed the check because it didn't _exactly_ match the format for which the check was searching the `build.gradle` contents. As a result, running `react-native link` would incorrectly duplicate the dependency definition and thus cause a crash upon launching the app. This change adds an `installPattern` to the object returned from `makeBuildPatch`, which includes the particular dependency name and is valid for both `compile` definition formats. This commit adds an additional compile definition in the associated fixture, an additional test case in `isInstalled.spec.js` to check for this additional format, and an additional test in `makeBuildPatch.spec.js` to ensure the object returned includes the aforementioned `installPattern` Regex pattern. Sign the [CLA][2], if you haven't already. ✅ Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it. ✅ Make sure all **tests pass** on both [Travis][3] and [Circle CI][4]. PRs that break tests are unlikely to be merged. For more info, see the ["Pull Requests"][5] section of our "Contributing" guidelines. [1]: https://medium.com/martinkonicek/what-is-a-test-plan-8bfc840ec171#.y9lcuqqi9 [2]: https://code.facebook.com/cla [3]: https://travis-ci.org/facebook/react-native [4]: http://circleci.com/gh/facebook/react-native [5]: https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#pull-requests Closes facebook#14475 Differential Revision: D5398552 Pulled By: hramos fbshipit-source-id: 1eaf84ba5bcfc43202f13c6b8fcfc68c30f36c33
1 parent 51e258e commit 5f14088

File tree

5 files changed

+21
-7
lines changed

5 files changed

+21
-7
lines changed

local-cli/link/__fixtures__/android/patchedBuild.gradle

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
dependencies {
22
compile project(':test')
3+
compile(project(':test2')) {
4+
exclude(group: 'org.unwanted', module: 'test10')
5+
}
36
compile fileTree(dir: "libs", include: ["*.jar"])
47
compile "com.android.support:appcompat-v7:23.0.1"
58
compile "com.facebook.react:react-native:0.18.+"

local-cli/link/__tests__/android/isInstalled.spec.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ const projectConfig = {
88
};
99

1010
describe('android::isInstalled', () => {
11-
it('should return true when project is already in build.gradle', () =>
11+
it('should return true when project is already in build.gradle', () => {
1212
expect(isInstalled(projectConfig, 'test')).toBeTruthy()
13-
);
13+
expect(isInstalled(projectConfig, 'test2')).toBeTruthy()
14+
});
1415

1516
it('should return false when project is not in build.gradle', () =>
16-
expect(isInstalled(projectConfig, 'test2')).toBeFalsy()
17+
expect(isInstalled(projectConfig, 'test3')).toBeFalsy()
1718
);
1819
});

local-cli/link/__tests__/android/makeBuildPatch.spec.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,10 @@ describe('makeBuildPatch', () => {
1313
const {patch} = makeBuildPatch(name);
1414
expect(patch).toBe(` compile project(':${name}')\n`);
1515
});
16+
17+
it('should make a correct install check pattern', () => {
18+
const {installPattern} = makeBuildPatch(name);
19+
const match = `/\\s{4}(compile)(\\(|\\s)(project)\\(\\':${name}\\'\\)(\\)|\\s)/`;
20+
expect(installPattern.toString()).toBe(match);
21+
});
1622
});

local-cli/link/android/isInstalled.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ const fs = require('fs');
22
const makeBuildPatch = require('./patches/makeBuildPatch');
33

44
module.exports = function isInstalled(config, name) {
5-
return fs
6-
.readFileSync(config.buildGradlePath)
7-
.indexOf(makeBuildPatch(name).patch) > -1;
5+
const buildGradle = fs.readFileSync(config.buildGradlePath);
6+
return makeBuildPatch(name).installPattern.test(buildGradle);
87
};
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
module.exports = function makeBuildPatch(name) {
2+
const installPattern = new RegExp(
3+
`\\s{4}(compile)(\\(|\\s)(project)\\(\\\':${name}\\\'\\)(\\)|\\s)`
4+
)
5+
26
return {
7+
installPattern,
38
pattern: /[^ \t]dependencies {\n/,
4-
patch: ` compile project(':${name}')\n`,
9+
patch: ` compile project(':${name}')\n`
510
};
611
};

0 commit comments

Comments
 (0)