Skip to content

Plamen5kov/remove obsolete code #2461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 2, 2017
Merged

Plamen5kov/remove obsolete code #2461

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e8b7d3
refactoring
Plamen5kov Jan 30, 2017
1d1a1aa
lint
Plamen5kov Jan 30, 2017
7496bd4
update after review
Plamen5kov Jan 31, 2017
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
3 changes: 0 additions & 3 deletions lib/declarations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ interface IOptions extends ICommonOptions {
baseConfig: string;
client: boolean;
compileSdk: number;
copyFrom: string;
copyTo: string;
debugTransport: boolean;
emulator: boolean;
Expand All @@ -81,7 +80,6 @@ interface IOptions extends ICommonOptions {
keyStoreAliasPassword: string;
keyStorePassword: string;
keyStorePath: string;
linkTo: string;
ng: boolean;
tsc: boolean;
androidTypings: boolean;
Expand All @@ -90,7 +88,6 @@ interface IOptions extends ICommonOptions {
port: Number;
production: boolean; //npm flag
sdk: string;
tnsModulesVersion: string;
teamId: string;
syncAllFiles: boolean;
liveEdit: boolean;
Expand Down
2 changes: 1 addition & 1 deletion lib/npm-installation-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class NpmInstallationManager implements INpmInstallationManager {
let cliVersionRange = `~${this.$staticConfig.version}`;
let latestVersion = await this.getLatestVersion(packageName);
if (semver.satisfies(latestVersion, cliVersionRange)) {
return latestVersion;
return Promise.resolve(latestVersion);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this changed? When you mark method with async keyword, the returned value is always wrapped in Promise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't know that, will revert it to previous state

}
let data = await this.$npm.view(packageName, { json: true, "versions": true });

Expand Down
3 changes: 0 additions & 3 deletions lib/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ export class Options extends commonOptionsLibPath.OptionsBase {
frameworkName: { type: OptionType.String },
framework: { type: OptionType.String },
frameworkVersion: { type: OptionType.String },
copyFrom: { type: OptionType.String },
linkTo: { type: OptionType.String },
forDevice: { type: OptionType.Boolean },
provision: { type: OptionType.Object },
client: { type: OptionType.Boolean, default: true },
Expand All @@ -27,7 +25,6 @@ export class Options extends commonOptionsLibPath.OptionsBase {
keyStoreAliasPassword: { type: OptionType.String },
ignoreScripts: { type: OptionType.Boolean },
disableNpmInstall: { type: OptionType.Boolean },
tnsModulesVersion: { type: OptionType.String },
compileSdk: { type: OptionType.Number },
port: { type: OptionType.Number },
copyTo: { type: OptionType.String },
Expand Down
2 changes: 1 addition & 1 deletion lib/services/init-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class InitService implements IInitService {

// In case console is interactive and --force is not specified, do not read the version from package.json, show all available versions to the user.
let tnsCoreModulesVersionInPackageJson = this.useDefaultValue ? projectData.dependencies[constants.TNS_CORE_MODULES_NAME] : null;
projectData.dependencies[constants.TNS_CORE_MODULES_NAME] = this.$options.tnsModulesVersion || tnsCoreModulesVersionInPackageJson || (await this.getVersionData(constants.TNS_CORE_MODULES_NAME))["version"];
projectData.dependencies[constants.TNS_CORE_MODULES_NAME] = tnsCoreModulesVersionInPackageJson || (await this.getVersionData(constants.TNS_CORE_MODULES_NAME))["version"];

this.$fs.writeJson(this.projectFilePath, projectData);
} catch (err) {
Expand Down
108 changes: 26 additions & 82 deletions lib/services/project-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as constants from "../constants";
import * as osenv from "osenv";
import * as path from "path";
import * as shelljs from "shelljs";

Expand All @@ -19,72 +18,36 @@ export class ProjectService implements IProjectService {
if (!projectName) {
this.$errors.fail("You must specify <App name> when creating a new project.");
}

projectName = await this.$projectNameService.ensureValidName(projectName, { force: this.$options.force });

let projectId = this.$options.appid || this.$projectHelper.generateDefaultAppId(projectName, constants.DEFAULT_APP_IDENTIFIER_PREFIX);

let projectDir = path.join(path.resolve(this.$options.path || "."), projectName);
this.$fs.createDirectory(projectDir);
if (this.$fs.exists(projectDir) && !this.$fs.isEmptyDir(projectDir)) {
this.$errors.fail("Path already exists and is not empty %s", projectDir);
}

let projectId = this.$options.appid || this.$projectHelper.generateDefaultAppId(projectName, constants.DEFAULT_APP_IDENTIFIER_PREFIX);
this.createPackageJson(projectDir, projectId);

let customAppPath = this.getCustomAppPath();
if (customAppPath) {
customAppPath = path.resolve(customAppPath);
if (!this.$fs.exists(customAppPath)) {
this.$errors.failWithoutHelp(`The specified path "${customAppPath}" doesn't exist. Check that you specified the path correctly and try again.`);
}

let customAppContents = this.$fs.enumerateFilesInDirectorySync(customAppPath);
if (customAppContents.length === 0) {
this.$errors.failWithoutHelp(`The specified path "${customAppPath}" is empty directory.`);
}
this.$logger.trace(`Creating a new NativeScript project with name ${projectName} and id ${projectId} at location ${projectDir}`);
if (!selectedTemplate) {
selectedTemplate = constants.RESERVED_TEMPLATE_NAMES["default"];
}
let templatePath = await this.$projectTemplatesService.prepareTemplate(selectedTemplate, projectDir);
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe all of these operations should be in the try-catch block below.

await this.extractTemplate(projectDir, templatePath);

this.$logger.trace("Creating a new NativeScript project with name %s and id %s at location %s", projectName, projectId, projectDir);

let projectAppDirectory = path.join(projectDir, constants.APP_FOLDER_NAME);
let appPath: string = null;
if (customAppPath) {
this.$logger.trace("Using custom app from %s", customAppPath);

// Make sure that the source app/ is not a direct ancestor of a target app/
let relativePathFromSourceToTarget = path.relative(customAppPath, projectAppDirectory);
// path.relative returns second argument if the paths are located on different disks
// so in this case we don't need to make the check for direct ancestor
if (relativePathFromSourceToTarget !== projectAppDirectory) {
let doesRelativePathGoUpAtLeastOneDir = relativePathFromSourceToTarget.split(path.sep)[0] === "..";
if (!doesRelativePathGoUpAtLeastOneDir) {
this.$errors.fail("Project dir %s must not be created at/inside the template used to create the project %s.", projectDir, customAppPath);
}
}
this.$logger.trace("Copying custom app into %s", projectAppDirectory);
appPath = customAppPath;
} else {
let templatePath = await this.$projectTemplatesService.prepareTemplate(selectedTemplate, projectDir);
this.$logger.trace(`Copying application from '${templatePath}' into '${projectAppDirectory}'.`);
let templatePackageJson = this.$fs.readJson(path.join(templatePath, "package.json"));
selectedTemplate = templatePackageJson.name;
appPath = templatePath;
}
let packageName = constants.TNS_CORE_MODULES_NAME;
await this.$npm.install(packageName, projectDir, { save: true, "save-exact": true });

try {
//TODO: plamen5kov: move copy of template and npm uninstall in prepareTemplate logic
await this.createProjectCore(projectDir, appPath, projectId);
let templatePackageJsonData = this.getDataFromJson(appPath);
let templatePackageJsonData = this.getDataFromJson(templatePath);
this.mergeProjectAndTemplateProperties(projectDir, templatePackageJsonData); //merging dependencies from template (dev && prod)
this.removeMergedDependencies(projectDir, templatePackageJsonData);

await this.$npm.install(projectDir, projectDir, { "ignore-scripts": this.$options.ignoreScripts });
selectedTemplate = selectedTemplate || "";
let templateName = (constants.RESERVED_TEMPLATE_NAMES[selectedTemplate.toLowerCase()] || selectedTemplate/*user template*/) || constants.RESERVED_TEMPLATE_NAMES["default"];
await this.$npm.uninstall(selectedTemplate, { save: true }, projectDir);

// TODO: plamen5kov: remove later (put only so tests pass (need to fix tests))
this.$logger.trace(`Using NativeScript verified template: ${templateName} with version undefined.`);
let templatePackageJson = this.$fs.readJson(path.join(templatePath, "package.json"));
await this.$npm.uninstall(templatePackageJson.name, { save: true }, projectDir);
} catch (err) {
this.$fs.deleteDirectory(projectDir);
throw err;
Expand All @@ -103,6 +66,18 @@ export class ProjectService implements IProjectService {
return null;
}

private async extractTemplate(projectDir: string, realTemplatePath: string): Promise<void> {
this.$fs.ensureDirectoryExists(projectDir);

let appDestinationPath = path.join(projectDir, constants.APP_FOLDER_NAME);
this.$fs.createDirectory(appDestinationPath);

this.$logger.trace(`Copying application from '${realTemplatePath}' into '${appDestinationPath}'.`);
shelljs.cp('-R', path.join(realTemplatePath, "*"), appDestinationPath);

this.$fs.createDirectory(path.join(projectDir, "platforms"));
}

private removeMergedDependencies(projectDir: string, templatePackageJsonData: any): void {
let extractedTemplatePackageJsonPath = path.join(projectDir, constants.APP_FOLDER_NAME, constants.PACKAGE_JSON_FILE_NAME);
for (let key in templatePackageJsonData) {
Expand All @@ -129,6 +104,8 @@ export class ProjectService implements IProjectService {
}
this.$logger.trace("New project package.json data: ", projectPackageJsonData);
this.$fs.writeJson(projectPackageJsonPath, projectPackageJsonData);
} else {
this.$errors.failWithoutHelp(`Couldn't find package.json data in installed template`);
}
}

Expand All @@ -147,42 +124,9 @@ export class ProjectService implements IProjectService {
return sortedDeps;
}

private async createProjectCore(projectDir: string, appSourcePath: string, projectId: string): Promise<void> {
this.$fs.ensureDirectoryExists(projectDir);

let appDestinationPath = path.join(projectDir, constants.APP_FOLDER_NAME);
this.$fs.createDirectory(appDestinationPath);

shelljs.cp('-R', path.join(appSourcePath, "*"), appDestinationPath);

this.$fs.createDirectory(path.join(projectDir, "platforms"));

let tnsModulesVersion = this.$options.tnsModulesVersion;
let packageName = constants.TNS_CORE_MODULES_NAME;
if (tnsModulesVersion) {
packageName = `${packageName}@${tnsModulesVersion}`;
}
await this.$npm.install(packageName, projectDir, { save: true, "save-exact": true });
}

private createPackageJson(projectDir: string, projectId: string): void {
this.$projectDataService.initialize(projectDir);
this.$projectDataService.setValue("id", projectId);
}

private getCustomAppPath(): string {
let customAppPath = this.$options.copyFrom || this.$options.linkTo;
if (customAppPath) {
if (customAppPath.indexOf("http://") === 0) {
this.$errors.fail("Only local paths for custom app are supported.");
}

if (customAppPath.substr(0, 1) === '~') {
customAppPath = path.join(osenv.home(), customAppPath.substr(1));
}
}

return customAppPath;
}
}
$injector.register("projectService", ProjectService);
33 changes: 12 additions & 21 deletions lib/services/project-templates-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,28 @@ temp.track();

export class ProjectTemplatesService implements IProjectTemplatesService {

public constructor(private $errors: IErrors,
private $fs: IFileSystem,
public constructor(private $fs: IFileSystem,
private $logger: ILogger,
private $npmInstallationManager: INpmInstallationManager) { }

public async prepareTemplate(originalTemplateName: string, projectDir: string): Promise<string> {
let realTemplatePath: string;
if (originalTemplateName) {
// support <reserved_name>@<version> syntax
let data = originalTemplateName.split("@"),
name = data[0],
version = data[1];
// support <reserved_name>@<version> syntax
let data = originalTemplateName.split("@"),
name = data[0],
version = data[1];

if (constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()]) {
realTemplatePath = await this.prepareNativeScriptTemplate(constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()], version, projectDir);
} else {
// Use the original template name, specified by user as it may be case-sensitive.
realTemplatePath = await this.prepareNativeScriptTemplate(name, version, projectDir);
}
if (constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()]) {
realTemplatePath = await this.prepareNativeScriptTemplate(constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()], version, projectDir);
} else {
realTemplatePath = await this.prepareNativeScriptTemplate(constants.RESERVED_TEMPLATE_NAMES["default"], null/*version*/, projectDir);
// Use the original template name, specified by user as it may be case-sensitive.
realTemplatePath = await this.prepareNativeScriptTemplate(originalTemplateName, version, projectDir);
}

if (realTemplatePath) {
//this removes dependencies from templates so they are not copied to app folder
this.$fs.deleteDirectory(path.join(realTemplatePath, constants.NODE_MODULES_FOLDER_NAME));
return realTemplatePath;
}
//this removes dependencies from templates so they are not copied to app folder
this.$fs.deleteDirectory(path.join(realTemplatePath, constants.NODE_MODULES_FOLDER_NAME));

this.$errors.failWithoutHelp("Unable to find the template in temp directory. " +
`Please open an issue at https://github.com/NativeScript/nativescript-cli/issues and send the output of the same command executed with --log trace.`);
return realTemplatePath;
}

/**
Expand Down
7 changes: 5 additions & 2 deletions test/project-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class ProjectIntegrationTest {

public async createProject(projectName: string, template?: string): Promise<void> {
let projectService = this.testInjector.resolve("projectService");
if (!template) {
template = constants.RESERVED_TEMPLATE_NAMES["default"];
}
return projectService.createProject(projectName, template);
}

Expand Down Expand Up @@ -67,7 +70,7 @@ class ProjectIntegrationTest {
let tnsCoreModulesRecord = packageJsonContent["dependencies"][constants.TNS_CORE_MODULES_NAME];
assert.isTrue(tnsCoreModulesRecord !== null);

let sourceDir = projectSourceDirectory || options.copyFrom;
let sourceDir = projectSourceDirectory || options.template;

let expectedFiles = fs.enumerateFilesInDirectorySync(sourceDir);
let actualFiles = fs.enumerateFilesInDirectorySync(appDirectoryPath);
Expand Down Expand Up @@ -436,7 +439,7 @@ describe("Project Service Tests", () => {
options.path = tempFolder;

await projectIntegrationTest.createProject(projectName);
options.copyFrom = defaultTemplatePath;
options.template = defaultTemplatePath;

await projectIntegrationTest.assertProject(tempFolder, projectName, `org.nativescript.${projectName}`, null);
});
Expand Down
3 changes: 2 additions & 1 deletion test/project-templates-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ProjectTemplatesService } from "../lib/services/project-templates-servi
import { assert } from "chai";
import * as path from "path";
import temp = require("temp");
import * as constants from "../lib/constants";

let isDeleteDirectoryCalledForNodeModulesDir = false;
let nativeScriptValidatedTemplatePath = "nsValidatedTemplatePath";
Expand Down Expand Up @@ -92,7 +93,7 @@ describe("project-templates-service", () => {
testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
let tempFolder = temp.mkdirSync("preparetemplate");
let actualPathToTemplate = await projectTemplatesService.prepareTemplate(undefined, tempFolder);
let actualPathToTemplate = await projectTemplatesService.prepareTemplate(constants.RESERVED_TEMPLATE_NAMES["default"], tempFolder);
assert.strictEqual(path.basename(actualPathToTemplate), nativeScriptValidatedTemplatePath);
assert.strictEqual(isDeleteDirectoryCalledForNodeModulesDir, true, "When correct path is returned, template's node_modules directory should be deleted.");
});
Expand Down