Skip to content

Commit

Permalink
Standardise configuration mechanism (react-native-community#254)
Browse files Browse the repository at this point in the history
Summary:
---------

This pull requests brings a new way of configuring the CLI and drops never properly standardised and documented "rnpm" configuration.

This is done in order to support new "auto-linking" feature and consolidate the configuration into a single place, that is easy to customise by developers.

### Highlights

Given the scope of this PR, it's hard to write down every tiny detail. I've tried to leave as many comments as possible throughout the code to make it easier for you to navigate and understand some of the code patterns.

Please see the highlighted changes below:

- We now use `cosmiconfig` to load user preferences. We do that by taking "react-native" out of "package.json" and reading "react-native.config.js". We still read "rnpm" for legacy purposes and print appropriate deprecation messages along the instructions on what to do in order to upgrade. Jest validation library makes this kind of things super easy.

- We validate the provided configuration by user using Jest validation library. This gives instant feedback whether the configuration is correct or not.

- We now read configuration in a single place (happens in main CLI file) and pass it down to commands. Previously, we used to call `findPlugins` multiple times w/o cache, causing expensive globs to be performed.

- Project configuration is lazy. We won't glob node_modules and/or look for native files unless you explicitly read for that package configuration.

- Better support for out-of-tree-platforms - no need to define "haste" yourself, we are able to infer it from other properties. The files are also better organised now, causing less of maintenance pain with "linkConfig" 

- We consider our automatically generated configuration a set of defaults. Users can override settings for the project and each of the dependencies. Useful, especially with auto-linking feature to disable certain packages from it or when you don't want to link particular platform automatically. This has been historically really hard to implement

- Global flags (e.g. "reactNativePath") can now be defined in configuration, instead of passing every time around. This fixes issues with "packager.sh" script (starting automatically Metro when running from Xcode) when run from RNTester.xcodeproj

### Next steps

Once this PR is merged, we can concurrently start working/merging other "auto-linking" PRs. In the meantime, I'll submit a PR to move regular "link" (soon to be considered a legacy) to use the new configuration format as well.

The new configuration has been designed in the way to still include previous configuration keys to support "link" and other community packages. 

For now, we print handy deprecation messages to help migrate the community from "rnpm" to "react-native" configuration. 

When "link" gets deprecated/removed forever in favour of "auto-linking", we should revisit the configuration and eventually, remove extraneous keys out of it. With "auto-linking", we don't need majority of it.

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

Run `react-native config` to output the configuration.
  • Loading branch information
grabbou authored and dratwas committed Jul 12, 2019
1 parent ac673ba commit 31e68a6
Show file tree
Hide file tree
Showing 22 changed files with 984 additions and 147 deletions.
3 changes: 3 additions & 0 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
"commander": "^2.19.0",
"compression": "^1.7.1",
"connect": "^3.6.5",
"cosmiconfig": "^5.1.0",
"deepmerge": "^3.2.0",
"denodeify": "^1.2.1",
"envinfo": "^7.1.0",
"errorhandler": "^1.5.0",
Expand All @@ -32,6 +34,7 @@
"glob": "^7.1.1",
"graceful-fs": "^4.1.3",
"inquirer": "^3.0.6",
"joi": "^14.3.1",
"lodash": "^4.17.5",
"metro": "^0.53.1",
"metro-config": "^0.53.1",
Expand Down
52 changes: 5 additions & 47 deletions packages/cli/src/cliEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,21 @@
import chalk from 'chalk';
import childProcess from 'child_process';
import commander from 'commander';
import minimist from 'minimist';
import path from 'path';

import type {CommandT, ContextT} from './tools/types.flow';
import getLegacyConfig from './tools/getLegacyConfig';

import {getCommands} from './commands';
import init from './commands/init/init';
import assertRequiredOptions from './tools/assertRequiredOptions';
import logger from './tools/logger';
import findPlugins from './tools/findPlugins';
import {setProjectDir} from './tools/PackageManager';
import pkgJson from '../package.json';
import loadConfig from './tools/config';

commander
.option('--version', 'Print CLI version')
.option('--projectRoot [string]', 'Path to the root of the project')
.option('--reactNativePath [string]', 'Path to React Native')
.option('--verbose', 'Increase logging verbosity');

commander.on('command:*', () => {
Expand Down Expand Up @@ -117,15 +116,6 @@ const addCommand = (command: CommandT, ctx: ContextT) => {
opt.default,
),
);

/**
* We want every command (like "start", "link") to accept below options.
* To achieve that we append them to regular options of each command here.
* This way they'll be displayed in the commands --help menus.
*/
cmd
.option('--projectRoot [string]', 'Path to the root of the project')
.option('--reactNativePath [string]', 'Path to React Native');
};

async function run() {
Expand Down Expand Up @@ -156,43 +146,11 @@ async function setupAndRun() {
}
}

/**
* At this point, commander arguments are not parsed yet because we need to
* add all the commands and their options. That's why we resort to using
* minimist for parsing some global options.
*/
const options = minimist(process.argv.slice(2));

const root = options.projectRoot
? path.resolve(options.projectRoot)
: process.cwd();

const reactNativePath = options.reactNativePath
? path.resolve(options.reactNativePath)
: (() => {
try {
return path.dirname(
// $FlowIssue: Wrong `require.resolve` type definition
require.resolve('react-native/package.json', {
paths: [root],
}),
);
} catch (_ignored) {
throw new Error(
'Unable to find React Native files. Make sure "react-native" module is installed in your project dependencies.',
);
}
})();

const ctx = {
...getLegacyConfig(root),
reactNativePath,
root,
};
const ctx = loadConfig();

setProjectDir(ctx.root);

const commands = getCommands(ctx.root);
const commands = getCommands(ctx);

commands.forEach(command => addCommand(command, ctx));

Expand Down
11 changes: 11 additions & 0 deletions packages/cli/src/commands/config/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* @flow
*/
import {type ContextT} from '../../tools/types.flow';
export default {
name: 'config',
description: 'Print CLI configuration',
func: async (argv: string[], ctx: ContextT) => {
console.log(JSON.stringify(ctx, null, 2));
},
};
28 changes: 16 additions & 12 deletions packages/cli/src/commands/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import path from 'path';

import findPlugins from '../tools/findPlugins';
import logger from '../tools/logger';

import type {
Expand All @@ -13,6 +12,8 @@ import type {
LocalCommandT,
} from '../tools/types.flow';

import {type ContextT} from '../tools/types.flow';

import server from './server/server';
import runIOS from './runIOS/runIOS';
import runAndroid from './runAndroid/runAndroid';
Expand All @@ -27,6 +28,7 @@ import upgrade from './upgrade/upgrade';
import logAndroid from './logAndroid/logAndroid';
import logIOS from './logIOS/logIOS';
import info from './info/info';
import config from './config/config';

/**
* List of built-in commands
Expand All @@ -47,6 +49,7 @@ const loadLocalCommands: Array<LocalCommandT> = [
logAndroid,
logIOS,
info,
config,
];

/**
Expand All @@ -55,10 +58,11 @@ const loadLocalCommands: Array<LocalCommandT> = [
* This checks all CLI plugins for presence of 3rd party packages that define commands
* and loads them
*/
const loadProjectCommands = (root: string): Array<ProjectCommandT> => {
const plugins = findPlugins(root);

return plugins.commands.reduce((acc: Array<CommandT>, pathToCommands) => {
const loadProjectCommands = ({
root,
commands,
}: ContextT): Array<ProjectCommandT> => {
return commands.reduce((acc: Array<ProjectCommandT>, cmdPath: string) => {
/**
* `pathToCommand` is a path to a file where commands are defined, relative to `node_modules`
* folder.
Expand All @@ -67,12 +71,12 @@ const loadProjectCommands = (root: string): Array<ProjectCommandT> => {
* into consideration.
*/
const name =
pathToCommands[0] === '@'
? pathToCommands
cmdPath[0] === '@'
? cmdPath
.split(path.sep)
.slice(0, 2)
.join(path.sep)
: pathToCommands.split(path.sep)[0];
: cmdPath.split(path.sep)[0];

const pkg = require(path.join(root, 'node_modules', name, 'package.json'));

Expand All @@ -81,7 +85,7 @@ const loadProjectCommands = (root: string): Array<ProjectCommandT> => {
| Array<ProjectCommandT> = require(path.join(
root,
'node_modules',
pathToCommands,
cmdPath,
));

if (Array.isArray(requiredCommands)) {
Expand All @@ -90,14 +94,14 @@ const loadProjectCommands = (root: string): Array<ProjectCommandT> => {
);
}

return acc.concat({...requiredCommands});
return acc.concat({...requiredCommands, pkg});
}, []);
};

/**
* Loads all the commands inside a given `root` folder
*/
export function getCommands(root: string): Array<CommandT> {
export function getCommands(ctx: ContextT): Array<CommandT> {
return [
...loadLocalCommands,
{
Expand All @@ -111,6 +115,6 @@ export function getCommands(root: string): Array<CommandT> {
);
},
},
...loadProjectCommands(root),
...loadProjectCommands(ctx),
];
}
12 changes: 11 additions & 1 deletion packages/cli/src/commands/info/__tests__/info.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,17 @@ jest.mock('../../../tools/logger', () => ({
log: jest.fn(),
}));

const ctx = {reactNativePath: '', root: ''};
const ctx = {
root: '',
reactNativePath: '',
dependencies: {},
platforms: {},
commands: [],
haste: {
platforms: [],
providesModuleNodeModules: [],
},
};

beforeEach(() => {
jest.resetAllMocks();
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/commands/link/getDependencyConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export default function getDependencyConfig(
Object.keys(availablePlatforms).forEach(platform => {
platformConfigs[platform] = availablePlatforms[platform].dependencyConfig(
folder,
// $FlowIssue: Flow can't match platform config with its appropriate config function
config[platform] || {},
);
});
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/commands/link/getProjectConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default function getProjectConfig(
logger.debug(`Getting project config for ${getPlatformName(platform)}...`);
platformConfigs[platform] = availablePlatforms[platform].projectConfig(
ctx.root,
// $FlowIssue: Flow can't match platform config with its appropriate config function
config[platform] || {},
);
});
Expand Down
8 changes: 4 additions & 4 deletions packages/cli/src/commands/link/linkAll.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,19 @@ function linkAll(

const projectAssets = getAssets(context.root);
const dependencies = getProjectDependencies(context.root);
const depenendenciesConfig = dependencies.map(dependnecy =>
getDependencyConfig(context, platforms, dependnecy),
const dependenciesConfig = dependencies.map(dependency =>
getDependencyConfig(context, platforms, dependency),
);

const assets = dedupeAssets(
depenendenciesConfig.reduce(
dependenciesConfig.reduce(
(acc, dependency) => acc.concat(dependency.assets),
projectAssets,
),
);

const tasks = flatten(
depenendenciesConfig.map(config => [
dependenciesConfig.map(config => [
() => promisify(config.commands.prelink || commandStub),
() => linkDependency(platforms, project, config),
() => promisify(config.commands.postlink || commandStub),
Expand Down
7 changes: 7 additions & 0 deletions packages/cli/src/commands/upgrade/__tests__/upgrade.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ const olderVersion = '0.56.0';
const ctx = {
root: '/project/root',
reactNativePath: '',
commands: [],
platforms: {},
dependencies: {},
haste: {
providesModuleNodeModules: [],
platforms: [],
},
};
const opts = {
legacy: false,
Expand Down
Loading

0 comments on commit 31e68a6

Please sign in to comment.