Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
f40353f
refactor: properly handle invalid template package
Aug 3, 2019
710d8fb
refactor: error checks when getting template name
Aug 3, 2019
1c69278
refactor: remove duplicate exception throw
Aug 3, 2019
30b0698
refactor: check if assets dest exists
Aug 3, 2019
e27b60b
refactor: add error checks when upgrading
Aug 3, 2019
9f6cea1
chore: add comment for using process.exit()
Aug 3, 2019
1d4b52c
refactor: improve error outputs in link command
Aug 3, 2019
b44a7e8
refactor: use for of loop
Aug 3, 2019
c4f1f46
refactor: check package before uninstalling
Aug 4, 2019
bc6c633
refactor: use CLIError instead of direct exit
Aug 4, 2019
d6f5657
refactor: handle potential android pkg name errors
Aug 4, 2019
a69ee5b
refactor: fix typos/mistakes in error outputs
Aug 4, 2019
f321431
refactor: add adb & android sdk path checks
Aug 5, 2019
389e60f
refactor: fix imports
Aug 5, 2019
8a138ce
refactor: improve install apk error outputs
Aug 5, 2019
207ea95
Apply suggestions from code review
Aug 5, 2019
6bf9dba
refactor: add init doc link in error output
Aug 5, 2019
168d679
test: make sure all tests pass
Aug 5, 2019
d851843
refactor: use try/catch around readFileSync
Aug 6, 2019
c0a2cac
refactor: do not read build.gradle twice
Aug 6, 2019
d073a04
refactor: clean up code in android linking
Aug 6, 2019
9087af4
fix: packageJson not defined in templateName
Aug 6, 2019
fbf5bd8
refactor: remove unintentional --verbose flag
Aug 6, 2019
af2b0cc
refactor: make flow very happy
Aug 6, 2019
f2f6e08
refactor: remove unnecessary variable definition
Aug 23, 2019
415bb25
pkgjson undefined tweak
thymikee Oct 11, 2019
879f9b8
eslint fix
thymikee Nov 28, 2019
e87dc75
remove adb/android_home checks because we have doctor
thymikee Nov 28, 2019
1e4ccb1
use mockImplementationOnce
thymikee Nov 28, 2019
a3b88f4
revert link changes, as it's going to be removed in next version
thymikee Nov 28, 2019
3091c13
inline fns
thymikee Nov 28, 2019
8b23e14
revert uninstall changes as we're removing that in next version
thymikee Nov 28, 2019
37d4466
adjust copy
thymikee Nov 28, 2019
ea4ffd8
mock existsSync once
thymikee Nov 28, 2019
398e7c4
remove unused code
thymikee Dec 2, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/cli/src/commands/bundle/assetPathUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function getAndroidAssetSuffix(scale: number): string {
case 4:
return 'xxxhdpi';
default:
throw new Error('no such scale');
return '';
}
}

Expand Down
8 changes: 7 additions & 1 deletion packages/cli/src/commands/bundle/saveAssets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import fs from 'fs';
import filterPlatformAssetScales from './filterPlatformAssetScales';
import getAssetDestPathAndroid from './getAssetDestPathAndroid';
import getAssetDestPathIOS from './getAssetDestPathIOS';
import {logger} from '@react-native-community/cli-tools';
import {logger, CLIError} from '@react-native-community/cli-tools';
import {AssetData} from './buildBundle';

