refactor: fix migrate command PR comments

KristianDD · KristianDD · commit 520ce8d9eb52 · 2019-06-21T00:17:37.000+03:00
diff --git a/lib/commands/migrate.ts b/lib/commands/migrate.ts
@@ -14,7 +14,7 @@ export class MigrateCommand implements ICommand {
 
 	public async canExecute(args: string[]): Promise<boolean> {
 		if (!await this.$migrateController.shouldMigrate({ projectDir: this.$projectData.projectDir })) {
-			this.$errors.failWithoutHelp('Project is already compatible with NativeScript "v6.0.0". To get the latest NativesScript packages execute "tns update".');
+			this.$errors.failWithoutHelp('Project is compatible with NativeScript "v6.0.0". To get the latest NativesScript packages execute "tns update".');
 		}
 
 		return true;
diff --git a/lib/controllers/migrate-controller.ts b/lib/controllers/migrate-controller.ts
@@ -1,9 +1,9 @@
 import * as path from "path";
 import * as semver from "semver";
 import * as constants from "../constants";
-import { BaseUpdateController } from "./base-update-controller";
+import { UpdateControllerBase } from "./update-controller-base";
 
-export class MigrateController extends BaseUpdateController implements IMigrateController {
+export class MigrateController extends UpdateControllerBase implements IMigrateController {
 	constructor(
 		protected $fs: IFileSystem,
 		protected $platformCommandHelper: IPlatformCommandHelper,
@@ -12,6 +12,7 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
 		protected $packageManager: IPackageManager,
 		private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
 		private $logger: ILogger,
+		private $errors: IErrors,
 		private $addPlatformService: IAddPlatformService,
 		private $pluginsService: IPluginsService,
 		private $projectDataService: IProjectDataService) {
@@ -28,15 +29,15 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
 	];
 
 	static readonly migrationDependencies : IMigrationDependency[] = [
-		{ packageName: constants.TNS_CORE_MODULES_NAME, isDev: false, verifiedVersion: "6.0.0-next-2019-06-10-092158-01"},
-		{ packageName: constants.TNS_CORE_MODULES_WIDGETS_NAME, isDev: false, verifiedVersion: "6.0.0-next-2019-06-10-092158-01"},
+		{ packageName: constants.TNS_CORE_MODULES_NAME, verifiedVersion: "6.0.0-next-2019-06-10-092158-01"},
+		{ packageName: constants.TNS_CORE_MODULES_WIDGETS_NAME, verifiedVersion: "6.0.0-next-2019-06-10-092158-01"},
 		{ packageName: "node-sass", isDev: true, verifiedVersion: "4.12.0"},
 		{ packageName: "typescript", isDev: true, verifiedVersion: "3.4.1"},
 		{ packageName: "less", isDev: true, verifiedVersion: "3.9.0"},
 		{ packageName: "nativescript-dev-sass", isDev: true, replaceWith: "node-sass"},
 		{ packageName: "nativescript-dev-typescript", isDev: true, replaceWith: "typescript"},
 		{ packageName: "nativescript-dev-less", isDev: true, replaceWith: "less"},
-		{ packageName: constants.WEBPACK_PLUGIN_NAME, isDev: true, shouldAdd: true, verifiedVersion: "0.25.0-webpack-2019-06-11-105349-01"},
+		{ packageName: constants.WEBPACK_PLUGIN_NAME, isDev: true, shouldAddIfMissing: true, verifiedVersion: "0.25.0-webpack-2019-06-11-105349-01"},
 		{ packageName: "nativescript-camera", verifiedVersion: "4.5.0"},
 		{ packageName: "nativescript-geolocation", verifiedVersion: "5.1.0"},
 		{ packageName: "nativescript-imagepicker", verifiedVersion: "6.2.0"},
@@ -50,16 +51,16 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
 		{ packageName: "nativescript-ui-autocomplete", verifiedVersion: "5.0.0-androidx-110619"},
 		{ packageName: "nativescript-datetimepicker", verifiedVersion: "1.1.0"},
 		//TODO update with compatible with webpack only hooks
-		{ packageName: "kinvey-nativescript-sdk", verifiedVersion: "4.1.0"},
+		{ packageName: "kinvey-nativescript-sdk", verifiedVersion: "4.2.1"},
 		//TODO update with compatible with webpack only hooks
-		{ packageName: "nativescript-plugin-firebase", verifiedVersion: "8.3.2"},
+		{ packageName: "nativescript-plugin-firebase", verifiedVersion: "9.0.1"},
 		//TODO update with no prerelease version compatible with webpack only hooks
 		{ packageName: "nativescript-vue", verifiedVersion: "2.3.0-rc.0"},
 		{ packageName: "nativescript-permissions", verifiedVersion: "1.3.0"},
 		{ packageName: "nativescript-cardview", verifiedVersion: "3.2.0"}
 	];
 
-	static readonly tempFolder: string = ".migration_backup";
+	static readonly backupFolder: string = ".migration_backup";
 	static readonly migrateFailMessage: string = "Could not migrate the project!";
 	static readonly backupFailMessage: string = "Could not backup project folders!";
 
@@ -72,42 +73,50 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
 
 	public async migrate({projectDir}: {projectDir: string}): Promise<void> {
 		const projectData = this.$projectDataService.getProjectData(projectDir);
-		const tmpDir = path.join(projectDir, MigrateController.tempFolder);
+		const backupDir = path.join(projectDir, MigrateController.backupFolder);
 
 		try {
 			this.$logger.info("Backup project configuration.");
-			this.backup(MigrateController.folders, tmpDir, projectData);
+			this.backup(MigrateController.folders, backupDir, projectData.projectDir);
 			this.$logger.info("Backup project configuration complete.");
 		} catch (error) {
 			this.$logger.error(MigrateController.backupFailMessage);
-			this.$fs.deleteDirectory(tmpDir);
+			this.$fs.deleteDirectory(backupDir);
 			return;
 		}
 
 		try {
 			await this.cleanUpProject(projectData);
 			await this.migrateDependencies(projectData);
 		} catch (error) {
-			this.restoreBackup(MigrateController.folders, tmpDir, projectData);
+			this.restoreBackup(MigrateController.folders, backupDir, projectData.projectDir);
 			this.$logger.error(MigrateController.migrateFailMessage);
 		}
 	}
 
 	public async shouldMigrate({projectDir}: IProjectDir): Promise<boolean> {
 		const projectData = this.$projectDataService.getProjectData(projectDir);
+
 		for (let i = 0; i < MigrateController.migrationDependencies.length; i++) {
 			const dependency = MigrateController.migrationDependencies[i];
-			const collection = dependency.isDev ? projectData.devDependencies : projectData.dependencies;
-			if (dependency.replaceWith && collection && collection[dependency.packageName]) {
+			const hasDependency = this.hasDependency(dependency, projectData);
+
+			if (hasDependency && dependency.replaceWith) {
+				return true;
+			}
+
+			if (hasDependency && await this.shouldMigrateDependencyVersion(dependency, projectData)) {
 				return true;
 			}
 
-			if (!this.shouldSkipMigrationDependency(dependency, projectData) && await this.shouldMigrateDependencyVersion(dependency, projectData)) {
+			if (!hasDependency && dependency.shouldAddIfMissing) {
 				return true;
 			}
 		}
+
 		for (const platform in this.$devicePlatformsConstants) {
-			if (await this.shouldUpdateRuntimeVersion({ targetVersion: this.verifiedPlatformVersions[platform.toLowerCase()], platform, projectData, shouldAdd: true})) {
+			const hasRuntimeDependency = this.hasRuntimeDependency({platform, projectData});
+			if (!hasRuntimeDependency || await this.shouldUpdateRuntimeVersion({ targetVersion: this.verifiedPlatformVersions[platform.toLowerCase()], platform, projectData})) {
 				return true;
 			}
 		}
@@ -125,27 +134,39 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
 	}
 
 	private async migrateDependencies(projectData: IProjectData): Promise<void> {
-		this.$logger.info("Start migrating dependencies.");
+		this.$logger.info("Start dependencies migration.");
 		for (let i = 0; i < MigrateController.migrationDependencies.length; i++) {
 			const dependency = MigrateController.migrationDependencies[i];
-			if (this.shouldSkipMigrationDependency(dependency, projectData)) {
-				continue;
-			}
+			const hasDependency = this.hasDependency(dependency, projectData);
 
-			if (dependency.replaceWith) {
+			if (hasDependency && dependency.replaceWith) {
 				this.$pluginsService.removeFromPackageJson(dependency.packageName, dependency.isDev, projectData.projectDir);
 				const replacementDep = _.find(MigrateController.migrationDependencies, migrationPackage => migrationPackage.packageName === dependency.replaceWith);
+				if (!replacementDep) {
+					this.$errors.failWithoutHelp("Failed to find replacement dependency.");
+				}
 				this.$logger.info(`Replacing '${dependency.packageName}' with '${replacementDep.packageName}'.`, );
 				this.$pluginsService.addToPackageJson(replacementDep.packageName, replacementDep.verifiedVersion, replacementDep.isDev, projectData.projectDir);
-			} else if (await this.shouldMigrateDependencyVersion(dependency, projectData)) {
+				continue;
+			}
+
+			if (hasDependency && await this.shouldMigrateDependencyVersion(dependency, projectData)) {
 				this.$logger.info(`Updating '${dependency.packageName}' to compatible version '${dependency.verifiedVersion}'`);
 				this.$pluginsService.addToPackageJson(dependency.packageName, dependency.verifiedVersion, dependency.isDev, projectData.projectDir);
+				continue;
+			}
+
+			if (!hasDependency && dependency.shouldAddIfMissing) {
+				this.$logger.info(`Adding '${dependency.packageName}' with version '${dependency.verifiedVersion}'`);
+				this.$pluginsService.addToPackageJson(dependency.packageName, dependency.verifiedVersion, dependency.isDev, projectData.projectDir);
+				continue;
 			}
 		}
 
 		for (const platform in this.$devicePlatformsConstants) {
 			const lowercasePlatform = platform.toLowerCase();
-			if (await this.shouldUpdateRuntimeVersion({targetVersion: this.verifiedPlatformVersions[lowercasePlatform], platform, projectData, shouldAdd: true})) {
+			const hasRuntimeDependency = this.hasRuntimeDependency({platform, projectData});
+			if (!hasRuntimeDependency || await this.shouldUpdateRuntimeVersion({targetVersion: this.verifiedPlatformVersions[lowercasePlatform], platform, projectData})) {
 				const verifiedPlatformVersion = this.verifiedPlatformVersions[lowercasePlatform];
 				const platformData = this.$platformsDataService.getPlatformData(lowercasePlatform, projectData);
 				this.$logger.info(`Updating ${platform} platform to version '${verifiedPlatformVersion}'.`);
@@ -166,38 +187,15 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
 
 	private async shouldMigrateDependencyVersion(dependency: IMigrationDependency, projectData: IProjectData): Promise<boolean> {
 		const collection = dependency.isDev ? projectData.devDependencies : projectData.dependencies;
-		if (
-			collection &&
-			collection[dependency.packageName]
-		) {
-			const maxSatisfyingVersion = await this.getMaxDependencyVersion(dependency.packageName, collection[dependency.packageName]);
-			if (maxSatisfyingVersion) {
-				const isPrereleaseVersion = semver.prerelease(maxSatisfyingVersion);
-				const coerceMaxSatisfying = semver.coerce(maxSatisfyingVersion).version;
-				const coerceVerifiedVersion = semver.coerce(dependency.verifiedVersion).version;
-				//This makes sure that if the user has a prerelease 6.0.0-next-2019-06-10-092158-01 version we will update it to 6.0.0
-				if (isPrereleaseVersion) {
-					if (semver.gt(coerceMaxSatisfying, coerceVerifiedVersion)) {
-						return false;
-					}
-
-					//TODO This should be removed once we update the verified versions to no prerelease versions
-					if (semver.eq(maxSatisfyingVersion, dependency.verifiedVersion)) {
-						return false;
-					}
-				} else if (semver.gte(coerceMaxSatisfying, coerceVerifiedVersion)) {
-					return false;
-				}
-			}
-		}
+		const maxSatisfyingVersion = await this.getMaxDependencyVersion(dependency.packageName, collection[dependency.packageName]);
 
-		return true;
+		return !(maxSatisfyingVersion && semver.gte(maxSatisfyingVersion, dependency.verifiedVersion));
 	}
 
-	private shouldSkipMigrationDependency(dependency: IMigrationDependency, projectData: IProjectData) {
-		if (!dependency.shouldAdd) {
-			return this.shouldSkipDependency(dependency, projectData);
-		}
+	protected async shouldUpdateRuntimeVersion({targetVersion, platform, projectData}: {targetVersion: string, platform: string, projectData: IProjectData}): Promise<boolean> {
+		const maxRuntimeVersion = await this.getMaxRuntimeVersion({platform, projectData});
+
+		return !(maxRuntimeVersion && semver.gte(maxRuntimeVersion, targetVersion));
 	}
 }
 
diff --git a/lib/controllers/update-controller-base.ts b/lib/controllers/update-controller-base.ts
@@ -1,61 +1,63 @@
 import * as path from "path";
 import * as semver from "semver";
 
-export class BaseUpdateController {
+export class UpdateControllerBase {
 	constructor(protected $fs: IFileSystem,
 		protected $platformCommandHelper: IPlatformCommandHelper,
 		protected $platformsDataService: IPlatformsDataService,
 		protected $packageInstallationManager: IPackageInstallationManager,
 		protected $packageManager: IPackageManager) {
 	}
 
-	protected restoreBackup(folders: string[], tmpDir: string, projectData: IProjectData): void {
+	protected restoreBackup(folders: string[], backupDir: string, projectDir: string): void {
 		for (const folder of folders) {
-			this.$fs.deleteDirectory(path.join(projectData.projectDir, folder));
+			this.$fs.deleteDirectory(path.join(projectDir, folder));
 
-			const folderToCopy = path.join(tmpDir, folder);
+			const folderToCopy = path.join(backupDir, folder);
 
 			if (this.$fs.exists(folderToCopy)) {
-				this.$fs.copyFile(folderToCopy, projectData.projectDir);
+				this.$fs.copyFile(folderToCopy, projectDir);
 			}
 		}
 	}
 
-	protected backup(folders: string[], tmpDir: string, projectData: IProjectData): void {
-		this.$fs.deleteDirectory(tmpDir);
-		this.$fs.createDirectory(tmpDir);
+	protected backup(folders: string[], backupDir: string, projectDir: string): void {
+		this.$fs.deleteDirectory(backupDir);
+		this.$fs.createDirectory(backupDir);
 		for (const folder of folders) {
-			const folderToCopy = path.join(projectData.projectDir, folder);
+			const folderToCopy = path.join(projectDir, folder);
 			if (this.$fs.exists(folderToCopy)) {
-				this.$fs.copyFile(folderToCopy, tmpDir);
+				this.$fs.copyFile(folderToCopy, backupDir);
 			}
 		}
 	}
 
-	protected shouldSkipDependency(dependency: IDependency, projectData: IProjectData): boolean {
+	protected hasDependency(dependency: IDependency, projectData: IProjectData): boolean {
 		const collection = dependency.isDev ? projectData.devDependencies : projectData.dependencies;
-		if (!collection[dependency.packageName]) {
+		if (collection && collection[dependency.packageName]) {
 			return true;
 		}
 	}
 
-	protected async shouldUpdateRuntimeVersion({targetVersion, platform, projectData, shouldAdd}: {targetVersion: string, platform: string, projectData: IProjectData, shouldAdd: boolean}) {
+	protected hasRuntimeDependency({platform, projectData}: {platform: string, projectData: IProjectData}): boolean {
 		const lowercasePlatform = platform.toLowerCase();
 		const currentPlatformVersion = this.$platformCommandHelper.getCurrentPlatformVersion(lowercasePlatform, projectData);
-		const platformData = this.$platformsDataService.getPlatformData(lowercasePlatform, projectData);
+
 		if (currentPlatformVersion) {
-			const maxPlatformSatisfyingVersion = await this.getMaxDependencyVersion(platformData.frameworkPackageName, currentPlatformVersion) || currentPlatformVersion;
-			if (semver.gte(maxPlatformSatisfyingVersion, targetVersion)) {
-				return false;
-			}
-		} else if (!shouldAdd) {
-			return false;
+			return true;
 		}
+	}
 
-		return true;
+	protected async getMaxRuntimeVersion({platform, projectData}: {platform: string, projectData: IProjectData}) {
+		const lowercasePlatform = platform.toLowerCase();
+		const currentPlatformVersion = this.$platformCommandHelper.getCurrentPlatformVersion(lowercasePlatform, projectData);
+		const platformData = this.$platformsDataService.getPlatformData(lowercasePlatform, projectData);
+		if (currentPlatformVersion) {
+			return await this.getMaxDependencyVersion(platformData.frameworkPackageName, currentPlatformVersion) || currentPlatformVersion;
+		}
 	}
 
-	protected async getMaxDependencyVersion(dependency: string, version: string) {
+	protected async getMaxDependencyVersion(dependency: string, version: string): Promise<string> {
 		let maxDependencyVersion;
 		if (semver.valid(version)) {
 			maxDependencyVersion = version;
diff --git a/lib/definitions/migrate.d.ts b/lib/definitions/migrate.d.ts
@@ -9,8 +9,8 @@ interface IDependency {
 }
 
 interface IMigrationDependency extends IDependency {
-	mustRemove?: boolean;
+	shouldRemove?: boolean;
 	replaceWith?: string;
 	verifiedVersion?: string;
-	shouldAdd?: boolean;
+	shouldAddIfMissing?: boolean;
 }
diff --git a/lib/definitions/plugins.d.ts b/lib/definitions/plugins.d.ts
@@ -1,8 +1,8 @@
 interface IPluginsService {
 	add(plugin: string, projectData: IProjectData): Promise<void>; // adds plugin by name, github url, local path and et.
 	remove(pluginName: string, projectData: IProjectData): Promise<void>; // removes plugin only by name
-	addToPackageJson(plugin: string, version: string, dev: boolean, projectDir: string): void;
-	removeFromPackageJson(plugin: string, dev: boolean, projectDir: string): void;
+	addToPackageJson(plugin: string, version: string, isDev: boolean, projectDir: string): void;
+	removeFromPackageJson(plugin: string, isDev: boolean, projectDir: string): void;
 	getAllInstalledPlugins(projectData: IProjectData): Promise<IPluginData[]>;
 	ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise<void>;
 
diff --git a/lib/services/plugins-service.ts b/lib/services/plugins-service.ts
@@ -88,21 +88,21 @@ export class PluginsService implements IPluginsService {
 		}
 	}
 
-	public addToPackageJson(plugin: string, version: string, dev: boolean, projectDir: string) {
+	public addToPackageJson(plugin: string, version: string, isDev: boolean, projectDir: string) {
 		const packageJsonPath = this.getPackageJsonFilePath(projectDir);
 		const packageJsonContent = this.$fs.readJson(packageJsonPath);
-		const collectionKey = dev ? "devDependencies" : "dependencies";
+		const collectionKey = isDev ? "devDependencies" : "dependencies";
 
 		packageJsonContent[collectionKey] = packageJsonContent[collectionKey] || {};
 		packageJsonContent[collectionKey][plugin] = version;
 
 		this.$fs.writeJson(packageJsonPath, packageJsonContent);
 	}
 
-	public removeFromPackageJson(plugin: string, dev: boolean, projectDir: string) {
+	public removeFromPackageJson(plugin: string, isDev: boolean, projectDir: string) {
 		const packageJsonPath = this.getPackageJsonFilePath(projectDir);
 		const packageJsonContent = this.$fs.readJson(packageJsonPath);
-		const collection = dev ? packageJsonContent.devDependencies : packageJsonContent.dependencies;
+		const collection = isDev ? packageJsonContent.devDependencies : packageJsonContent.dependencies;
 
 		if (collection && collection[plugin]) {
 			delete collection[plugin];

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ export class MigrateCommand implements ICommand {`
`14`	`14`
`15`	`15`	`public async canExecute(args: string[]): Promise<boolean> {`
`16`	`16`	`if (!await this.$migrateController.shouldMigrate({ projectDir: this.$projectData.projectDir })) {`
`17`		`- this.$errors.failWithoutHelp('Project is already compatible with NativeScript "v6.0.0". To get the latest NativesScript packages execute "tns update".');`
	`17`	`+ this.$errors.failWithoutHelp('Project is compatible with NativeScript "v6.0.0". To get the latest NativesScript packages execute "tns update".');`
`18`	`18`	`}`
`19`	`19`
`20`	`20`	`return true;`
Original file line number	Diff line number	Diff line change
`@@ -9,8 +9,8 @@ interface IDependency {`
`9`	`9`	`}`
`10`	`10`
`11`	`11`	`interface IMigrationDependency extends IDependency {`
`12`		`- mustRemove?: boolean;`
	`12`	`+ shouldRemove?: boolean;`
`13`	`13`	`replaceWith?: string;`
`14`	`14`	`verifiedVersion?: string;`
`15`		`- shouldAdd?: boolean;`
	`15`	`+ shouldAddIfMissing?: boolean;`
`16`	`16`	`}`