From f8be283825bded5d76c7bb3536c2fac8e64b528a Mon Sep 17 00:00:00 2001 From: Matei Radu Date: Sat, 16 Mar 2019 15:09:33 +0100 Subject: [PATCH] Add verbose logging flag, debug logging for (un)link commands (#239) Summary: --------- This PR addresses #96 and: * Adds the `--verbose` flag to enable `DEBUG` logs * Adds `setVerbose(boolean)` to `tools/logger.js` so that it can enable/disable `DEBUG` logs * Adds debug logs for the (un)link commands * Adds Flow typings to `commands/link/android/unlinkAssets.js` I haven't added `DEBUG` logs to other parts of the CLI because: * I'm not that familiar with the other CLI commands * I'd like some feedback on what I've done so far before going on Also, would it be better to add debug logs in chunks, say one PR per command? I suspect this PR would become too big otherwise. Test Plan: ---------- These additions do not change any implementations but I ran the tests anyway, all green. I also ran it against two existing projects and a freshly created one: no changes in behavior. Screenshots: ---------- How the new flag is presented when using `--help`: Screenshot 2019-03-15 at 22 07 26 How `link packageName --verbose` looks like: Screenshot 2019-03-15 at 22 09 08 How `link packageName --verbose` looks like (module with assets): Screenshot 2019-03-15 at 22 33 28 How `unlink packageName --verbose` looks like: Screenshot 2019-03-15 at 22 09 34 Screenshot 2019-03-15 at 22 13 58 --- packages/cli/src/cliEntry.js | 5 ++++- .../src/commands/link/android/copyAssets.js | 3 +++ .../link/android/patches/applyPatch.js | 5 +++++ .../link/android/patches/revokePatch.js | 5 +++++ .../src/commands/link/android/unlinkAssets.js | 9 ++++++++- .../cli/src/commands/link/getProjectConfig.js | 3 +++ .../link/ios/addToHeaderSearchPaths.js | 2 ++ .../cli/src/commands/link/ios/copyAssets.js | 13 ++++++++----- .../commands/link/ios/registerNativeModule.js | 5 +++++ .../link/ios/removeFromHeaderSearchPaths.js | 2 ++ .../cli/src/commands/link/ios/unlinkAssets.js | 10 ++++++---- .../link/ios/unregisterNativeModule.js | 8 ++++++++ packages/cli/src/commands/link/link.js | 19 ++++++++++++++++++- .../cli/src/commands/link/pods/addPodEntry.js | 10 +++++++--- .../cli/src/commands/link/pods/readPodfile.js | 2 ++ .../src/commands/link/pods/removePodEntry.js | 3 +++ .../cli/src/commands/link/pods/savePodFile.js | 2 ++ .../link/pods/unregisterNativeModule.js | 2 ++ packages/cli/src/tools/logger.js | 11 ++++++++++- 19 files changed, 103 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/cliEntry.js b/packages/cli/src/cliEntry.js index dcd9941c56..fab22bb7ce 100644 --- a/packages/cli/src/cliEntry.js +++ b/packages/cli/src/cliEntry.js @@ -24,7 +24,8 @@ import pkgJson from '../package.json'; commander .option('--version', 'Print CLI version') .option('--projectRoot [string]', 'Path to the root of the project') - .option('--reactNativePath [string]', 'Path to React Native'); + .option('--reactNativePath [string]', 'Path to React Native') + .option('--verbose', 'Increase logging verbosity'); commander.on('command:*', () => { printUnknownCommand(commander.args.join(' ')); @@ -204,6 +205,8 @@ async function setupAndRun() { if (commander.args.length === 0 && commander.version === true) { console.log(pkgJson.version); } + + logger.setVerbose(commander.verbose); } export default { diff --git a/packages/cli/src/commands/link/android/copyAssets.js b/packages/cli/src/commands/link/android/copyAssets.js index fb8f124db4..1f475fcdbe 100644 --- a/packages/cli/src/commands/link/android/copyAssets.js +++ b/packages/cli/src/commands/link/android/copyAssets.js @@ -10,6 +10,7 @@ import fs from 'fs-extra'; import path from 'path'; import groupFilesByType from '../groupFilesByType'; +import logger from '../../../tools/logger'; /** * Copies each file from an array of assets provided to targetPath directory @@ -23,8 +24,10 @@ export default function copyAssetsAndroid( ) { const assets = groupFilesByType(files); + logger.debug(`Assets path: ${project.assetsPath}`); (assets.font || []).forEach(asset => { const fontsDir = path.join(project.assetsPath, 'fonts'); + logger.debug(`Copying asset ${asset}`); // @todo: replace with fs.mkdirSync(path, {recursive}) + fs.copyFileSync // and get rid of fs-extra once we move to Node 10 fs.copySync(asset, path.join(fontsDir, path.basename(asset))); diff --git a/packages/cli/src/commands/link/android/patches/applyPatch.js b/packages/cli/src/commands/link/android/patches/applyPatch.js index eb8f0d863f..704646ebdd 100644 --- a/packages/cli/src/commands/link/android/patches/applyPatch.js +++ b/packages/cli/src/commands/link/android/patches/applyPatch.js @@ -8,8 +8,13 @@ */ import fs from 'fs'; +import logger from '../../../../tools/logger'; export default function applyPatch(file, patch) { + if (file) { + logger.debug(`Patching ${file}`); + } + fs.writeFileSync( file, fs diff --git a/packages/cli/src/commands/link/android/patches/revokePatch.js b/packages/cli/src/commands/link/android/patches/revokePatch.js index ed410953d9..ca0fb753d5 100644 --- a/packages/cli/src/commands/link/android/patches/revokePatch.js +++ b/packages/cli/src/commands/link/android/patches/revokePatch.js @@ -8,8 +8,13 @@ */ import fs from 'fs'; +import logger from '../../../../tools/logger'; export default function revokePatch(file, patch) { + if (file) { + logger.debug(`Patching ${file}`); + } + fs.writeFileSync( file, fs.readFileSync(file, 'utf8').replace(patch.patch, ''), diff --git a/packages/cli/src/commands/link/android/unlinkAssets.js b/packages/cli/src/commands/link/android/unlinkAssets.js index e1b39fd1d6..c15aa4c2c0 100644 --- a/packages/cli/src/commands/link/android/unlinkAssets.js +++ b/packages/cli/src/commands/link/android/unlinkAssets.js @@ -5,11 +5,13 @@ * LICENSE file in the root directory of this source tree. * * @format + * @flow */ import fs from 'fs'; import path from 'path'; import groupFilesByType from '../groupFilesByType'; +import logger from '../../../tools/logger'; /** * Copies each file from an array of assets provided to targetPath directory @@ -17,9 +19,13 @@ import groupFilesByType from '../groupFilesByType'; * For now, the only types of files that are handled are: * - Fonts (otf, ttf) - copied to targetPath/fonts under original name */ -export default function unlinkAssetsAndroid(files, project) { +export default function unlinkAssetsAndroid( + files: Array, + project: {assetsPath: string}, +) { const assets = groupFilesByType(files); + logger.debug(`Assets path: ${project.assetsPath}`); (assets.font || []).forEach(file => { const filePath = path.join( project.assetsPath, @@ -27,6 +33,7 @@ export default function unlinkAssetsAndroid(files, project) { path.basename(file), ); if (fs.existsSync(filePath)) { + logger.debug(`Removing asset ${filePath}`); fs.unlinkSync(filePath); } }); diff --git a/packages/cli/src/commands/link/getProjectConfig.js b/packages/cli/src/commands/link/getProjectConfig.js index f5095315ad..9172f012bf 100644 --- a/packages/cli/src/commands/link/getProjectConfig.js +++ b/packages/cli/src/commands/link/getProjectConfig.js @@ -9,6 +9,8 @@ import type { } from '../../tools/types.flow'; import getPackageConfiguration from '../../tools/getPackageConfiguration'; +import {getPlatformName} from '../../tools/getPlatforms'; +import logger from '../../tools/logger'; export default function getProjectConfig( ctx: ContextT, @@ -19,6 +21,7 @@ export default function getProjectConfig( const platformConfigs = {ios: undefined, android: undefined}; Object.keys(availablePlatforms).forEach(platform => { + logger.debug(`Getting project config for ${getPlatformName(platform)}...`); platformConfigs[platform] = availablePlatforms[platform].projectConfig( ctx.root, config[platform] || {}, diff --git a/packages/cli/src/commands/link/ios/addToHeaderSearchPaths.js b/packages/cli/src/commands/link/ios/addToHeaderSearchPaths.js index 280499253a..83177ba74d 100644 --- a/packages/cli/src/commands/link/ios/addToHeaderSearchPaths.js +++ b/packages/cli/src/commands/link/ios/addToHeaderSearchPaths.js @@ -8,7 +8,9 @@ */ import mapHeaderSearchPaths from './mapHeaderSearchPaths'; +import logger from '../../../tools/logger'; export default function addToHeaderSearchPaths(project, path) { + logger.debug(`Adding ${path} to header search paths`); mapHeaderSearchPaths(project, searchPaths => searchPaths.concat(path)); } diff --git a/packages/cli/src/commands/link/ios/copyAssets.js b/packages/cli/src/commands/link/ios/copyAssets.js index cf9fcd796f..f3e89c161c 100644 --- a/packages/cli/src/commands/link/ios/copyAssets.js +++ b/packages/cli/src/commands/link/ios/copyAssets.js @@ -14,6 +14,7 @@ import groupFilesByType from '../groupFilesByType'; import createGroupWithMessage from './createGroupWithMessage'; import getPlist from './getPlist'; import writePlist from './writePlist'; +import logger from '../../../tools/logger'; /** * This function works in a similar manner to its Android version, @@ -28,11 +29,13 @@ export default function linkAssetsIOS(files, projectConfig) { function addResourceFile(f) { return (f || []) - .map(asset => - project.addResourceFile(path.relative(projectConfig.sourceDir, asset), { - target: project.getFirstTarget().uuid, - }), - ) + .map(asset => { + logger.debug(`Linking asset ${asset}`); + return project.addResourceFile( + path.relative(projectConfig.sourceDir, asset), + {target: project.getFirstTarget().uuid}, + ); + }) .filter(file => file) // xcode returns false if file is already there .map(file => file.basename); } diff --git a/packages/cli/src/commands/link/ios/registerNativeModule.js b/packages/cli/src/commands/link/ios/registerNativeModule.js index d8d9e7551d..943296aaf0 100644 --- a/packages/cli/src/commands/link/ios/registerNativeModule.js +++ b/packages/cli/src/commands/link/ios/registerNativeModule.js @@ -20,6 +20,7 @@ import createGroupWithMessage from './createGroupWithMessage'; import addFileToProject from './addFileToProject'; import addProjectToLibraries from './addProjectToLibraries'; import addSharedLibraries from './addSharedLibraries'; +import logger from '../../../tools/logger'; /** * Register native module IOS adds given dependency to project by adding @@ -32,6 +33,7 @@ export default function registerNativeModuleIOS( dependencyConfig, projectConfig, ) { + logger.debug(`Reading ${projectConfig.pbxprojPath}`); const project = xcode.project(projectConfig.pbxprojPath).parseSync(); const dependencyProject = xcode .project(dependencyConfig.pbxprojPath) @@ -55,6 +57,7 @@ export default function registerNativeModuleIOS( if (!product.isTVOS) { for (i = 0; i < targets.length; i++) { if (!targets[i].isTVOS) { + logger.debug(`Adding ${product.name} to ${targets[i].target.name}`); project.addStaticLibrary(product.name, { target: targets[i].uuid, }); @@ -65,6 +68,7 @@ export default function registerNativeModuleIOS( if (product.isTVOS) { for (i = 0; i < targets.length; i++) { if (targets[i].isTVOS) { + logger.debug(`Adding ${product.name} to ${targets[i].target.name}`); project.addStaticLibrary(product.name, { target: targets[i].uuid, }); @@ -83,5 +87,6 @@ export default function registerNativeModuleIOS( ); } + logger.debug(`Writing changes to ${projectConfig.pbxprojPath}`); fs.writeFileSync(projectConfig.pbxprojPath, project.writeSync()); } diff --git a/packages/cli/src/commands/link/ios/removeFromHeaderSearchPaths.js b/packages/cli/src/commands/link/ios/removeFromHeaderSearchPaths.js index ee5333fb50..9db40a0beb 100644 --- a/packages/cli/src/commands/link/ios/removeFromHeaderSearchPaths.js +++ b/packages/cli/src/commands/link/ios/removeFromHeaderSearchPaths.js @@ -8,11 +8,13 @@ */ import mapHeaderSearchPaths from './mapHeaderSearchPaths'; +import logger from '../../../tools/logger'; /** * Given Xcode project and absolute path, it makes sure there are no headers referring to it */ export default function addToHeaderSearchPaths(project, path) { + logger.debug(`Removing ${path} from header search paths`); mapHeaderSearchPaths(project, searchPaths => searchPaths.filter(searchPath => searchPath !== path), ); diff --git a/packages/cli/src/commands/link/ios/unlinkAssets.js b/packages/cli/src/commands/link/ios/unlinkAssets.js index 546e1d3714..f0da534237 100644 --- a/packages/cli/src/commands/link/ios/unlinkAssets.js +++ b/packages/cli/src/commands/link/ios/unlinkAssets.js @@ -15,6 +15,7 @@ import log from '../../../tools/logger'; import groupFilesByType from '../groupFilesByType'; import getPlist from './getPlist'; import writePlist from './writePlist'; +import logger from '../../../tools/logger'; /** * Unlinks assets from iOS project. Removes references for fonts from `Info.plist` @@ -41,12 +42,13 @@ export default function unlinkAssetsIOS(files, projectConfig) { const removeResourceFiles = (f = []) => (f || []) - .map(asset => - project.removeResourceFile( + .map(asset => { + logger.debug(`Unlinking asset ${asset}`); + return project.removeResourceFile( path.relative(projectConfig.sourceDir, asset), {target: project.getFirstTarget().uuid}, - ), - ) + ); + }) .map(file => file.basename); removeResourceFiles(assets.image); diff --git a/packages/cli/src/commands/link/ios/unregisterNativeModule.js b/packages/cli/src/commands/link/ios/unregisterNativeModule.js index 6dbc116443..b0a1239afd 100644 --- a/packages/cli/src/commands/link/ios/unregisterNativeModule.js +++ b/packages/cli/src/commands/link/ios/unregisterNativeModule.js @@ -21,6 +21,7 @@ import removeProjectFromLibraries from './removeProjectFromLibraries'; import removeFromStaticLibraries from './removeFromStaticLibraries'; import removeFromHeaderSearchPaths from './removeFromHeaderSearchPaths'; import removeSharedLibraries from './removeSharedLibraries'; +import logger from '../../../tools/logger'; /** * Unregister native module IOS @@ -32,6 +33,7 @@ export default function unregisterNativeModule( projectConfig, iOSDependencies, ) { + logger.debug(`Reading ${projectConfig.pbxprojPath}`); const project = xcode.project(projectConfig.pbxprojPath).parseSync(); const dependencyProject = xcode .project(dependencyConfig.pbxprojPath) @@ -47,6 +49,11 @@ export default function unregisterNativeModule( removeProjectFromLibraries(libraries, file); getTargets(dependencyProject).forEach(target => { + logger.debug( + `Removing ${target.name} from ${ + project.getFirstTarget().firstTarget.name + }`, + ); removeFromStaticLibraries(project, target.name, { target: project.getFirstTarget().uuid, }); @@ -70,5 +77,6 @@ export default function unregisterNativeModule( ); } + logger.debug(`Writing changes to ${projectConfig.pbxprojPath}`); fs.writeFileSync(projectConfig.pbxprojPath, project.writeSync()); } diff --git a/packages/cli/src/commands/link/link.js b/packages/cli/src/commands/link/link.js index 13d6010775..b4538c9cea 100644 --- a/packages/cli/src/commands/link/link.js +++ b/packages/cli/src/commands/link/link.js @@ -20,7 +20,7 @@ import linkDependency from './linkDependency'; import linkAssets from './linkAssets'; import linkAll from './linkAll'; import findReactNativeScripts from '../../tools/findReactNativeScripts'; -import getPlatforms from '../../tools/getPlatforms'; +import getPlatforms, {getPlatformName} from '../../tools/getPlatforms'; type FlagsType = { platforms?: Array, @@ -37,9 +37,21 @@ function link([rawPackageName]: Array, ctx: ContextT, opts: FlagsType) { let project; try { platforms = getPlatforms(ctx.root); + logger.debug( + 'Available platforms: ' + + `${Object.getOwnPropertyNames(platforms) + .map(platform => getPlatformName(platform)) + .join(', ')}`, + ); if (opts.platforms) { platforms = pick(platforms, opts.platforms); } + logger.debug( + 'Targeted platforms: ' + + `${Object.getOwnPropertyNames(platforms) + .map(platform => getPlatformName(platform)) + .join(', ')}`, + ); project = getProjectConfig(ctx, platforms); } catch (err) { logger.error( @@ -62,9 +74,14 @@ function link([rawPackageName]: Array, ctx: ContextT, opts: FlagsType) { } if (rawPackageName === undefined) { + logger.debug( + 'No package name provided, will attemp to link all possible packages.', + ); return linkAll(ctx, platforms, project); } + logger.debug(`Package to link: ${rawPackageName}`); + // Trim the version / tag out of the package name (eg. package@latest) const packageName = rawPackageName.replace(/^(.+?)(@.+?)$/gi, '$1'); diff --git a/packages/cli/src/commands/link/pods/addPodEntry.js b/packages/cli/src/commands/link/pods/addPodEntry.js index 25ce4875ec..e1040759d1 100644 --- a/packages/cli/src/commands/link/pods/addPodEntry.js +++ b/packages/cli/src/commands/link/pods/addPodEntry.js @@ -7,6 +7,8 @@ * @format */ +import logger from '../../../tools/logger'; + export default function addPodEntry( podLines, linesToAddEntry, @@ -20,11 +22,13 @@ export default function addPodEntry( } if (Array.isArray(linesToAddEntry)) { - linesToAddEntry.map(({line, indentation}, idx) => - podLines.splice(line + idx, 0, getLineToAdd(newEntry, indentation)), - ); + linesToAddEntry.map(({line, indentation}, idx) => { + logger.debug(`Adding ${podName} to Pod file"`); + podLines.splice(line + idx, 0, getLineToAdd(newEntry, indentation)); + }); } else { const {line, indentation} = linesToAddEntry; + logger.debug(`Adding ${podName} to Pod file"`); podLines.splice(line, 0, getLineToAdd(newEntry, indentation)); } } diff --git a/packages/cli/src/commands/link/pods/readPodfile.js b/packages/cli/src/commands/link/pods/readPodfile.js index 42fc6be4aa..8dd4193254 100644 --- a/packages/cli/src/commands/link/pods/readPodfile.js +++ b/packages/cli/src/commands/link/pods/readPodfile.js @@ -8,8 +8,10 @@ */ import fs from 'fs'; +import logger from '../../../tools/logger'; export default function readPodfile(podfilePath) { + logger.debug(`Reading ${podfilePath}`); const podContent = fs.readFileSync(podfilePath, 'utf8'); return podContent.split(/\r?\n/g); } diff --git a/packages/cli/src/commands/link/pods/removePodEntry.js b/packages/cli/src/commands/link/pods/removePodEntry.js index 1de92fe9be..eafa7b34fc 100644 --- a/packages/cli/src/commands/link/pods/removePodEntry.js +++ b/packages/cli/src/commands/link/pods/removePodEntry.js @@ -7,11 +7,14 @@ * @format */ +import logger from '../../../tools/logger'; + export default function removePodEntry(podfileContent, podName) { // this regex should catch line(s) with full pod definition, like: pod 'podname', :path => '../node_modules/podname', :subspecs => ['Sub2', 'Sub1'] const podRegex = new RegExp( `\\n( |\\t)*pod\\s+("|')${podName}("|')(,\\s*(:[a-z]+\\s*=>)?\\s*(("|').*?("|')|\\[[\\s\\S]*?\\]))*\\n`, 'g', ); + logger.debug(`Removing ${podName} from Pod file`); return podfileContent.replace(podRegex, '\n'); } diff --git a/packages/cli/src/commands/link/pods/savePodFile.js b/packages/cli/src/commands/link/pods/savePodFile.js index 5148cc7749..4cfaba26eb 100644 --- a/packages/cli/src/commands/link/pods/savePodFile.js +++ b/packages/cli/src/commands/link/pods/savePodFile.js @@ -8,8 +8,10 @@ */ import fs from 'fs'; +import logger from '../../../tools/logger'; export default function savePodFile(podfilePath, podLines) { const newPodfile = podLines.join('\n'); + logger.debug(`Writing changes to ${podfilePath}`); fs.writeFileSync(podfilePath, newPodfile); } diff --git a/packages/cli/src/commands/link/pods/unregisterNativeModule.js b/packages/cli/src/commands/link/pods/unregisterNativeModule.js index 9e42b5e6fa..96abbd0e50 100644 --- a/packages/cli/src/commands/link/pods/unregisterNativeModule.js +++ b/packages/cli/src/commands/link/pods/unregisterNativeModule.js @@ -9,6 +9,7 @@ import fs from 'fs'; import removePodEntry from './removePodEntry'; +import logger from '../../../tools/logger'; /** * Unregister native module IOS with CocoaPods @@ -16,5 +17,6 @@ import removePodEntry from './removePodEntry'; export default function unregisterNativeModule(dependencyConfig, iOSProject) { const podContent = fs.readFileSync(iOSProject.podfile, 'utf8'); const removed = removePodEntry(podContent, dependencyConfig.podspec); + logger.debug(`Writing changes to ${iOSProject.podfile}`); fs.writeFileSync(iOSProject.podfile, removed); } diff --git a/packages/cli/src/tools/logger.js b/packages/cli/src/tools/logger.js index 35aeff6815..42e6e262f2 100644 --- a/packages/cli/src/tools/logger.js +++ b/packages/cli/src/tools/logger.js @@ -5,6 +5,8 @@ import chalk from 'chalk'; const SEPARATOR = ', '; +let verbose = false; + const formatMessages = (messages: Array) => chalk.reset(messages.join(SEPARATOR)); @@ -25,13 +27,19 @@ const error = (...messages: Array) => { }; const debug = (...messages: Array) => { - console.log(`${chalk.gray.bold('debug')} ${formatMessages(messages)}`); + if (verbose) { + console.log(`${chalk.gray.bold('debug')} ${formatMessages(messages)}`); + } }; const log = (...messages: Array) => { console.log(`${formatMessages(messages)}`); }; +const setVerbose = (level: boolean) => { + verbose = level; +}; + export default { success, info, @@ -39,4 +47,5 @@ export default { error, debug, log, + setVerbose, };