Skip to content

Commit

Permalink
Uses a single code path to link and unlink all platforms
Browse files Browse the repository at this point in the history
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes #17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
  • Loading branch information
rozele authored and facebook-github-bot committed Feb 13, 2018
1 parent 6893a26 commit 1673c57
Show file tree
Hide file tree
Showing 13 changed files with 97 additions and 154 deletions.
2 changes: 2 additions & 0 deletions local-cli/core/android/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,5 @@ exports.dependencyConfig = function dependencyConfigAndroid(folder, userConfig)

return { sourceDir, folder, manifest, packageImportPath, packageInstance };
};

exports.linkConfig = require('../../link/android');
2 changes: 2 additions & 0 deletions local-cli/core/ios/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,5 @@ exports.projectConfig = function projectConfigIOS(folder, userConfig) {
};

exports.dependencyConfig = exports.projectConfig;

exports.linkConfig = require('../../link/ios');
10 changes: 7 additions & 3 deletions local-cli/link/__tests__/link.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,10 @@ describe('link', () => {
it('should register native module when android/ios projects are present', (done) => {
const registerNativeModule = sinon.stub();
const dependencyConfig = {android: {}, ios: {}, assets: [], commands: {}};
const androidLinkConfig = require('../android');
const iosLinkConfig = require('../ios');
const config = {
getPlatformConfig: () => ({ios: {}, android: {}}),
getPlatformConfig: () => ({ios: { linkConfig: iosLinkConfig }, android: { linkConfig: androidLinkConfig }}),
getProjectConfig: () => ({android: {}, ios: {}, assets: []}),
getDependencyConfig: sinon.stub().returns(dependencyConfig),
};
Expand Down Expand Up @@ -223,8 +225,9 @@ describe('link', () => {
sinon.stub().returns(false)
);

const linkConfig = require('../ios');
const config = {
getPlatformConfig: () => ({ ios: {}}),
getPlatformConfig: () => ({ ios: { linkConfig: linkConfig }}),
getProjectConfig: () => ({ ios: {}, assets: [] }),
getDependencyConfig: sinon.stub().returns({
ios: {}, assets: [], commands: { prelink, postlink },
Expand All @@ -251,8 +254,9 @@ describe('link', () => {
copyAssets
);

const linkConfig = require('../ios');
const config = {
getPlatformConfig: () => ({ ios: {} }),
getPlatformConfig: () => ({ ios: { linkConfig: linkConfig } }),
getProjectConfig: () => ({ ios: {}, assets: projectAssets }),
getDependencyConfig: sinon.stub().returns(dependencyConfig),
};
Expand Down
4 changes: 2 additions & 2 deletions local-cli/link/android/copyAssets.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ const groupFilesByType = require('../groupFilesByType');
* For now, the only types of files that are handled are:
* - Fonts (otf, ttf) - copied to targetPath/fonts under original name
*/
module.exports = function copyAssetsAndroid(files, targetPath) {
module.exports = function copyAssetsAndroid(files, project) {
const assets = groupFilesByType(files);

(assets.font || []).forEach(asset =>
fs.copySync(asset, path.join(targetPath, 'fonts', path.basename(asset)))
fs.copySync(asset, path.join(project.assetsPath, 'fonts', path.basename(asset)))
);
};
9 changes: 9 additions & 0 deletions local-cli/link/android/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module.exports = function() {
return {
isInstalled: require('./isInstalled'),
register: require('./registerNativeModule'),
unregister: require('./unregisterNativeModule'),
copyAssets: require('./copyAssets'),
unlinkAssets: require('./unlinkAssets')
};
};
4 changes: 2 additions & 2 deletions local-cli/link/android/unlinkAssets.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ const groupFilesByType = require('../groupFilesByType');
* For now, the only types of files that are handled are:
* - Fonts (otf, ttf) - copied to targetPath/fonts under original name
*/
module.exports = function unlinkAssetsAndroid(files, targetPath) {
module.exports = function unlinkAssetsAndroid(files, project) {
const assets = groupFilesByType(files);

(assets.font || []).forEach((file) => {
const filePath = path.join(targetPath, 'fonts', path.basename(file));
const filePath = path.join(project.assetsPath, 'fonts', path.basename(file));
if (fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
}
Expand Down
6 changes: 6 additions & 0 deletions local-cli/link/ios/common/isInstalled.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const isInstalledIOS = require('../isInstalled');
const isInstalledPods = require('../../pods/isInstalled');

module.exports = function isInstalled(config, name) {
return isInstalledIOS(config, name) || isInstalledPods(config, name);
};
16 changes: 16 additions & 0 deletions local-cli/link/ios/common/registerNativeModule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const registerDependencyIOS = require('../registerNativeModule');
const registerDependencyPods = require('../../pods/registerNativeModule');

module.exports = function registerNativeModule(
name,
dependencyConfig,
params,
projectConfig
) {
if (projectConfig.podfile && dependencyConfig.podspec) {
registerDependencyPods(name, dependencyConfig, projectConfig);
}
else {
registerDependencyIOS(dependencyConfig, projectConfig);
}
};
22 changes: 22 additions & 0 deletions local-cli/link/ios/common/unregisterNativeModule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const compact = require('lodash').compact;
const isInstalledIOS = require('../isInstalled');
const isInstalledPods = require('../../pods/isInstalled');
const unregisterDependencyIOS = require('../registerNativeModule');
const unregisterDependencyPods = require('../../pods/registerNativeModule');

module.exports = function unregisterNativeModule(
name,
dependencyConfig,
projectConfig,
otherDependencies
) {
const isIosInstalled = isInstalledIOS(projectConfig, dependencyConfig);
const isPodInstalled = isInstalledPods(projectConfig, dependencyConfig);
if (isIosInstalled) {
const iOSDependencies = compact(otherDependencies.map(d => d.config.ios));
unregisterDependencyIOS(dependencyConfig, projectConfig, iOSDependencies);
}
else if (isPodInstalled) {
unregisterDependencyPods(dependencyConfig, projectConfig);
}
};
9 changes: 9 additions & 0 deletions local-cli/link/ios/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module.exports = function() {
return {
isInstalled: require('./common/isInstalled'),
register: require('./common/registerNativeModule'),
unregister: require('./common/unregisterNativeModule'),
copyAssets: require('./copyAssets'),
unlinkAssets: require('./unlinkAssets')
};
};
76 changes: 2 additions & 74 deletions local-cli/link/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,6 @@ const chalk = require('chalk');
* run Flow. */
const isEmpty = require('lodash').isEmpty;
const promiseWaterfall = require('./promiseWaterfall');
const registerDependencyAndroid = require('./android/registerNativeModule');
const registerDependencyIOS = require('./ios/registerNativeModule');
const registerDependencyPods = require('./pods/registerNativeModule');
const isInstalledAndroid = require('./android/isInstalled');
const isInstalledIOS = require('./ios/isInstalled');
const isInstalledPods = require('./pods/isInstalled');
const copyAssetsAndroid = require('./android/copyAssets');
const copyAssetsIOS = require('./ios/copyAssets');
const getProjectDependencies = require('./getProjectDependencies');
const getDependencyConfig = require('./getDependencyConfig');
const pollParams = require('./pollParams');
Expand All @@ -47,37 +39,8 @@ log.heading = 'rnpm-link';

const dedupeAssets = (assets) => uniqBy(assets, asset => path.basename(asset));


const linkDependencyAndroid = (androidProject, dependency) => {
if (!androidProject || !dependency.config.android) {
return null;
}

const isInstalled = isInstalledAndroid(androidProject, dependency.name);

if (isInstalled) {
log.info(chalk.grey(`Android module ${dependency.name} is already linked`));
return null;
}

return pollParams(dependency.config.params).then(params => {
log.info(`Linking ${dependency.name} android dependency`);

registerDependencyAndroid(
dependency.name,
dependency.config.android,
params,
androidProject
);

log.info(`Android module ${dependency.name} has been successfully linked`);
});
};

const linkDependencyPlatforms = (platforms, project, dependency) => {
const ignorePlatforms = ['android', 'ios'];
const linkDependency = (platforms, project, dependency) => {
Object.keys(platforms || {})
.filter(platform => ignorePlatforms.indexOf(platform) < 0)
.forEach(platform => {
if (!project[platform] || !dependency.config[platform]) {
return null;
Expand Down Expand Up @@ -110,45 +73,12 @@ const linkDependencyPlatforms = (platforms, project, dependency) => {
});
};

const linkDependencyIOS = (iOSProject, dependency) => {
if (!iOSProject || !dependency.config.ios) {
return;
}

const isInstalled = isInstalledIOS(iOSProject, dependency.config.ios) || isInstalledPods(iOSProject, dependency.config.ios);
if (isInstalled) {
log.info(chalk.grey(`iOS module ${dependency.name} is already linked`));
return;
}

log.info(`Linking ${dependency.name} ios dependency`);
if (iOSProject.podfile && dependency.config.ios.podspec) {
registerDependencyPods(dependency, iOSProject);
}
else {
registerDependencyIOS(dependency.config.ios, iOSProject);
}
log.info(`iOS module ${dependency.name} has been successfully linked`);
};

const linkAssets = (platforms, project, assets) => {
if (isEmpty(assets)) {
return;
}

if (project.ios) {
log.info('Linking assets to ios project');
copyAssetsIOS(assets, project.ios);
}

if (project.android) {
log.info('Linking assets to android project');
copyAssetsAndroid(assets, project.android.assetsPath);
}

const ignorePlatforms = ['android', 'ios'];
Object.keys(platforms || {})
.filter(platform => ignorePlatforms.indexOf(platform) < 0)
.forEach(platform => {
const linkConfig = platforms[platform] && platforms[platform].linkConfig && platforms[platform].linkConfig();
if (!linkConfig || !linkConfig.copyAssets) {
Expand Down Expand Up @@ -212,9 +142,7 @@ function link(args: Array<string>, config: RNConfig) {

const tasks = flatten(dependencies.map(dependency => [
() => promisify(dependency.config.commands.prelink || commandStub),
() => linkDependencyAndroid(project.android, dependency),
() => linkDependencyIOS(project.ios, dependency),
() => linkDependencyPlatforms(platforms, project, dependency),
() => linkDependency(platforms, project, dependency),
() => promisify(dependency.config.commands.postlink || commandStub),
]));

Expand Down
4 changes: 2 additions & 2 deletions local-cli/link/pods/registerNativeModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ const findMarkedLinesInPodfile = require('./findMarkedLinesInPodfile');
const addPodEntry = require('./addPodEntry');
const savePodFile = require('./savePodFile');

module.exports = function registerNativeModulePods(dependency, iOSProject) {
module.exports = function registerNativeModulePods(name, dependencyConfig, iOSProject) {
const podLines = readPodfile(iOSProject.podfile);
const linesToAddEntry = getLinesToAddEntry(podLines, iOSProject);
addPodEntry(podLines, linesToAddEntry, dependency.config.ios.podspec, dependency.name);
addPodEntry(podLines, linesToAddEntry, dependencyConfig.podspec, name);
savePodFile(iOSProject.podfile, podLines);
};

Expand Down
Loading

0 comments on commit 1673c57

Please sign in to comment.