From f0fb1e5ba6599dfac5089f209fdad938163a72e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 21 Jun 2019 12:46:20 +0200 Subject: [PATCH] feat: filter out non-native deps from config output; fix missing hooks --- jest/setupUnitTests.js | 16 +- packages/cli/src/commands/config/config.js | 23 ++- .../src/commands/link/__tests__/link-test.js | 28 ++-- .../link}/__tests__/makeHook-test.js | 0 packages/cli/src/commands/link/link.js | 5 +- packages/cli/src/commands/link/linkAll.js | 5 +- .../config => commands/link}/makeHook.js | 0 packages/cli/src/commands/link/unlink.js | 5 +- packages/cli/src/tools/config/index.js | 145 ++++++++++-------- .../__tests__/findLineToAddPod-test.js | 2 +- types/index.js | 6 +- 11 files changed, 140 insertions(+), 95 deletions(-) rename packages/cli/src/{tools/config => commands/link}/__tests__/makeHook-test.js (100%) rename packages/cli/src/{tools/config => commands/link}/makeHook.js (100%) diff --git a/jest/setupUnitTests.js b/jest/setupUnitTests.js index cff360d25..a6ea7c372 100644 --- a/jest/setupUnitTests.js +++ b/jest/setupUnitTests.js @@ -2,6 +2,20 @@ * @flow */ -jest.mock('@react-native-community/cli-tools'); +jest.mock('@react-native-community/cli-tools', () => { + return { + ...jest.requireActual('@react-native-community/cli-tools'), + logger: { + success: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + log: jest.fn(), + setVerbose: jest.fn(), + isVerbose: jest.fn(), + }, + }; +}); jest.setTimeout(20000); diff --git a/packages/cli/src/commands/config/config.js b/packages/cli/src/commands/config/config.js index 36b0c7130..37705d0d3 100644 --- a/packages/cli/src/commands/config/config.js +++ b/packages/cli/src/commands/config/config.js @@ -2,10 +2,31 @@ * @flow */ import {type ConfigT} from 'types'; + +function isValidRNDependency(config) { + return ( + Object.keys(config.platforms).filter(key => Boolean(config.platforms[key])) + .length !== 0 || + Object.keys(config.hooks).length !== 0 || + config.assets.length !== 0 || + config.params.length !== 0 + ); +} + +function filterConfig(config) { + const filtered = {...config}; + Object.keys(filtered.dependencies).forEach(item => { + if (!isValidRNDependency(filtered.dependencies[item])) { + delete filtered.dependencies[item]; + } + }); + return filtered; +} + export default { name: 'config', description: 'Print CLI configuration', func: async (argv: string[], ctx: ConfigT) => { - console.log(JSON.stringify(ctx, null, 2)); + console.log(JSON.stringify(filterConfig(ctx), null, 2)); }, }; diff --git a/packages/cli/src/commands/link/__tests__/link-test.js b/packages/cli/src/commands/link/__tests__/link-test.js index a9c173f3c..02c537344 100644 --- a/packages/cli/src/commands/link/__tests__/link-test.js +++ b/packages/cli/src/commands/link/__tests__/link-test.js @@ -1,9 +1,15 @@ import {func as link} from '../link'; import loadConfig from '../../../tools/config'; +import makeHook from '../makeHook'; jest.mock('chalk', () => ({grey: str => str, bold: str => str})); jest.mock('../../../tools/config'); jest.mock('../../../tools/logger'); +jest.mock('../makeHook', () => { + return jest.fn(() => { + return jest.fn(() => Promise.resolve()); + }); +}); const baseConfig = loadConfig(); @@ -54,9 +60,9 @@ describe('link', () => { expect(spy).toHaveBeenCalled(); }); - it('should register native module when android/ios projects are present', done => { - const prelink = jest.fn(); - const postlink = jest.fn(); + it('should register native module when android/ios projects are present', async () => { + const prelink = 'node prelink.js'; + const postlink = 'node postlink.js'; const registerNativeModule = jest.fn(); const config = { @@ -87,19 +93,9 @@ describe('link', () => { }, }; - link(['react-native-blur'], config, {}).then(() => { - expect(registerNativeModule.mock.calls).toHaveLength(2); - - expect(prelink.mock.invocationCallOrder[0]).toBeLessThan( - registerNativeModule.mock.invocationCallOrder[0], - ); - - expect(postlink.mock.invocationCallOrder[0]).toBeGreaterThan( - registerNativeModule.mock.invocationCallOrder[0], - ); - - done(); - }); + await link(['react-native-blur'], config, {}); + expect(registerNativeModule.mock.calls).toHaveLength(2); + expect(makeHook.mock.calls).toEqual([[prelink], [postlink]]); }); it('should copy assets only from the specific dependency that we are linking', done => { diff --git a/packages/cli/src/tools/config/__tests__/makeHook-test.js b/packages/cli/src/commands/link/__tests__/makeHook-test.js similarity index 100% rename from packages/cli/src/tools/config/__tests__/makeHook-test.js rename to packages/cli/src/commands/link/__tests__/makeHook-test.js diff --git a/packages/cli/src/commands/link/link.js b/packages/cli/src/commands/link/link.js index 94627c22b..d7e458e82 100644 --- a/packages/cli/src/commands/link/link.js +++ b/packages/cli/src/commands/link/link.js @@ -15,6 +15,7 @@ import getPlatformName from './getPlatformName'; import linkDependency from './linkDependency'; import linkAssets from './linkAssets'; import linkAll from './linkAll'; +import makeHook from './makeHook'; type FlagsType = { platforms?: Array, @@ -68,11 +69,11 @@ async function link( try { if (dependency.hooks.prelink) { - await dependency.hooks.prelink(); + await makeHook(dependency.hooks.prelink)(); } await linkDependency(platforms, project, dependency); if (dependency.hooks.postlink) { - await dependency.hooks.postlink(); + await makeHook(dependency.hooks.postlink)(); } await linkAssets(platforms, project, dependency.assets); } catch (error) { diff --git a/packages/cli/src/commands/link/linkAll.js b/packages/cli/src/commands/link/linkAll.js index 50878141f..97ce27ade 100644 --- a/packages/cli/src/commands/link/linkAll.js +++ b/packages/cli/src/commands/link/linkAll.js @@ -9,6 +9,7 @@ import {CLIError, logger} from '@react-native-community/cli-tools'; import type {ConfigT} from 'types'; import linkAssets from './linkAssets'; import linkDependency from './linkDependency'; +import makeHook from './makeHook'; const dedupeAssets = (assets: Array): Array => uniqBy(assets, asset => path.basename(asset)); @@ -35,11 +36,11 @@ async function linkAll(config: ConfigT, options: Options) { const dependency = config.dependencies[key]; try { if (dependency.hooks.prelink) { - await dependency.hooks.prelink(); + await makeHook(dependency.hooks.prelink)(); } await linkDependency(config.platforms, config.project, dependency); if (dependency.hooks.postlink) { - await dependency.hooks.postlink(); + await makeHook(dependency.hooks.postlink)(); } } catch (error) { throw new CLIError( diff --git a/packages/cli/src/tools/config/makeHook.js b/packages/cli/src/commands/link/makeHook.js similarity index 100% rename from packages/cli/src/tools/config/makeHook.js rename to packages/cli/src/commands/link/makeHook.js diff --git a/packages/cli/src/commands/link/unlink.js b/packages/cli/src/commands/link/unlink.js index 411b6d70b..5b91768b1 100644 --- a/packages/cli/src/commands/link/unlink.js +++ b/packages/cli/src/commands/link/unlink.js @@ -11,6 +11,7 @@ import {flatMap, values, difference} from 'lodash'; import {logger, CLIError} from '@react-native-community/cli-tools'; import type {ConfigT} from 'types'; import getPlatformName from './getPlatformName'; +import makeHook from './makeHook'; const unlinkDependency = ( platforms, @@ -91,7 +92,7 @@ async function unlink(args: Array, ctx: ConfigT) { const dependencies = values(otherDependencies); try { if (dependency.hooks.preulink) { - await dependency.hooks.preulink(); + await makeHook(dependency.hooks.preulink)(); } unlinkDependency( ctx.platforms, @@ -101,7 +102,7 @@ async function unlink(args: Array, ctx: ConfigT) { dependencies, ); if (dependency.hooks.postunlink) { - await dependency.hooks.postunlink(); + await makeHook(dependency.hooks.postunlink)(); } } catch (error) { throw new CLIError( diff --git a/packages/cli/src/tools/config/index.js b/packages/cli/src/tools/config/index.js index 704f03bee..a78f9f99c 100644 --- a/packages/cli/src/tools/config/index.js +++ b/packages/cli/src/tools/config/index.js @@ -2,29 +2,54 @@ * @flow */ import path from 'path'; -import {mapValues} from 'lodash'; import chalk from 'chalk'; - +import {logger, inlineString} from '@react-native-community/cli-tools'; +import * as ios from '@react-native-community/cli-platform-ios'; +import * as android from '@react-native-community/cli-platform-android'; import findDependencies from './findDependencies'; import resolveReactNativePath from './resolveReactNativePath'; import findAssets from './findAssets'; -import makeHook from './makeHook'; import { readConfigFromDisk, readDependencyConfigFromDisk, } from './readConfigFromDisk'; - import {type ConfigT} from 'types'; - import assign from '../assign'; import merge from '../merge'; import resolveNodeModuleDir from './resolveNodeModuleDir'; -/** - * Built-in platforms - */ -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'; + +function getDependencyConfig( + root, + dependencyName, + finalConfig, + config, + userConfig, + isPlatform, +) { + return merge( + { + root, + name: dependencyName, + platforms: Object.keys(finalConfig.platforms).reduce( + (dependency, platform) => { + // Linking platforms is not supported + dependency[platform] = isPlatform + ? null + : finalConfig.platforms[platform].dependencyConfig( + root, + config.dependency.platforms[platform] || {}, + ); + return dependency; + }, + {}, + ), + assets: findAssets(root, config.dependency.assets), + hooks: config.dependency.hooks, + params: config.dependency.params, + }, + userConfig.dependencies[dependencyName] || {}, + ); +} /** * Loads CLI configuration @@ -32,6 +57,35 @@ import {logger, inlineString} from '@react-native-community/cli-tools'; function loadConfig(projectRoot: string = process.cwd()): ConfigT { const userConfig = readConfigFromDisk(projectRoot); + const initialConfig: ConfigT = { + root: projectRoot, + get reactNativePath() { + return userConfig.reactNativePath + ? path.resolve(projectRoot, userConfig.reactNativePath) + : resolveReactNativePath(projectRoot); + }, + dependencies: {}, + commands: userConfig.commands, + get assets() { + return findAssets(projectRoot, userConfig.assets); + }, + platforms: userConfig.platforms, + haste: { + providesModuleNodeModules: [], + platforms: Object.keys(userConfig.platforms), + }, + get project() { + const project = {}; + for (const platform in finalConfig.platforms) { + project[platform] = finalConfig.platforms[platform].projectConfig( + projectRoot, + userConfig.project[platform] || {}, + ); + } + return project; + }, + }; + const finalConfig = findDependencies(projectRoot).reduce( (acc: ConfigT, dependencyName) => { let root; @@ -53,9 +107,8 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { } /** - * This workaround is necessary for development only before - * first 0.60.0-rc.0 gets released and we can switch to it - * while testing. + * @todo: remove this code once `react-native` is published with + * `platforms` and `commands` inside `react-native.config.js`. */ if (dependencyName === 'react-native') { if (Object.keys(config.platforms).length === 0) { @@ -72,39 +125,24 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { * Legacy `rnpm` config required `haste` to be defined. With new config, * we do it automatically. * - * Remove this once `rnpm` config is deprecated. + * @todo: Remove this once `rnpm` config is deprecated and all major RN libs are converted. */ const haste = config.haste || { providesModuleNodeModules: isPlatform ? [dependencyName] : [], platforms: Object.keys(config.platforms), }; - return assign({}, acc, { + return (assign({}, acc, { dependencies: assign({}, acc.dependencies, { // $FlowExpectedError: Dynamic getters are not supported get [dependencyName]() { - return merge( - { - root, - name: dependencyName, - platforms: Object.keys(finalConfig.platforms).reduce( - (dependency, platform) => { - // Linking platforms is not supported - dependency[platform] = isPlatform - ? null - : finalConfig.platforms[platform].dependencyConfig( - root, - config.dependency.platforms[platform] || {}, - ); - return dependency; - }, - {}, - ), - assets: findAssets(root, config.dependency.assets), - hooks: mapValues(config.dependency.hooks, makeHook), - params: config.dependency.params, - }, - userConfig.dependencies[dependencyName] || {}, + return getDependencyConfig( + root, + dependencyName, + finalConfig, + config, + userConfig, + isPlatform, ); }, }), @@ -120,36 +158,9 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { ], platforms: [...acc.haste.platforms, ...haste.platforms], }, - }); + }): ConfigT); }, - ({ - root: projectRoot, - get reactNativePath() { - return userConfig.reactNativePath - ? path.resolve(projectRoot, userConfig.reactNativePath) - : resolveReactNativePath(projectRoot); - }, - dependencies: {}, - commands: userConfig.commands, - get assets() { - return findAssets(projectRoot, userConfig.assets); - }, - platforms: userConfig.platforms, - haste: { - providesModuleNodeModules: [], - platforms: Object.keys(userConfig.platforms), - }, - get project() { - const project = {}; - for (const platform in finalConfig.platforms) { - project[platform] = finalConfig.platforms[platform].projectConfig( - projectRoot, - userConfig.project[platform] || {}, - ); - } - return project; - }, - }: ConfigT), + initialConfig, ); return finalConfig; diff --git a/packages/platform-ios/src/link-pods/__tests__/findLineToAddPod-test.js b/packages/platform-ios/src/link-pods/__tests__/findLineToAddPod-test.js index 245190d01..a3d4e4e46 100644 --- a/packages/platform-ios/src/link-pods/__tests__/findLineToAddPod-test.js +++ b/packages/platform-ios/src/link-pods/__tests__/findLineToAddPod-test.js @@ -15,7 +15,7 @@ const path = require('path'); const PODFILES_PATH = path.join(__dirname, '../__fixtures__/'); const LINE_AFTER_TARGET_IN_TEST_PODFILE = 4; -console.log(PODFILES_PATH); + describe('pods::findLineToAddPod', () => { it('returns null if file is not Podfile', () => { const podfile = readPodfile(path.join(PODFILES_PATH, 'Info.plist')); diff --git a/types/index.js b/types/index.js index 0c435a0e0..c2c6d6523 100644 --- a/types/index.js +++ b/types/index.js @@ -121,9 +121,9 @@ export type ConfigT = {| }, assets: string[], hooks: { - [key: string]: () => void, - prelink?: () => void, - postlink?: () => void, + [key: string]: string, + prelink?: string, + postlink?: string, }, params: InquirerPromptT[], },