From f7871850d8b3c7f0058de46b231f8c383e2112c4 Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Wed, 10 Apr 2019 19:39:41 +0200 Subject: [PATCH 1/5] Do not throw on invalid packages --- .../__snapshots__/index-test.js.snap | 2 ++ .../src/tools/config/__tests__/index-test.js | 16 +++++++++++++ packages/cli/src/tools/config/index.js | 24 +++++++++++++++---- .../src/tools/config/readConfigFromDisk.js | 7 +++--- packages/tools/src/errors.js | 4 +++- 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap b/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap index d695d85b0..9ce2310e1 100644 --- a/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap +++ b/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap @@ -175,3 +175,5 @@ Object { }, } `; + +exports[`should skip packages that have invalid configuration 1`] = `Object {}`; diff --git a/packages/cli/src/tools/config/__tests__/index-test.js b/packages/cli/src/tools/config/__tests__/index-test.js index cba33e1a2..42aaa04d1 100644 --- a/packages/cli/src/tools/config/__tests__/index-test.js +++ b/packages/cli/src/tools/config/__tests__/index-test.js @@ -236,3 +236,19 @@ test('should not add default React Native config when one present', () => { const {commands} = loadConfig(DIR); expect(commands).toMatchSnapshot(); }); + +test('should skip packages that have invalid configuration', () => { + writeFiles(DIR, { + 'node_modules/react-native/package.json': '{}', + 'node_modules/react-native/react-native.config.js': `module.exports = { + invalidProperty: 5 + }`, + 'package.json': `{ + "dependencies": { + "react-native": "0.0.1" + } + }`, + }); + const {dependencies} = loadConfig(DIR); + expect(dependencies).toMatchSnapshot(); +}); diff --git a/packages/cli/src/tools/config/index.js b/packages/cli/src/tools/config/index.js index d1e31efde..cb6d1841a 100644 --- a/packages/cli/src/tools/config/index.js +++ b/packages/cli/src/tools/config/index.js @@ -3,6 +3,7 @@ */ import path from 'path'; import {mapValues} from 'lodash'; +import chalk from 'chalk'; import findDependencies from './findDependencies'; import resolveReactNativePath from './resolveReactNativePath'; @@ -23,6 +24,7 @@ import merge from '../merge'; */ import * as ios from '@react-native-community/cli-platform-ios'; import * as android from '@react-native-community/cli-platform-android'; +import {logger, inlineString} from '@react-native-community/cli-tools'; /** * Loads CLI configuration @@ -34,14 +36,28 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { (acc: ConfigT, dependencyName) => { const root = path.join(projectRoot, 'node_modules', dependencyName); - const config = - readLegacyDependencyConfigFromDisk(root) || - readDependencyConfigFromDisk(root); + let config; + try { + config = + readLegacyDependencyConfigFromDisk(root) || + readDependencyConfigFromDisk(root); + } catch (error) { + logger.warn( + inlineString(` + Package ${chalk.bold( + dependencyName, + )} has been ignored because it contains invalid configuration. + + Reason: ${chalk.dim(error.message)} + `), + ); + return acc; + } /** * This workaround is neccessary for development only before * first 0.60.0-rc.0 gets released and we can switch to it - * while testing. + * while */ if (dependencyName === 'react-native') { if (Object.keys(config.platforms).length === 0) { diff --git a/packages/cli/src/tools/config/readConfigFromDisk.js b/packages/cli/src/tools/config/readConfigFromDisk.js index c4723afdf..121937e2b 100644 --- a/packages/cli/src/tools/config/readConfigFromDisk.js +++ b/packages/cli/src/tools/config/readConfigFromDisk.js @@ -6,6 +6,7 @@ import Joi from 'joi'; import cosmiconfig from 'cosmiconfig'; import path from 'path'; +import chalk from 'chalk'; import { type UserDependencyConfigT, @@ -116,9 +117,9 @@ export function readLegacyDependencyConfigFromDisk( // @todo: paste a link to documentation that explains the migration steps logger.warn( - `Package '${path.basename( - name, - )}' is using deprecated "rnpm" config that will stop working from next release. Consider upgrading to the new config format.`, + `Package ${chalk.bold( + path.basename(name), + )} is using deprecated "rnpm" config that will stop working from next release. Consider upgrading to the new config format.`, ); const result = Joi.validate(transformedConfig, schema.dependencyConfig); diff --git a/packages/tools/src/errors.js b/packages/tools/src/errors.js index 0fdfbe82e..e7aff63c3 100644 --- a/packages/tools/src/errors.js +++ b/packages/tools/src/errors.js @@ -11,7 +11,7 @@ */ export class CLIError extends Error { constructor(msg: string, originError?: Error | string) { - super(msg.replace(/(\s{2,})/gm, ' ').trim()); + super(inlineString(msg)); if (originError) { this.stack = typeof originError === 'string' @@ -25,3 +25,5 @@ export class CLIError extends Error { } } } + +export const inlineString = str => str.replace(/(\s{2,})/gm, ' ').trim(); From 6ed96f938451afdc44f859852095d6601f499499 Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Wed, 10 Apr 2019 19:43:31 +0200 Subject: [PATCH 2/5] Fix type error --- packages/tools/src/errors.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/tools/src/errors.js b/packages/tools/src/errors.js index e7aff63c3..6cb5de299 100644 --- a/packages/tools/src/errors.js +++ b/packages/tools/src/errors.js @@ -26,4 +26,5 @@ export class CLIError extends Error { } } -export const inlineString = str => str.replace(/(\s{2,})/gm, ' ').trim(); +export const inlineString = (str: string) => + str.replace(/(\s{2,})/gm, ' ').trim(); From af79e50087b9e83d6f5305edbe8c69634b0a269a Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Wed, 10 Apr 2019 20:22:36 +0200 Subject: [PATCH 3/5] Update test --- .../tools/config/__tests__/__snapshots__/index-test.js.snap | 2 ++ packages/cli/src/tools/config/__tests__/index-test.js | 6 +++++- packages/cli/src/tools/config/index.js | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap b/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap index 9ce2310e1..33e0367f6 100644 --- a/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap +++ b/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap @@ -177,3 +177,5 @@ Object { `; exports[`should skip packages that have invalid configuration 1`] = `Object {}`; + +exports[`should skip packages that have invalid configuration 2`] = `"Package react-native has been ignored because it contains invalid configuration. Reason: Unknown option invalidProperty with value \\"5\\" was found. This is either a typing error or a user mistake. Fixing it will remove this message."`; diff --git a/packages/cli/src/tools/config/__tests__/index-test.js b/packages/cli/src/tools/config/__tests__/index-test.js index 42aaa04d1..8a58dd9b7 100644 --- a/packages/cli/src/tools/config/__tests__/index-test.js +++ b/packages/cli/src/tools/config/__tests__/index-test.js @@ -10,6 +10,8 @@ import { getTempDirectory, } from '../../../../../../jest/helpers'; +import {logger} from '@react-native-community/cli-tools'; + const DIR = getTempDirectory('resolve_config_path_test'); // Removes string from all key/values within an object @@ -249,6 +251,8 @@ test('should skip packages that have invalid configuration', () => { } }`, }); + const spy = jest.spyOn(logger, 'warn'); const {dependencies} = loadConfig(DIR); - expect(dependencies).toMatchSnapshot(); + expect(dependencies).toMatchSnapshot('dependencies config'); + expect(spy.mock.calls[0][0]).toMatchSnapshot('logged warning'); }); diff --git a/packages/cli/src/tools/config/index.js b/packages/cli/src/tools/config/index.js index cb6d1841a..de2dbf513 100644 --- a/packages/cli/src/tools/config/index.js +++ b/packages/cli/src/tools/config/index.js @@ -57,7 +57,7 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { /** * This workaround is neccessary for development only before * first 0.60.0-rc.0 gets released and we can switch to it - * while + * while testing */ if (dependencyName === 'react-native') { if (Object.keys(config.platforms).length === 0) { From f4b5eb00f898c26a99b57d896e29449ddbc2024a Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Wed, 10 Apr 2019 20:26:24 +0200 Subject: [PATCH 4/5] add dot --- packages/cli/src/tools/config/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/tools/config/index.js b/packages/cli/src/tools/config/index.js index de2dbf513..640d4d13b 100644 --- a/packages/cli/src/tools/config/index.js +++ b/packages/cli/src/tools/config/index.js @@ -57,7 +57,7 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { /** * This workaround is neccessary for development only before * first 0.60.0-rc.0 gets released and we can switch to it - * while testing + * while testing. */ if (dependencyName === 'react-native') { if (Object.keys(config.platforms).length === 0) { From 6813bc78ab0b49a7961dd14c54d905b085ac916a Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Wed, 10 Apr 2019 20:36:25 +0200 Subject: [PATCH 5/5] Update snapshots --- .../tools/config/__tests__/__snapshots__/index-test.js.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap b/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap index 33e0367f6..02aeb3db5 100644 --- a/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap +++ b/packages/cli/src/tools/config/__tests__/__snapshots__/index-test.js.snap @@ -176,6 +176,6 @@ Object { } `; -exports[`should skip packages that have invalid configuration 1`] = `Object {}`; +exports[`should skip packages that have invalid configuration: dependencies config 1`] = `Object {}`; -exports[`should skip packages that have invalid configuration 2`] = `"Package react-native has been ignored because it contains invalid configuration. Reason: Unknown option invalidProperty with value \\"5\\" was found. This is either a typing error or a user mistake. Fixing it will remove this message."`; +exports[`should skip packages that have invalid configuration: logged warning 1`] = `"Package react-native has been ignored because it contains invalid configuration. Reason: Unknown option invalidProperty with value \\"5\\" was found. This is either a typing error or a user mistake. Fixing it will remove this message."`;