From 19830623da835a1b5336b58f17607cbb6b490313 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 23 Apr 2018 00:06:34 +0300 Subject: [PATCH 1/4] feat: Allow templates to be full applications Currently when a project is created the content of the template is placed inside the `app` directory of the newly created project. This leads to some issues when you want to support more complex scenarios, for example it is difficult to add configuration file (like nsconfig.json or webpack.config.js) in the root of the project. The suggested solution to allow templates to be the full application is to check the template from which the application is created. In case the template contains a nativescript key and templateVersion property in it, check its value. In case it is v1, use the old way, i.e. place the content of the template in the app directory of the created project. In case it is v2 place the content of the template at the root of the application. In case it is anything else - throw an error. In case it is missing, use v1 as default. The solution ensures backwards compatiblity with existing templates and allows creating new types of templates. --- lib/constants.ts | 12 ++ lib/definitions/project.d.ts | 9 +- lib/services/project-service.ts | 96 +++++++++++--- lib/services/project-templates-service.ts | 43 +++++- test/project-templates-service.ts | 155 ++++++++++++++-------- test/stubs.ts | 4 +- 6 files changed, 231 insertions(+), 88 deletions(-) diff --git a/lib/constants.ts b/lib/constants.ts index 6a52e9788e..fdaa683893 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -137,6 +137,7 @@ export const enum DebugTools { export const enum TrackActionNames { Build = "Build", CreateProject = "Create project", + UsingTemplate = "Using Template", Debug = "Debug", Deploy = "Deploy", LiveSync = "LiveSync", @@ -145,6 +146,8 @@ export const enum TrackActionNames { CheckEnvironmentRequirements = "Check Environment Requirements" } +export const AnalyticsEventLabelDelimiter = "__"; + export const enum BuildStates { Clean = "Clean", Incremental = "Incremental" @@ -189,3 +192,12 @@ export class SubscribeForNewsletterMessages { public static ReviewPrivacyPolicyMsg = `You can review the Progress Software Privacy Policy at \`${PROGRESS_PRIVACY_POLICY_URL}\``; public static PromptMsg = "Input your e-mail address to agree".green + " or " + "leave empty to decline".red.bold + ":"; } + +export class TemplateVersions { + public static v1 = "v1"; + public static v2 = "v2"; +} + +export class ProjectTemplateErrors { + public static InvalidTemplateVersionStringFormat = "The template '%s' has a NativeScript version '%s' that is not supported. Unable to create project from it."; +} diff --git a/lib/definitions/project.d.ts b/lib/definitions/project.d.ts index 4ec6eb1d49..2ce4c8c5ef 100644 --- a/lib/definitions/project.d.ts +++ b/lib/definitions/project.d.ts @@ -201,6 +201,11 @@ interface IImageDefinitionsStructure { android: IImageDefinitionGroup; } +interface ITemplateData { + templatePath: string; + templateVersion: string; +} + /** * Describes working with templates. */ @@ -211,9 +216,9 @@ interface IProjectTemplatesService { * In case templateName is a special word, validated from us (for ex. typescript), resolve the real template name and add it to npm cache. * In any other cases try to `npm install` the specified templateName to temp directory. * @param {string} templateName The name of the template. - * @return {string} Path to the directory where extracted template can be found. + * @return {ITemplateData} Data describing the template - location where it is installed and its NativeScript version. */ - prepareTemplate(templateName: string, projectDir: string): Promise; + prepareTemplate(templateName: string, projectDir: string): Promise; } interface IPlatformProjectServiceBase { diff --git a/lib/services/project-service.ts b/lib/services/project-service.ts index 0e4146efed..c4ab580b4e 100644 --- a/lib/services/project-service.ts +++ b/lib/services/project-service.ts @@ -46,8 +46,8 @@ export class ProjectService implements IProjectService { } try { - const templatePath = await this.$projectTemplatesService.prepareTemplate(selectedTemplate, projectDir); - await this.extractTemplate(projectDir, templatePath); + const { templatePath, templateVersion } = await this.$projectTemplatesService.prepareTemplate(selectedTemplate, projectDir); + await this.extractTemplate(projectDir, templatePath, templateVersion); await this.ensureAppResourcesExist(projectDir); @@ -57,17 +57,24 @@ export class ProjectService implements IProjectService { await this.$npmInstallationManager.install(constants.TNS_CORE_MODULES_NAME, projectDir, { dependencyType: "save" }); } - this.mergeProjectAndTemplateProperties(projectDir, templatePackageJsonData); //merging dependencies from template (dev && prod) - this.removeMergedDependencies(projectDir, templatePackageJsonData); + if (templateVersion === constants.TemplateVersions.v1) { + this.mergeProjectAndTemplateProperties(projectDir, templatePackageJsonData); // merging dependencies from template (dev && prod) + this.removeMergedDependencies(projectDir, templatePackageJsonData); + } + + const templatePackageJson = this.$fs.readJson(path.join(templatePath, constants.PACKAGE_JSON_FILE_NAME)); + // Install devDependencies and execute all scripts: await this.$npm.install(projectDir, projectDir, { disableNpmInstall: false, frameworkPath: null, ignoreScripts: projectOptions.ignoreScripts }); - const templatePackageJson = this.$fs.readJson(path.join(templatePath, "package.json")); await this.$npm.uninstall(templatePackageJson.name, { save: true }, projectDir); + if (templateVersion === constants.TemplateVersions.v2) { + this.alterPackageJsonData(projectDir, projectId); + } } catch (err) { this.$fs.deleteDirectory(projectDir); throw err; @@ -100,21 +107,32 @@ export class ProjectService implements IProjectService { return null; } - private async extractTemplate(projectDir: string, realTemplatePath: string): Promise { + private async extractTemplate(projectDir: string, realTemplatePath: string, templateVersion: string): Promise { this.$fs.ensureDirectoryExists(projectDir); - const appDestinationPath = this.$projectData.getAppDirectoryPath(projectDir); - this.$fs.createDirectory(appDestinationPath); + this.$logger.trace(`Template version is ${templateVersion}`); + let destinationDir = ""; + switch (templateVersion) { + case constants.TemplateVersions.v2: + destinationDir = projectDir; + break; + case constants.TemplateVersions.v1: + default: + const appDestinationPath = this.$projectData.getAppDirectoryPath(projectDir); + this.$fs.createDirectory(appDestinationPath); + destinationDir = appDestinationPath; + break; + } - this.$logger.trace(`Copying application from '${realTemplatePath}' into '${appDestinationPath}'.`); - shelljs.cp('-R', path.join(realTemplatePath, "*"), appDestinationPath); + this.$logger.trace(`Copying application from '${realTemplatePath}' into '${destinationDir}'.`); + shelljs.cp('-R', path.join(realTemplatePath, "*"), destinationDir); this.$fs.createDirectory(path.join(projectDir, "platforms")); } private async ensureAppResourcesExist(projectDir: string): Promise { - const appPath = this.$projectData.getAppDirectoryPath(projectDir), - appResourcesDestinationPath = this.$projectData.getAppResourcesDirectoryPath(projectDir); + const appPath = this.$projectData.getAppDirectoryPath(projectDir); + const appResourcesDestinationPath = this.$projectData.getAppResourcesDirectoryPath(projectDir); if (!this.$fs.exists(appResourcesDestinationPath)) { this.$fs.createDirectory(appResourcesDestinationPath); @@ -128,11 +146,20 @@ export class ProjectService implements IProjectService { ignoreScripts: false }); - const defaultTemplateAppResourcesPath = path.join(projectDir, constants.NODE_MODULES_FOLDER_NAME, - defaultTemplateName, constants.APP_RESOURCES_FOLDER_NAME); + const obsoleteAppResourcesPath = path.join(projectDir, + constants.NODE_MODULES_FOLDER_NAME, + defaultTemplateName, + constants.APP_RESOURCES_FOLDER_NAME); + + const defaultTemplateAppResourcesPath = path.join(projectDir, + constants.NODE_MODULES_FOLDER_NAME, + defaultTemplateName, + constants.APP_FOLDER_NAME, + constants.APP_RESOURCES_FOLDER_NAME); - if (this.$fs.exists(defaultTemplateAppResourcesPath)) { - shelljs.cp('-R', defaultTemplateAppResourcesPath, appPath); + const pathToAppResources = this.$fs.exists(defaultTemplateAppResourcesPath) ? defaultTemplateAppResourcesPath : obsoleteAppResourcesPath; + if (this.$fs.exists(pathToAppResources)) { + shelljs.cp('-R', pathToAppResources, appPath); } await this.$npm.uninstall(defaultTemplateName, { save: true }, projectDir); @@ -188,13 +215,38 @@ export class ProjectService implements IProjectService { private createPackageJson(projectDir: string, projectId: string): void { const projectFilePath = path.join(projectDir, this.$staticConfig.PROJECT_FILE_NAME); - this.$fs.writeJson(projectFilePath, { - "description": "NativeScript Application", - "license": "SEE LICENSE IN ", - "readme": "NativeScript Application", - "repository": "" - }); + this.$fs.writeJson(projectFilePath, this.packageJsonDefaultData); + + this.setAppId(projectDir, projectId); + } + + private get packageJsonDefaultData(): IStringDictionary { + return { + description: "NativeScript Application", + license: "SEE LICENSE IN ", + readme: "NativeScript Application", + repository: "" + }; + } + + private alterPackageJsonData(projectDir: string, projectId: string): void { + const projectFilePath = path.join(projectDir, this.$staticConfig.PROJECT_FILE_NAME); + + const packageJsonData = this.$fs.readJson(projectFilePath); + + // Remove the metadata keys from the package.json + let updatedPackageJsonData = _.omitBy(packageJsonData, (value: any, key: string) => _.startsWith(key, "_")); + updatedPackageJsonData = _.merge(updatedPackageJsonData, this.packageJsonDefaultData); + + if (updatedPackageJsonData.nativescript && updatedPackageJsonData.nativescript.templateVersion) { + delete updatedPackageJsonData.nativescript.templateVersion; + } + + this.$fs.writeJson(projectFilePath, updatedPackageJsonData); + this.setAppId(projectDir, projectId); + } + private setAppId(projectDir: string, projectId: string): void { this.$projectDataService.setNSValue(projectDir, "id", projectId); } } diff --git a/lib/services/project-templates-service.ts b/lib/services/project-templates-service.ts index 632b8afb30..c55c8970bd 100644 --- a/lib/services/project-templates-service.ts +++ b/lib/services/project-templates-service.ts @@ -1,6 +1,7 @@ import * as path from "path"; import * as temp from "temp"; import * as constants from "../constants"; +import { format } from "util"; temp.track(); export class ProjectTemplatesService implements IProjectTemplatesService { @@ -8,9 +9,10 @@ export class ProjectTemplatesService implements IProjectTemplatesService { public constructor(private $analyticsService: IAnalyticsService, private $fs: IFileSystem, private $logger: ILogger, - private $npmInstallationManager: INpmInstallationManager) { } + private $npmInstallationManager: INpmInstallationManager, + private $errors: IErrors) { } - public async prepareTemplate(originalTemplateName: string, projectDir: string): Promise { + public async prepareTemplate(originalTemplateName: string, projectDir: string): Promise { // support @ syntax const data = originalTemplateName.split("@"), name = data[0], @@ -18,23 +20,52 @@ export class ProjectTemplatesService implements IProjectTemplatesService { const templateName = constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()] || name; - const realTemplatePath = await this.prepareNativeScriptTemplate(templateName, version, projectDir); + const templatePath = await this.prepareNativeScriptTemplate(templateName, version, projectDir); await this.$analyticsService.track("Template used for project creation", templateName); - const templateNameToBeTracked = this.getTemplateNameToBeTracked(templateName, realTemplatePath); + const templateNameToBeTracked = this.getTemplateNameToBeTracked(templateName, templatePath); + const templateVersion = this.getTemplateVersion(templatePath); if (templateNameToBeTracked) { await this.$analyticsService.trackEventActionInGoogleAnalytics({ action: constants.TrackActionNames.CreateProject, isForDevice: null, additionalData: templateNameToBeTracked }); + + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: constants.TrackActionNames.UsingTemplate, + additionalData: `${templateNameToBeTracked}${constants.AnalyticsEventLabelDelimiter}${templateVersion}` + }); } // 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.$fs.deleteDirectory(path.join(templatePath, constants.NODE_MODULES_FOLDER_NAME)); + + return { templatePath, templateVersion }; + } + + private getTemplateVersion(templatePath: string): string { + this.$logger.trace(`Checking the NativeScript version of the template located at ${templatePath}.`); + const pathToPackageJson = path.join(templatePath, constants.PACKAGE_JSON_FILE_NAME); + if (this.$fs.exists(pathToPackageJson)) { + const packageJsonContent = this.$fs.readJson(pathToPackageJson); + const templateVersionFromPackageJson: string = packageJsonContent && packageJsonContent.nativescript && packageJsonContent.nativescript.templateVersion; + + if (templateVersionFromPackageJson) { + this.$logger.trace(`The template ${templatePath} has version ${templateVersionFromPackageJson}.`); + + if (_.values(constants.TemplateVersions).indexOf(templateVersionFromPackageJson) === -1) { + this.$errors.failWithoutHelp(format(constants.ProjectTemplateErrors.InvalidTemplateVersionStringFormat, templatePath, templateVersionFromPackageJson)); + } + + return templateVersionFromPackageJson; + } + } - return realTemplatePath; + const defaultVersion = constants.TemplateVersions.v1; + this.$logger.trace(`The template ${templatePath} does not specify version or we were unable to find out the version. Using default one ${defaultVersion}`); + return defaultVersion; } /** diff --git a/test/project-templates-service.ts b/test/project-templates-service.ts index 5ee01a0fbe..37e95b023c 100644 --- a/test/project-templates-service.ts +++ b/test/project-templates-service.ts @@ -4,30 +4,25 @@ import { ProjectTemplatesService } from "../lib/services/project-templates-servi import { assert } from "chai"; import * as path from "path"; import * as constants from "../lib/constants"; +import { format } from "util"; let isDeleteDirectoryCalledForNodeModulesDir = false; const nativeScriptValidatedTemplatePath = "nsValidatedTemplatePath"; -function createTestInjector(configuration?: { shouldNpmInstallThrow: boolean, npmInstallationDirContents: string[], npmInstallationDirNodeModulesContents: string[] }): IInjector { +function createTestInjector(configuration: { shouldNpmInstallThrow?: boolean, packageJsonContent?: any } = {}): IInjector { const injector = new Yok(); injector.register("errors", stubs.ErrorsStub); injector.register("logger", stubs.LoggerStub); injector.register("fs", { - readDirectory: (dirPath: string): string[] => { - if (dirPath.toLowerCase().indexOf("node_modules") !== -1) { - return configuration.npmInstallationDirNodeModulesContents; - } - return configuration.npmInstallationDirContents; - }, + exists: (pathToCheck: string) => true, + + readJson: (pathToFile: string) => configuration.packageJsonContent || {}, deleteDirectory: (directory: string) => { if (directory.indexOf("node_modules") !== -1) { isDeleteDirectoryCalledForNodeModulesDir = true; } - }, - - exists: (filePath: string): boolean => false - + } }); injector.register("npm", { @@ -61,8 +56,6 @@ function createTestInjector(configuration?: { shouldNpmInstallThrow: boolean, np } describe("project-templates-service", () => { - let testInjector: IInjector; - let projectTemplatesService: IProjectTemplatesService; beforeEach(() => { isDeleteDirectoryCalledForNodeModulesDir = false; }); @@ -70,47 +63,52 @@ describe("project-templates-service", () => { describe("prepareTemplate", () => { describe("throws error", () => { it("when npm install fails", async () => { - testInjector = createTestInjector({ shouldNpmInstallThrow: true, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: null }); - projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const testInjector = createTestInjector({ shouldNpmInstallThrow: true }); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); await assert.isRejected(projectTemplatesService.prepareTemplate("invalidName", "tempFolder")); }); }); describe("returns correct path to template", () => { it("when reserved template name is used", async () => { - testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] }); - projectTemplatesService = testInjector.resolve("projectTemplatesService"); - const actualPathToTemplate = await projectTemplatesService.prepareTemplate("typescript", "tempFolder"); - assert.strictEqual(path.basename(actualPathToTemplate), nativeScriptValidatedTemplatePath); + const testInjector = createTestInjector(); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const { templatePath } = await projectTemplatesService.prepareTemplate("typescript", "tempFolder"); + assert.strictEqual(path.basename(templatePath), nativeScriptValidatedTemplatePath); assert.strictEqual(isDeleteDirectoryCalledForNodeModulesDir, true, "When correct path is returned, template's node_modules directory should be deleted."); }); it("when reserved template name is used (case-insensitive test)", async () => { - testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] }); - projectTemplatesService = testInjector.resolve("projectTemplatesService"); - const actualPathToTemplate = await projectTemplatesService.prepareTemplate("tYpEsCriPT", "tempFolder"); - assert.strictEqual(path.basename(actualPathToTemplate), nativeScriptValidatedTemplatePath); + const testInjector = createTestInjector(); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const { templatePath } = await projectTemplatesService.prepareTemplate("tYpEsCriPT", "tempFolder"); + assert.strictEqual(path.basename(templatePath), nativeScriptValidatedTemplatePath); assert.strictEqual(isDeleteDirectoryCalledForNodeModulesDir, true, "When correct path is returned, template's node_modules directory should be deleted."); }); it("uses defaultTemplate when undefined is passed as parameter", async () => { - testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] }); - projectTemplatesService = testInjector.resolve("projectTemplatesService"); - const actualPathToTemplate = await projectTemplatesService.prepareTemplate(constants.RESERVED_TEMPLATE_NAMES["default"], "tempFolder"); - assert.strictEqual(path.basename(actualPathToTemplate), nativeScriptValidatedTemplatePath); + const testInjector = createTestInjector(); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const { templatePath } = await projectTemplatesService.prepareTemplate(constants.RESERVED_TEMPLATE_NAMES["default"], "tempFolder"); + assert.strictEqual(path.basename(templatePath), nativeScriptValidatedTemplatePath); assert.strictEqual(isDeleteDirectoryCalledForNodeModulesDir, true, "When correct path is returned, template's node_modules directory should be deleted."); }); }); describe("sends correct information to Google Analytics", () => { let analyticsService: IAnalyticsService; - let dataSentToGoogleAnalytics: IEventActionData; + let dataSentToGoogleAnalytics: IEventActionData[] = []; + let testInjector: IInjector; + let projectTemplatesService: IProjectTemplatesService; + beforeEach(() => { - testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] }); + testInjector = createTestInjector({ shouldNpmInstallThrow: false }); analyticsService = testInjector.resolve("analyticsService"); - dataSentToGoogleAnalytics = null; + const fs = testInjector.resolve("fs"); + fs.exists = (filePath: string) => false; + dataSentToGoogleAnalytics = []; analyticsService.trackEventActionInGoogleAnalytics = async (data: IEventActionData): Promise => { - dataSentToGoogleAnalytics = data; + dataSentToGoogleAnalytics.push(data); }; projectTemplatesService = testInjector.resolve("projectTemplatesService"); }); @@ -118,11 +116,17 @@ describe("project-templates-service", () => { it("sends template name when the template is used from npm", async () => { const templateName = "template-from-npm"; await projectTemplatesService.prepareTemplate(templateName, "tempFolder"); - assert.deepEqual(dataSentToGoogleAnalytics, { - action: constants.TrackActionNames.CreateProject, - isForDevice: null, - additionalData: templateName - }); + assert.deepEqual(dataSentToGoogleAnalytics, [ + { + action: constants.TrackActionNames.CreateProject, + isForDevice: null, + additionalData: templateName + }, + { + action: constants.TrackActionNames.UsingTemplate, + additionalData: `${templateName}${constants.AnalyticsEventLabelDelimiter}${constants.TemplateVersions.v1}` + } + ]); }); it("sends template name (from template's package.json) when the template is used from local path", async () => { @@ -132,11 +136,17 @@ describe("project-templates-service", () => { fs.exists = (path: string): boolean => true; fs.readJson = (filename: string, encoding?: string): any => ({ name: templateName }); await projectTemplatesService.prepareTemplate(localTemplatePath, "tempFolder"); - assert.deepEqual(dataSentToGoogleAnalytics, { - action: constants.TrackActionNames.CreateProject, - isForDevice: null, - additionalData: `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateName}` - }); + assert.deepEqual(dataSentToGoogleAnalytics, [ + { + action: constants.TrackActionNames.CreateProject, + isForDevice: null, + additionalData: `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateName}` + }, + { + action: constants.TrackActionNames.UsingTemplate, + additionalData: `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateName}${constants.AnalyticsEventLabelDelimiter}${constants.TemplateVersions.v1}` + } + ]); }); it("sends the template name (path to dirname) when the template is used from local path but there's no package.json at the root", async () => { @@ -145,26 +155,59 @@ describe("project-templates-service", () => { const fs = testInjector.resolve("fs"); fs.exists = (localPath: string): boolean => path.basename(localPath) !== constants.PACKAGE_JSON_FILE_NAME; await projectTemplatesService.prepareTemplate(localTemplatePath, "tempFolder"); - assert.deepEqual(dataSentToGoogleAnalytics, { - action: constants.TrackActionNames.CreateProject, - isForDevice: null, - additionalData: `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateName}` - }); + assert.deepEqual(dataSentToGoogleAnalytics, [ + { + action: constants.TrackActionNames.CreateProject, + isForDevice: null, + additionalData: `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateName}` + }, + { + action: constants.TrackActionNames.UsingTemplate, + additionalData: `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateName}${constants.AnalyticsEventLabelDelimiter}${constants.TemplateVersions.v1}` + } + ]); }); + }); - it("does not send anything when trying to get template name fails", async () => { - const templateName = "localtemplate"; - const localTemplatePath = `/Users/username/${templateName}`; - const fs = testInjector.resolve("fs"); - fs.exists = (localPath: string): boolean => true; - fs.readJson = (filename: string, encoding?: string): any => { - throw new Error("Unable to read json"); - }; + describe("template version", () => { + it("is default when template does not have package.json", async () => { + const testInjector = createTestInjector(); + testInjector.resolve("fs").exists = (filePath: string) => false; - await projectTemplatesService.prepareTemplate(localTemplatePath, "tempFolder"); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const { templateVersion } = await projectTemplatesService.prepareTemplate("typescript", "tempFolder"); + assert.strictEqual(templateVersion, constants.TemplateVersions.v1); + }); + + it("is default when template does not have nativescript key in its package.json", async () => { + const testInjector = createTestInjector(); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const { templateVersion } = await projectTemplatesService.prepareTemplate("typescript", "tempFolder"); + assert.strictEqual(templateVersion, constants.TemplateVersions.v1); + }); + + it("is default when template does not have templateVersion property in the nativescript key in its package.json", async () => { + const testInjector = createTestInjector({ packageJsonContent: { nativescript: {} } }); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const { templateVersion } = await projectTemplatesService.prepareTemplate("typescript", "tempFolder"); + assert.strictEqual(templateVersion, constants.TemplateVersions.v1); + }); - assert.deepEqual(dataSentToGoogleAnalytics, null); + it("is the one from template's package.json when it is valid version", async () => { + const testInjector = createTestInjector({ packageJsonContent: { nativescript: { templateVersion: constants.TemplateVersions.v2 } } }); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const { templateVersion } = await projectTemplatesService.prepareTemplate("typescript", "tempFolder"); + assert.strictEqual(templateVersion, constants.TemplateVersions.v2); }); + + it("fails when the templateVersion is invalid", async () => { + const notSupportedVersionString = "not supported version"; + const testInjector = createTestInjector({ packageJsonContent: { nativescript: { templateVersion: notSupportedVersionString } } }); + const projectTemplatesService = testInjector.resolve("projectTemplatesService"); + const expectedError = format(constants.ProjectTemplateErrors.InvalidTemplateVersionStringFormat, nativeScriptValidatedTemplatePath, notSupportedVersionString); + await assert.isRejected(projectTemplatesService.prepareTemplate("typescript", "tempFolder"), expectedError); + }); + }); }); }); diff --git a/test/stubs.ts b/test/stubs.ts index 8a43f1eebd..58998e5ce2 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -475,8 +475,8 @@ export class ProjectHelperStub implements IProjectHelper { } export class ProjectTemplatesService implements IProjectTemplatesService { - async prepareTemplate(templateName: string): Promise { - return Promise.resolve(""); + async prepareTemplate(templateName: string): Promise { + return Promise.resolve({}); } } From 72c7f2280cffe45f077bde81b478c5724ccb8b06 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 21 May 2018 14:29:00 +0300 Subject: [PATCH 2/4] chore: Ensure App_Resources are copied correctly from default template In case the used template does not have App_Resources, we need to copy them from the default template. Ensure the location of the App_Resources in the default template is calculated correctly based on its templateVersion property. --- lib/definitions/project.d.ts | 8 +++++ lib/services/project-service.ts | 39 +++++++++++++---------- lib/services/project-templates-service.ts | 2 +- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/lib/definitions/project.d.ts b/lib/definitions/project.d.ts index 2ce4c8c5ef..9d10039e16 100644 --- a/lib/definitions/project.d.ts +++ b/lib/definitions/project.d.ts @@ -219,6 +219,14 @@ interface IProjectTemplatesService { * @return {ITemplateData} Data describing the template - location where it is installed and its NativeScript version. */ prepareTemplate(templateName: string, projectDir: string): Promise; + + /** + * Gives information for the nativescript specific version of the template, for example v1, v2, etc. + * Defaults to v1 in case there's no version specified. + * @param {string} templatePath Full path to the template. + * @returns {string} The version, for example v1 or v2. + */ + getTemplateVersion(templatePath: string): string; } interface IPlatformProjectServiceBase { diff --git a/lib/services/project-service.ts b/lib/services/project-service.ts index c4ab580b4e..075cbe854a 100644 --- a/lib/services/project-service.ts +++ b/lib/services/project-service.ts @@ -113,15 +113,15 @@ export class ProjectService implements IProjectService { this.$logger.trace(`Template version is ${templateVersion}`); let destinationDir = ""; switch (templateVersion) { - case constants.TemplateVersions.v2: - destinationDir = projectDir; - break; case constants.TemplateVersions.v1: - default: const appDestinationPath = this.$projectData.getAppDirectoryPath(projectDir); this.$fs.createDirectory(appDestinationPath); destinationDir = appDestinationPath; break; + case constants.TemplateVersions.v2: + default: + destinationDir = projectDir; + break; } this.$logger.trace(`Copying application from '${realTemplatePath}' into '${destinationDir}'.`); @@ -146,20 +146,25 @@ export class ProjectService implements IProjectService { ignoreScripts: false }); - const obsoleteAppResourcesPath = path.join(projectDir, - constants.NODE_MODULES_FOLDER_NAME, - defaultTemplateName, - constants.APP_RESOURCES_FOLDER_NAME); - - const defaultTemplateAppResourcesPath = path.join(projectDir, - constants.NODE_MODULES_FOLDER_NAME, - defaultTemplateName, - constants.APP_FOLDER_NAME, - constants.APP_RESOURCES_FOLDER_NAME); + const defaultTemplatePath = path.join(projectDir, constants.NODE_MODULES_FOLDER_NAME, defaultTemplateName); + const defaultTemplateVersion = this.$projectTemplatesService.getTemplateVersion(defaultTemplatePath); + + let defaultTemplateAppResourcesPath: string = null; + switch (defaultTemplateVersion) { + case constants.TemplateVersions.v1: + defaultTemplateAppResourcesPath = path.join(projectDir, + constants.NODE_MODULES_FOLDER_NAME, + defaultTemplateName, + constants.APP_RESOURCES_FOLDER_NAME); + break; + case constants.TemplateVersions.v2: + default: + const defaultTemplateProjectData = this.$projectDataService.getProjectData(defaultTemplatePath); + defaultTemplateAppResourcesPath = defaultTemplateProjectData.appResourcesDirectoryPath; + } - const pathToAppResources = this.$fs.exists(defaultTemplateAppResourcesPath) ? defaultTemplateAppResourcesPath : obsoleteAppResourcesPath; - if (this.$fs.exists(pathToAppResources)) { - shelljs.cp('-R', pathToAppResources, appPath); + if (defaultTemplateAppResourcesPath && this.$fs.exists(defaultTemplateAppResourcesPath)) { + shelljs.cp('-R', defaultTemplateAppResourcesPath, appPath); } await this.$npm.uninstall(defaultTemplateName, { save: true }, projectDir); diff --git a/lib/services/project-templates-service.ts b/lib/services/project-templates-service.ts index c55c8970bd..2798d1850e 100644 --- a/lib/services/project-templates-service.ts +++ b/lib/services/project-templates-service.ts @@ -45,7 +45,7 @@ export class ProjectTemplatesService implements IProjectTemplatesService { return { templatePath, templateVersion }; } - private getTemplateVersion(templatePath: string): string { + public getTemplateVersion(templatePath: string): string { this.$logger.trace(`Checking the NativeScript version of the template located at ${templatePath}.`); const pathToPackageJson = path.join(templatePath, constants.PACKAGE_JSON_FILE_NAME); if (this.$fs.exists(pathToPackageJson)) { From 0254825b7fe2bca205267aba77ab2f1897878da9 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 21 May 2018 16:24:38 +0300 Subject: [PATCH 3/4] feat: Add after-createProject hook Add `after-createProject` hook, so template authors will be able to execute different operations based on their requirements. The hook is called at the end of the project creation step. Also remove usage of `$projectData` and use `$projectDataService` in order to ensure correct behavior in long living process --- lib/constants.ts | 4 +++ lib/definitions/project.d.ts | 36 +++++++++++++++++---------- lib/services/project-service.ts | 44 ++++++++++++++++++++++----------- test/stubs.ts | 4 +++ 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/lib/constants.ts b/lib/constants.ts index fdaa683893..5145ac96ec 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -201,3 +201,7 @@ export class TemplateVersions { export class ProjectTemplateErrors { public static InvalidTemplateVersionStringFormat = "The template '%s' has a NativeScript version '%s' that is not supported. Unable to create project from it."; } + +export class Hooks { + public static createProject = "createProject"; +} diff --git a/lib/definitions/project.d.ts b/lib/definitions/project.d.ts index 9d10039e16..3e30b01d55 100644 --- a/lib/definitions/project.d.ts +++ b/lib/definitions/project.d.ts @@ -1,12 +1,19 @@ -/** - * Describes available settings when creating new NativeScript application. - */ -interface IProjectSettings { +interface IProjectName { + projectName: string; +} + +interface IProjectSettingsBase extends IProjectName { /** * Name of the newly created application. */ projectName: string; + /** + * Defines whether the `npm install` command should be executed with `--ignore-scripts` option. + * When it is passed, all scripts (postinstall for example) will not be executed. + */ + ignoreScripts?: boolean; + /** * Selected template from which to create the project. If not specified, defaults to hello-world template. * Template can be any npm package, local dir, github url, .tgz file. @@ -19,7 +26,19 @@ interface IProjectSettings { * Application identifier for the newly created application. If not specified, defaults to org.nativescript.. */ appId?: string; +} +/** + * Describes information passed to project creation hook (createProject). + */ +interface IProjectCreationSettings extends IProjectSettingsBase, IProjectDir { + +} + +/** + * Describes available settings when creating new NativeScript application. + */ +interface IProjectSettings extends IProjectSettingsBase { /** * Path where the project will be created. If not specified, defaults to current working dir. */ @@ -29,17 +48,8 @@ interface IProjectSettings { * Defines if invalid application name can be used for project creation. */ force?: boolean; - - /** - * Defines whether the `npm install` command should be executed with `--ignore-scripts` option. - * When it is passed, all scripts (postinstall for example) will not be executed. - */ - ignoreScripts?: boolean; } -interface IProjectName { - projectName: string; -} interface ICreateProjectData extends IProjectDir, IProjectName { diff --git a/lib/services/project-service.ts b/lib/services/project-service.ts index 075cbe854a..4a7d7a678f 100644 --- a/lib/services/project-service.ts +++ b/lib/services/project-service.ts @@ -2,14 +2,15 @@ import * as constants from "../constants"; import * as path from "path"; import * as shelljs from "shelljs"; import { exported } from "../common/decorators"; +import { Hooks } from "../constants"; export class ProjectService implements IProjectService { - constructor(private $npm: INodePackageManager, + constructor(private $hooksService: IHooksService, + private $npm: INodePackageManager, private $errors: IErrors, private $fs: IFileSystem, private $logger: ILogger, - private $projectData: IProjectData, private $projectDataService: IProjectDataService, private $projectHelper: IProjectHelper, private $projectNameService: IProjectNameService, @@ -37,16 +38,25 @@ export class ProjectService implements IProjectService { this.$errors.fail("Path already exists and is not empty %s", projectDir); } - const projectId = projectOptions.appId || this.$projectHelper.generateDefaultAppId(projectName, constants.DEFAULT_APP_IDENTIFIER_PREFIX); - this.createPackageJson(projectDir, projectId); - - this.$logger.trace(`Creating a new NativeScript project with name ${projectName} and id ${projectId} at location ${projectDir}`); + const appId = projectOptions.appId || this.$projectHelper.generateDefaultAppId(projectName, constants.DEFAULT_APP_IDENTIFIER_PREFIX); + this.createPackageJson(projectDir, appId); + this.$logger.trace(`Creating a new NativeScript project with name ${projectName} and id ${appId} at location ${projectDir}`); if (!selectedTemplate) { selectedTemplate = constants.RESERVED_TEMPLATE_NAMES["default"]; } + const projectCreationData = await this.createProjectCore({ template: selectedTemplate, projectDir, ignoreScripts: projectOptions.ignoreScripts, appId: appId, projectName }); + + this.$logger.printMarkdown("Project `%s` was successfully created.", projectCreationData.projectName); + + return projectCreationData; + } + + private async createProjectCore(projectCreationSettings: IProjectCreationSettings): Promise { + const { template, projectDir, appId, projectName, ignoreScripts } = projectCreationSettings; + try { - const { templatePath, templateVersion } = await this.$projectTemplatesService.prepareTemplate(selectedTemplate, projectDir); + const { templatePath, templateVersion } = await this.$projectTemplatesService.prepareTemplate(template, projectDir); await this.extractTemplate(projectDir, templatePath, templateVersion); await this.ensureAppResourcesExist(projectDir); @@ -68,19 +78,22 @@ export class ProjectService implements IProjectService { await this.$npm.install(projectDir, projectDir, { disableNpmInstall: false, frameworkPath: null, - ignoreScripts: projectOptions.ignoreScripts + ignoreScripts }); await this.$npm.uninstall(templatePackageJson.name, { save: true }, projectDir); if (templateVersion === constants.TemplateVersions.v2) { - this.alterPackageJsonData(projectDir, projectId); + this.alterPackageJsonData(projectDir, appId); } } catch (err) { this.$fs.deleteDirectory(projectDir); throw err; } - this.$logger.printMarkdown("Project `%s` was successfully created.", projectName); + this.$hooksService.executeAfterHooks(Hooks.createProject, { + hookArgs: projectCreationSettings + }); + return { projectName, projectDir }; } @@ -114,7 +127,8 @@ export class ProjectService implements IProjectService { let destinationDir = ""; switch (templateVersion) { case constants.TemplateVersions.v1: - const appDestinationPath = this.$projectData.getAppDirectoryPath(projectDir); + const projectData = this.$projectDataService.getProjectData(projectDir); + const appDestinationPath = projectData.getAppDirectoryPath(projectDir); this.$fs.createDirectory(appDestinationPath); destinationDir = appDestinationPath; break; @@ -131,8 +145,9 @@ export class ProjectService implements IProjectService { } private async ensureAppResourcesExist(projectDir: string): Promise { - const appPath = this.$projectData.getAppDirectoryPath(projectDir); - const appResourcesDestinationPath = this.$projectData.getAppResourcesDirectoryPath(projectDir); + const projectData = this.$projectDataService.getProjectData(projectDir); + const appPath = projectData.getAppDirectoryPath(projectDir); + const appResourcesDestinationPath = projectData.getAppResourcesDirectoryPath(projectDir); if (!this.$fs.exists(appResourcesDestinationPath)) { this.$fs.createDirectory(appResourcesDestinationPath); @@ -172,7 +187,8 @@ export class ProjectService implements IProjectService { } private removeMergedDependencies(projectDir: string, templatePackageJsonData: any): void { - const extractedTemplatePackageJsonPath = path.join(this.$projectData.getAppDirectoryPath(projectDir), constants.PACKAGE_JSON_FILE_NAME); + const appDirectoryPath = this.$projectDataService.getProjectData(projectDir).appDirectoryPath; + const extractedTemplatePackageJsonPath = path.join(appDirectoryPath, constants.PACKAGE_JSON_FILE_NAME); for (const key in templatePackageJsonData) { if (constants.PackageJsonKeysToKeep.indexOf(key) === -1) { delete templatePackageJsonData[key]; diff --git a/test/stubs.ts b/test/stubs.ts index 58998e5ce2..88440bf3bb 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -478,6 +478,10 @@ export class ProjectTemplatesService implements IProjectTemplatesService { async prepareTemplate(templateName: string): Promise { return Promise.resolve({}); } + + getTemplateVersion(templatePath: string): string { + return constants.TemplateVersions.v1; + } } export class HooksServiceStub implements IHooksService { From cd3f6cb89b236963637a2d399a6df477c4bcd734 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 21 May 2018 23:22:39 +0300 Subject: [PATCH 4/4] chore: fix tests --- test/project-service.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/project-service.ts b/test/project-service.ts index 8b756ed935..00557de74b 100644 --- a/test/project-service.ts +++ b/test/project-service.ts @@ -18,6 +18,7 @@ import { Options } from "../lib/options"; import { HostInfo } from "../lib/common/host-info"; import { ProjectTemplatesService } from "../lib/services/project-templates-service"; import { SettingsService } from "../lib/common/test/unit-tests/stubs"; +import { DevicePlatformsConstants } from "../lib/common/mobile/device-platforms-constants"; const mockProjectNameValidator = { validate: () => true @@ -157,8 +158,13 @@ class ProjectIntegrationTest { }); this.testInjector.register("npmInstallationManager", NpmInstallationManager); this.testInjector.register("settingsService", SettingsService); - this.testInjector.register("devicePlatformsConstants", {}); - this.testInjector.register("androidResourcesMigrationService", {}); + this.testInjector.register("devicePlatformsConstants", DevicePlatformsConstants); + this.testInjector.register("androidResourcesMigrationService", { + hasMigrated: (appResourcesDir: string): boolean => true + }); + this.testInjector.register("hooksService", { + executeAfterHooks: async (commandName: string, hookArguments?: IDictionary): Promise => undefined + }); } } @@ -436,6 +442,9 @@ describe("Project Service Tests", () => { testInjector.register("projectHelper", {}); testInjector.register("npmInstallationManager", {}); testInjector.register("settingsService", SettingsService); + testInjector.register("hooksService", { + executeAfterHooks: async (commandName: string, hookArguments?: IDictionary): Promise => undefined + }); return testInjector; };