Skip to content

Commit

Permalink
Add verbose logging flag, debug logging for (un)link commands (#239)
Browse files Browse the repository at this point in the history
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`:
<img width="500" alt="Screenshot 2019-03-15 at 22 07 26" src="https://user-images.githubusercontent.com/1713151/54462920-a2d11a00-4771-11e9-812a-00972da67f1a.png">

How `link packageName --verbose` looks like:
<img width="816" alt="Screenshot 2019-03-15 at 22 09 08" src="https://user-images.githubusercontent.com/1713151/54463021-f2afe100-4771-11e9-9213-d1f783c1f0d5.png">

How `link packageName --verbose` looks like (module with assets):
<img width="789" alt="Screenshot 2019-03-15 at 22 33 28" src="https://user-images.githubusercontent.com/1713151/54463179-7b2e8180-4772-11e9-9547-1e8907d6151d.png">

How `unlink packageName --verbose` looks like:
<img width="803" alt="Screenshot 2019-03-15 at 22 09 34" src="https://user-images.githubusercontent.com/1713151/54463217-a022f480-4772-11e9-8130-9824bb0bb0f9.png">
<img width="805" alt="Screenshot 2019-03-15 at 22 13 58" src="https://user-images.githubusercontent.com/1713151/54463227-a618d580-4772-11e9-9420-315e35d3d55c.png">
  • Loading branch information
matei-radu authored and thymikee committed Mar 17, 2019
1 parent 3eebe31 commit 79f7d4e
Show file tree
Hide file tree
Showing 19 changed files with 103 additions and 16 deletions.
5 changes: 4 additions & 1 deletion packages/cli/src/cliEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(' '));
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/commands/link/android/copyAssets.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)));
Expand Down
5 changes: 5 additions & 0 deletions packages/cli/src/commands/link/android/patches/applyPatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions packages/cli/src/commands/link/android/patches/revokePatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, ''),
Expand Down
9 changes: 8 additions & 1 deletion packages/cli/src/commands/link/android/unlinkAssets.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,35 @@
* 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
*
* 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<string>,
project: {assetsPath: string},
) {
const assets = groupFilesByType(files);

logger.debug(`Assets path: ${project.assetsPath}`);
(assets.font || []).forEach(file => {
const filePath = path.join(
project.assetsPath,
'fonts',
path.basename(file),
);
if (fs.existsSync(filePath)) {
logger.debug(`Removing asset ${filePath}`);
fs.unlinkSync(filePath);
}
});
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/commands/link/getProjectConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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] || {},
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/commands/link/ios/addToHeaderSearchPaths.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
13 changes: 8 additions & 5 deletions packages/cli/src/commands/link/ios/copyAssets.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}
Expand Down
5 changes: 5 additions & 0 deletions packages/cli/src/commands/link/ios/registerNativeModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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,
});
Expand All @@ -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,
});
Expand All @@ -83,5 +87,6 @@ export default function registerNativeModuleIOS(
);
}

logger.debug(`Writing changes to ${projectConfig.pbxprojPath}`);
fs.writeFileSync(projectConfig.pbxprojPath, project.writeSync());
}
Original file line number Diff line number Diff line change
Expand Up @@ -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),
);
Expand Down
10 changes: 6 additions & 4 deletions packages/cli/src/commands/link/ios/unlinkAssets.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions packages/cli/src/commands/link/ios/unregisterNativeModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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,
});
Expand All @@ -70,5 +77,6 @@ export default function unregisterNativeModule(
);
}

logger.debug(`Writing changes to ${projectConfig.pbxprojPath}`);
fs.writeFileSync(projectConfig.pbxprojPath, project.writeSync());
}
19 changes: 18 additions & 1 deletion packages/cli/src/commands/link/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>,
Expand All @@ -37,9 +37,21 @@ function link([rawPackageName]: Array<string>, 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(
Expand All @@ -62,9 +74,14 @@ function link([rawPackageName]: Array<string>, 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');

Expand Down
10 changes: 7 additions & 3 deletions packages/cli/src/commands/link/pods/addPodEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @format
*/

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

export default function addPodEntry(
podLines,
linesToAddEntry,
Expand All @@ -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));
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/commands/link/pods/readPodfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
3 changes: 3 additions & 0 deletions packages/cli/src/commands/link/pods/removePodEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
2 changes: 2 additions & 0 deletions packages/cli/src/commands/link/pods/savePodFile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 2 additions & 0 deletions packages/cli/src/commands/link/pods/unregisterNativeModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@

import fs from 'fs';
import removePodEntry from './removePodEntry';
import logger from '../../../tools/logger';

/**
* Unregister native module IOS with CocoaPods
*/
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);
}
Loading

0 comments on commit 79f7d4e

Please sign in to comment.