feat: refactor install to use link directly instead of spawning it (r…

…eact-native-community#180)

Summary:
---------

Attempt No. 2 after react-native-community#178. Turned out I passed empty array as `platforms` argument to link, while I should pass `undefined`. We could actually add `--platforms` support to `install`/`uninstall` now, what do you think?

Test Plan:
----------

Uninstalling:

<img width="677" alt="screenshot 2019-02-19 at 13 42 54" src="https://user-images.githubusercontent.com/5106466/53015940-8ed51980-344c-11e9-9f22-0406c468a10d.png">

Installing: 

<img width="668" alt="screenshot 2019-02-19 at 13 43 02" src="https://user-images.githubusercontent.com/5106466/53015941-8ed51980-344c-11e9-8556-a6350f05536f.png">

README.md

@@ -35,7 +35,7 @@ node ./node_modules/.bin/react-native start
 
 You can also add npm scripts to call it with whichever package manager you use:
 
-```json5
+```json
 {
   "scripts": {
     "start": "react-native start"

packages/cli/src/cliEntry.js

@@ -4,7 +4,6 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @format
  * @flow
  */
 

packages/cli/src/core/getCommands.js

@@ -74,10 +74,8 @@ const loadProjectCommands = (root: string): Array<ProjectCommandT> => {
             .join(path.sep)
         : pathToCommands.split(path.sep)[0];
 
-    // $FlowFixMe: Non-literal require
     const pkg = require(path.join(root, 'node_modules', name, 'package.json'));
 
-    // $FlowFixMe: Non-literal require
     const requiredCommands:
       | ProjectCommandT
       | Array<ProjectCommandT> = require(path.join(

packages/cli/src/core/getPlatforms.js

@@ -45,3 +45,12 @@ export default function getPlatforms(root: string): PlatformsT {
     ...projectPlatforms,
   };
 }
+
+const names = {
+  ios: 'iOS',
+  android: 'Android',
+};
+
+export function getPlatformName(name: string) {
+  return names[name] || name;
+}

packages/cli/src/core/types.flow.js

@@ -107,6 +107,15 @@ export type DependencyConfigT = {
   ios: ?DependencyConfigIOST,
 };
 
+export type DependenciesConfig = {
+  config: DependencyConfigT,
+  name: string,
+  path: string,
+  assets: string[],
+  commands: { [name: string]: string },
+  params: InquirerPromptT[],
+};
+
 /**
  * Available platforms. Additional plugins should assert the type on their own.
  */

packages/cli/src/index.js

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @format
+ * @flow
  */
 
 // gracefulify() has to be called before anything else runs

packages/cli/src/install/install.js

@@ -4,30 +4,25 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @format
+ * @flow
  */
 
-import { spawnSync } from 'child_process';
+import type { ContextT } from '../core/types.flow';
 import logger from '../util/logger';
 import PackageManager from '../util/PackageManager';
+import link from '../link/link';
 
-const spawnOpts = {
-  stdio: 'inherit',
-  stdin: 'inherit',
-};
-
-async function install(args, ctx) {
+async function install(args: Array<string>, ctx: ContextT) {
   const name = args[0];
 
+  logger.info(`Installing "${name}"...`);
   new PackageManager({ projectDir: ctx.root }).install([name]);
 
-  const res = spawnSync('react-native', ['link', name], spawnOpts);
-
-  if (res.status) {
-    process.exit(res.status);
-  }
+  logger.info(`Linking "${name}"...`);
+  // eslint-disable-next-line import/no-named-as-default-member
+  await link.func([name], ctx, { platforms: undefined });
 
-  logger.info(`Module ${name} has been successfully installed & linked`);
+  logger.success(`Successfully installed and linked "${name}"`);
 }
 
 export default {

packages/cli/src/install/uninstall.js

@@ -4,30 +4,24 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @format
+ * @flow
  */
 
-import { spawnSync } from 'child_process';
+import type { ContextT } from '../core/types.flow';
 import logger from '../util/logger';
 import PackageManager from '../util/PackageManager';
+import link from '../link/unlink';
 
-const spawnOpts = {
-  stdio: 'inherit',
-  stdin: 'inherit',
-};
-
-async function uninstall(args, ctx) {
+async function uninstall(args: Array<string>, ctx: ContextT) {
   const name = args[0];
 
-  const res = spawnSync('react-native', ['unlink', name], spawnOpts);
-
-  if (res.status) {
-    process.exit(res.status);
-  }
+  logger.info(`Unlinking "${name}"...`);
+  await link.func([name], ctx);
 
+  logger.info(`Uninstalling "${name}"...`);
   new PackageManager({ projectDir: ctx.root }).uninstall([name]);
 
-  logger.info(`Module ${name} has been successfully uninstalled & unlinked`);
+  logger.success(`Successfully uninstalled and unlinked "${name}"`);
 }
 
 export default {

packages/cli/src/link/__tests__/link-test.js

@@ -224,11 +224,15 @@ describe('link', () => {
       commands: {},
     }));
 
-    jest.doMock('../../core/getPlatforms', () => () => ({
-      ios: { linkConfig: require('../ios').default },
-      android: { linkConfig: require('../android').default },
-      windows: { linkConfig: genericLinkConfig },
-    }));
+    jest.doMock('../../core/getPlatforms', () => {
+      const fn = () => ({
+        ios: { linkConfig: require('../ios').default },
+        android: { linkConfig: require('../android').default },
+        windows: { linkConfig: genericLinkConfig },
+      });
+      fn.getPlatformName = jest.fn();
+      return fn;
+    });
 
     jest.doMock('../getDependencyConfig', () => getDependencyConfig);
 
@@ -269,10 +273,14 @@ describe('link', () => {
       register: registerIOSNativeModule,
     });
 
-    jest.doMock('../../core/getPlatforms', () => () => ({
-      android: { linkConfig: genericAndroidLinkConfig },
-      ios: { linkConfig: genericIOSLinkConfig },
-    }));
+    jest.doMock('../../core/getPlatforms', () => {
+      const fn = () => ({
+        android: { linkConfig: genericAndroidLinkConfig },
+        ios: { linkConfig: genericIOSLinkConfig },
+      });
+      fn.getPlatformName = jest.fn();
+      return fn;
+    });
 
     jest.doMock(
       '../android/registerNativeModule.js',

packages/cli/src/link/getDependencyConfig.js

@@ -6,24 +6,14 @@ import path from 'path';
 import type {
   PlatformsT,
   ContextT,
-  InquirerPromptT,
-  DependencyConfigT,
+  DependenciesConfig,
 } from '../core/types.flow';
 
 import getPackageConfiguration from '../core/getPackageConfiguration';
 import getParams from '../core/getParams';
 import getHooks from '../core/getHooks';
 import getAssets from '../core/getAssets';
 
-type DependenciesConfig = {
-  config: DependencyConfigT,
-  name: string,
-  path: string,
-  assets: string[],
-  commands: { [name: string]: string },
-  params: InquirerPromptT[],
-};
-
 export default function getDependencyConfig(
   ctx: ContextT,
   availablePlatforms: PlatformsT,

packages/cli/src/link/getProjectDependencies.js

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @format
+ * @flow
  */
 
 import path from 'path';
@@ -23,9 +23,9 @@ const EXCLUDED_PROJECTS = [
 /**
  * Returns an array of dependencies that should be linked/checked.
  */
-export default function getProjectDependencies(cwd) {
-  const pjson = require(path.join(cwd, './package.json'));
-  return Object.keys(pjson.dependencies || {}).filter(
+export default function getProjectDependencies(cwd: string) {
+  const pkgJson = require(path.join(cwd, './package.json'));
+  return (Object.keys(pkgJson.dependencies || {}).filter(
     name => EXCLUDED_PROJECTS.includes(name) === false
-  );
+  ): Array<string>);
 }

packages/cli/src/link/link.js

@@ -4,15 +4,14 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @format
  * @flow
  */
 
 import { pick } from 'lodash';
 import type { ContextT } from '../core/types.flow';
 
 import promiseWaterfall from './promiseWaterfall';
-import log from '../util/logger';
+import logger from '../util/logger';
 import getDependencyConfig from './getDependencyConfig';
 import commandStub from './commandStub';
 import promisify from './promisify';
@@ -24,7 +23,7 @@ import findReactNativeScripts from '../util/findReactNativeScripts';
 import getPlatforms from '../core/getPlatforms';
 
 type FlagsType = {
-  platforms: Array<string>,
+  platforms?: Array<string>,
 };
 
 /**
@@ -43,7 +42,9 @@ function link([rawPackageName]: Array<string>, ctx: ContextT, opts: FlagsType) {
     }
     project = getProjectConfig(ctx, platforms);
   } catch (err) {
-    log.error('No package found. Are you sure this is a React Native project?');
+    logger.error(
+      'No package found. Are you sure this is a React Native project?'
+    );
     return Promise.reject(err);
   }
   const hasProjectConfig = Object.keys(platforms).reduce(
@@ -77,7 +78,7 @@ function link([rawPackageName]: Array<string>, ctx: ContextT, opts: FlagsType) {
   ];
 
   return promiseWaterfall(tasks).catch(err => {
-    log.error(
+    logger.error(
       `Something went wrong while linking. Error: ${err.message} \n` +
         'Please file an issue here: https://github.com/facebook/react-native/issues'
     );

packages/cli/src/link/linkAll.js

@@ -3,7 +3,7 @@
 import { uniqBy, flatten } from 'lodash';
 import path from 'path';
 import type { ContextT, PlatformsT, ProjectConfigT } from '../core/types.flow';
-import log from '../util/logger';
+import logger from '../util/logger';
 import getAssets from '../core/getAssets';
 import getProjectDependencies from './getProjectDependencies';
 import getDependencyConfig from './getDependencyConfig';
@@ -20,7 +20,7 @@ function linkAll(
   platforms: PlatformsT,
   project: ProjectConfigT
 ) {
-  log.warn(
+  logger.warn(
     'Running `react-native link` without package name is deprecated and will be removed ' +
       'in next release. If you use this command to link your project assets, ' +
       'please let us know about your use case here: https://goo.gl/RKTeoc'
@@ -49,7 +49,7 @@ function linkAll(
   );
 
   return promiseWaterfall(tasks).catch(err => {
-    log.error(
+    logger.error(
       `Something went wrong while linking. Error: ${err.message} \n` +
         'Please file an issue here: https://github.com/facebook/react-native/issues'
     );

packages/cli/src/link/linkAssets.js

@@ -1,14 +1,18 @@
 // @flow
 
 import { isEmpty } from 'lodash';
-import type { PlatformsT, ProjectConfigT } from '../core/types.flow';
+import type {
+  PlatformsT,
+  ProjectConfigT,
+  DependenciesConfig,
+} from '../core/types.flow';
 
-import log from '../util/logger';
+import logger from '../util/logger';
 
 const linkAssets = (
   platforms: PlatformsT,
   project: ProjectConfigT,
-  dependency: *
+  dependency: DependenciesConfig
 ) => {
   if (isEmpty(dependency.assets)) {
     return;
@@ -24,12 +28,12 @@ const linkAssets = (
       return;
     }
 
-    log.info(`Linking assets to ${platform} project`);
+    logger.info(`Linking assets to ${platform} project`);
     // $FlowFixMe: We check for existence of project[platform]
     linkConfig.copyAssets(dependency.assets, project[platform]);
   });
 
-  log.info('Assets have been successfully linked to your project');
+  logger.info('Assets have been successfully linked to your project');
 };
 
 export default linkAssets;