interface CopiedFiles {
Expand All @@ -30,6 +30,12 @@ function saveAssets(
return Promise.resolve();
}

if (!fs.existsSync(assetsDest)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @thecodrr @thymikee @grabbou,
I have a situation that I need to run bundle and copy assets into new created folder based on assetsDest and I was relying on https://github.com/react-native-community/cli/pull/609/files#diff-e59437386ac414095e6ed43a3da37303R93 to create folder if not exists.
Any specific reason that it needs to check assetsDest first before copying assets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are calling mkdirp in the copy function, maybe this is no longer needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i think it should be safe using mkdirp before copy files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check can be safely removed then. What do you think @thymikee?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @thecodrr @thymikee will open the PR

throw new CLIError(
`The specified assets destination folder "${assetsDest}" does not exist.`,
);
}

const getAssetDestPath =
platform === 'android' ? getAssetDestPathAndroid : getAssetDestPathIOS;

Expand Down
7 changes: 4 additions & 3 deletions packages/cli/src/commands/init/__tests__/template.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
jest.mock('execa', () => jest.fn());
import execa from 'execa';
import path from 'path';
import fs from 'fs';
import * as PackageManger from '../../../tools/packageManager';
import {
installTemplatePackage,
Expand Down Expand Up @@ -32,7 +33,7 @@ test('installTemplatePackage', async () => {

test('getTemplateConfig', () => {
jest.mock(
`${TEMPLATE_SOURCE_DIR}/node_modules/${TEMPLATE_NAME}/template.config`,
`${TEMPLATE_SOURCE_DIR}/node_modules/${TEMPLATE_NAME}/template.config.js`,
() => ({
placeholderName: 'someName',
templateDir: 'someDir',
Expand All @@ -42,7 +43,7 @@ test('getTemplateConfig', () => {
},
);
jest.spyOn(path, 'resolve').mockImplementationOnce((...e) => e.join('/'));

jest.spyOn(fs, 'existsSync').mockImplementationOnce(() => true);
expect(getTemplateConfig(TEMPLATE_NAME, TEMPLATE_SOURCE_DIR)).toEqual({
placeholderName: 'someName',
templateDir: 'someDir',
Expand All @@ -51,7 +52,7 @@ test('getTemplateConfig', () => {
TEMPLATE_SOURCE_DIR,
'node_modules',
TEMPLATE_NAME,
'template.config',
'template.config.js',
);
});

Expand Down
36 changes: 16 additions & 20 deletions packages/cli/src/commands/init/__tests__/templateName.test.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,23 @@
import {processTemplateName} from '../templateName';
import fs from 'fs';

const RN_NPM_PACKAGE = 'react-native';
const ABS_RN_PATH = '/path/to/react-native';
const ABS_RN_PATH_WINDOWS = 'path/to/react-native';

test('supports file protocol with absolute path', async () => {
if (process.platform === 'win32') {
console.warn('[SKIP] Jest virtual mocks seem to be broken on Windows');
return;
}
jest.mock(
`${ABS_RN_PATH}/package.json`,
() => ({
name: 'react-native',
}),
{virtual: true},
);
expect(await processTemplateName(`file://${ABS_RN_PATH}`)).toEqual({
uri: ABS_RN_PATH,
test('supports file protocol with absolute path', () => {
jest.spyOn(fs, 'existsSync').mockImplementationOnce(() => true);
jest
.spyOn(fs, 'readFileSync')
.mockImplementationOnce(() => JSON.stringify({name: 'react-native'}));
expect(processTemplateName(`file://${ABS_RN_PATH}`)).toEqual({
uri: process.platform === 'win32' ? ABS_RN_PATH_WINDOWS : ABS_RN_PATH,
name: RN_NPM_PACKAGE,
});
});

test('supports npm packages as template names', async () => {
expect(await processTemplateName(RN_NPM_PACKAGE)).toEqual({
test('supports npm packages as template names', () => {
expect(processTemplateName(RN_NPM_PACKAGE)).toEqual({
uri: RN_NPM_PACKAGE,
name: RN_NPM_PACKAGE,
});
Expand All @@ -37,15 +32,16 @@ test.each`
${'@scoped/name@tag'} | ${'@scoped/name@tag'} | ${'@scoped/name'}
`(
'supports versioned npm package "$templateName" as template name',
async ({templateName, uri, name}) => {
expect(await processTemplateName(templateName)).toEqual({uri, name});
({templateName, uri, name}) => {
expect(processTemplateName(templateName)).toEqual({uri, name});
},
);

test('supports path to tgz archives', async () => {
test('supports path to tgz archives', () => {
const ABS_RN_TARBALL_PATH =
'/path/to/react-native/react-native-1.2.3-rc.0.tgz';
expect(await processTemplateName(`file://${ABS_RN_TARBALL_PATH}`)).toEqual({
jest.spyOn(fs, 'existsSync').mockImplementationOnce(() => true);
expect(processTemplateName(`file://${ABS_RN_TARBALL_PATH}`)).toEqual({
uri: `file://${ABS_RN_TARBALL_PATH}`,
name: 'react-native',
});
Expand Down
17 changes: 13 additions & 4 deletions packages/cli/src/commands/init/template.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import execa from 'execa';
import path from 'path';
import {logger} from '@react-native-community/cli-tools';
import {logger, CLIError} from '@react-native-community/cli-tools';
import * as PackageManager from '../../tools/packageManager';
import copyFiles from '../../tools/copyFiles';
import replacePathSepForRegex from '../../tools/replacePathSepForRegex';
import fs from 'fs';
import chalk from 'chalk';

export type TemplateConfig = {
placeholderName: string;
Expand Down Expand Up @@ -33,11 +35,18 @@ export function getTemplateConfig(
templateSourceDir,
'node_modules',
templateName,
'template.config',
'template.config.js',
);

logger.debug(`Getting config from ${configFilePath}.js`);

logger.debug(`Getting config from ${configFilePath}`);
if (!fs.existsSync(configFilePath)) {
throw new CLIError(
`Couldn't find the "${configFilePath} file inside "${templateName}" template. Please make sure the template is valid.
Read more: ${chalk.underline.dim(
'https://github.com/react-native-community/cli/blob/master/docs/init.md#creating-custom-template',
)}`,
);
}
return require(configFilePath);
}

Expand Down
41 changes: 36 additions & 5 deletions packages/cli/src/commands/init/templateName.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import path from 'path';
import {URL} from 'url';
import fs from 'fs';
import {CLIError} from '@react-native-community/cli-tools';

const FILE_PROTOCOL = /file:/;
const TARBALL = /\.tgz$/;
Expand All @@ -9,20 +11,49 @@ const VERSIONED_PACKAGE = /(@?.+)(@)(.+)/;
function handleFileProtocol(filePath: string) {
let uri = new URL(filePath).pathname;
if (process.platform === 'win32') {
// On Windows, the pathname has an extra leading / so remove that
// On Windows, the pathname has an extra / at the start, so remove that
uri = uri.substring(1);
}
if (!fs.existsSync(uri)) {
throw new CLIError(
`Failed to retrieve template name. The specified template directory path "${uri}" does not exist or is invalid.`,
);
}
const packageJsonPath = path.join(uri, 'package.json');
let packageJson;
try {
packageJson = JSON.parse(
fs.readFileSync(packageJsonPath, {encoding: 'utf8'}),
);
} catch {
throw new CLIError(
'Failed to retrieve template name. We expect the template directory to include "package.json" file, but it was not found.',
);
}

if (!packageJson || !packageJson.name) {
throw new CLIError(
`Failed to retrieve template name. We expect the "package.json" of the template to include the "name" property, but we found "${
packageJson ? packageJson.name : 'undefined'
}" which is invalid.`,
);
}
return {
uri,
name: require(path.join(uri, 'package.json')).name,
name: packageJson.name,
};
}

function handleTarball(filePath: string) {
if (!fs.existsSync(filePath)) {
throw new CLIError(
`Failed to retrieve tarball name. The specified tarball path "${filePath}" does not exist or is invalid.`,
);
}
const nameWithVersion = path.parse(path.basename(filePath)).name;
const tarballVersionMatch = nameWithVersion.match(VERSION_POSTFIX);
if (!tarballVersionMatch) {
throw new Error(
throw new CLIError(
`Failed to retrieve tarball name. We expect the tarball to include package name and version, e.g.: "template-name-1.2.3-rc.0.tgz", but received: "${nameWithVersion}".`,
);
}
Expand All @@ -36,7 +67,7 @@ function handleTarball(filePath: string) {
function handleVersionedPackage(versionedPackage: string) {
const versionedPackageMatch = versionedPackage.match(VERSIONED_PACKAGE);
if (!versionedPackageMatch) {
throw new Error(
throw new CLIError(
`Failed to retrieve package name. We expect the package to include name and version, e.g.: "template-name@1.2.3-rc.0", but received: "${versionedPackage}".`,
);
}
Expand All @@ -46,7 +77,7 @@ function handleVersionedPackage(versionedPackage: string) {
};
}

export async function processTemplateName(templateName: string) {
export function processTemplateName(templateName: string) {
if (templateName.match(TARBALL)) {
return handleTarball(templateName);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/link/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ async function link(
);

if (rawPackageName === undefined) {
logger.debug('No package name provided, will linking all possible assets.');
logger.debug('No package name provided, will link all possible assets.');
return linkAll(ctx, {linkDeps: opts.all, linkAssets: true});
}

Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/link/linkAll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ async function linkAll(config: Config, options: Options) {
logger.info(
`Linking dependencies using "${chalk.bold(
'link',
)}" command is now legacy and likely unnecessary. We encourage you to try ${chalk.bold(
)}" command is now legacy and most likely unnecessary. We encourage you to try ${chalk.bold(
'autolinking',
)} that comes with React Native v0.60 default template. Autolinking happens at build time – during CocoaPods install or Gradle install phase. More information: ${chalk.dim.underline(
'https://github.com/react-native-community/cli/blob/master/docs/autolinking.md',
Expand Down
36 changes: 33 additions & 3 deletions packages/cli/src/commands/upgrade/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,52 @@ const webDiffUrl = 'https://react-native-community.github.io/upgrade-helper';
const rawDiffUrl =
'https://raw.githubusercontent.com/react-native-community/rn-diff-purge/diffs/diffs';

const isConnected = (output: string): boolean => {
// there is no reliable way of checking for internet connectivity, so we should just
// read the output from npm (to check for connectivity errors) which is faster and relatively more reliable.
return !output.includes('the host is inaccessible');
};

const checkForErrors = (output: string): void => {
if (!output) {
return;
}
if (!isConnected(output)) {
throw new CLIError(
'Upgrade failed. You do not seem to have an internet connection.',
);
}

if (output.includes('npm ERR')) {
throw new CLIError(`Upgrade failed with the following errors:\n${output}`);
}

if (output.includes('npm WARN')) {
logger.warn(output);
}
};

const getLatestRNVersion = async (): Promise<string> => {
logger.info('No version passed. Fetching latest...');
const {stdout} = await execa('npm', ['info', 'react-native', 'version']);
const {stdout, stderr} = await execa('npm', [
'info',
'react-native',
'version',
]);
checkForErrors(stderr);
return stdout;
};

const getRNPeerDeps = async (
version: string,
): Promise<{[key: string]: string}> => {
const {stdout} = await execa('npm', [
const {stdout, stderr} = await execa('npm', [
'info',
`react-native@${version}`,
'peerDependencies',
'--json',
]);

checkForErrors(stderr);
return JSON.parse(stdout);
};

Expand Down
Loading