Merge pull request #4769 from NativeScript/kddimitrov/migrate-dependency-in-wrong-goup

fix: dependency won't be migrated if in devDependencies

Author: KristianDD

lib/controllers/migrate-controller.ts

@@ -245,7 +245,7 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 		const hasDependency = this.hasDependency(dependency, projectData);
 
 		if (hasDependency && dependency.replaceWith) {
-			this.$pluginsService.removeFromPackageJson(dependency.packageName, dependency.isDev, projectData.projectDir);
+			this.$pluginsService.removeFromPackageJson(dependency.packageName, projectData.projectDir);
 			const replacementDep = _.find(this.migrationDependencies, migrationPackage => migrationPackage.packageName === dependency.replaceWith);
 			if (!replacementDep) {
 				this.$errors.failWithoutHelp("Failed to find replacement dependency.");
@@ -268,8 +268,11 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 	}
 
 	private async shouldMigrateDependencyVersion(dependency: IMigrationDependency, projectData: IProjectData): Promise<boolean> {
-		const collection = dependency.isDev ? projectData.devDependencies : projectData.dependencies;
-		const maxSatisfyingVersion = await this.getMaxDependencyVersion(dependency.packageName, collection[dependency.packageName]);
+		const devDependencies = projectData.devDependencies || {};
+		const dependencies = projectData.dependencies || {};
+		const packageName = dependency.packageName;
+		const version = dependencies[packageName] || devDependencies[packageName];
+		const maxSatisfyingVersion = await this.getMaxDependencyVersion(dependency.packageName, version);
 
 		return !(maxSatisfyingVersion && semver.gte(maxSatisfyingVersion, dependency.verifiedVersion));
 	}

lib/controllers/update-controller-base.ts

@@ -33,8 +33,10 @@ export class UpdateControllerBase {
 	}
 
 	protected hasDependency(dependency: IDependency, projectData: IProjectData): boolean {
-		const collection = dependency.isDev ? projectData.devDependencies : projectData.dependencies;
-		return collection && collection[dependency.packageName];
+		const devDependencies = projectData.devDependencies;
+		const dependencies = projectData.dependencies;
+
+		return (dependencies && dependencies[dependency.packageName]) || (devDependencies && devDependencies[dependency.packageName]);
 	}
 
 	protected hasRuntimeDependency({platform, projectData}: {platform: string, projectData: IProjectData}): boolean {

lib/controllers/update-controller.ts

@@ -121,16 +121,17 @@ export class UpdateController extends UpdateControllerBase implements IUpdateCon
 				continue;
 			}
 
-			if (await this.shouldUpdateDependency(dependency, templateVersion, areDev, projectData)) {
+			if (await this.shouldUpdateDependency(dependency, templateVersion, projectData)) {
 				this.$logger.info(`Updating '${dependency}' to version '${templateVersion}'.`);
 				this.$pluginsService.addToPackageJson(dependency, templateVersion, areDev, projectData.projectDir);
 			}
 		}
 	}
 
-	private async shouldUpdateDependency(dependency: string, targetVersion: string, isDev: boolean, projectData: IProjectData) {
-		const collection = isDev ? projectData.devDependencies : projectData.dependencies;
-		const projectVersion = collection[dependency];
+	private async shouldUpdateDependency(dependency: string, targetVersion: string, projectData: IProjectData) {
+		const devDependencies = projectData.devDependencies || {};
+		const dependencies = projectData.dependencies || {};
+		const projectVersion = dependencies[dependency] || devDependencies[dependency];
 		const maxSatisfyingTargetVersion = await this.getMaxDependencyVersion(dependency, targetVersion);
 		const maxSatisfyingProjectVersion = await this.getMaxDependencyVersion(dependency, projectVersion);
 
@@ -144,7 +145,7 @@ export class UpdateController extends UpdateControllerBase implements IUpdateCon
 				continue;
 			}
 
-			if (await this.shouldUpdateDependency(dependency, templateVersion, areDev, projectData)) {
+			if (await this.shouldUpdateDependency(dependency, templateVersion, projectData)) {
 				return true;
 			}
 		}

lib/definitions/plugins.d.ts

@@ -2,7 +2,7 @@ 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, isDev: boolean, projectDir: string): void;
-	removeFromPackageJson(plugin: string, isDev: boolean, projectDir: string): void;
+	removeFromPackageJson(plugin: string, projectDir: string): void;
 	getAllInstalledPlugins(projectData: IProjectData): Promise<IPluginData[]>;
 	ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise<void>;
 

lib/services/plugins-service.ts

@@ -90,23 +90,27 @@ export class PluginsService implements IPluginsService {
 
 	public addToPackageJson(plugin: string, version: string, isDev: boolean, projectDir: string) {
 		const packageJsonPath = this.getPackageJsonFilePath(projectDir);
-		const packageJsonContent = this.$fs.readJson(packageJsonPath);
+		let packageJsonContent = this.$fs.readJson(packageJsonPath);
 		const collectionKey = isDev ? "devDependencies" : "dependencies";
+		const oppositeCollectionKey = isDev ? "dependencies" : "devDependencies";
+		if (packageJsonContent[oppositeCollectionKey] && packageJsonContent[oppositeCollectionKey][plugin]) {
+			const result = this.removeDependencyFromPackageJsonContent(plugin, packageJsonContent);
+			packageJsonContent = result.packageJsonContent;
+		}
 
 		packageJsonContent[collectionKey] = packageJsonContent[collectionKey] || {};
 		packageJsonContent[collectionKey][plugin] = version;
 
 		this.$fs.writeJson(packageJsonPath, packageJsonContent);
 	}
 
-	public removeFromPackageJson(plugin: string, isDev: boolean, projectDir: string) {
+	public removeFromPackageJson(plugin: string, projectDir: string) {
 		const packageJsonPath = this.getPackageJsonFilePath(projectDir);
 		const packageJsonContent = this.$fs.readJson(packageJsonPath);
-		const collection = isDev ? packageJsonContent.devDependencies : packageJsonContent.dependencies;
+		const result = this.removeDependencyFromPackageJsonContent(plugin, packageJsonContent);
 
-		if (collection && collection[plugin]) {
-			delete collection[plugin];
-			this.$fs.writeJson(packageJsonPath, packageJsonContent);
+		if (result.hasModifiedPackageJson) {
+			this.$fs.writeJson(packageJsonPath, result.packageJsonContent);
 		}
 	}
 
@@ -199,6 +203,25 @@ export class PluginsService implements IPluginsService {
 		return pluginData;
 	}
 
+	private removeDependencyFromPackageJsonContent(dependency: string, packageJsonContent: Object): {hasModifiedPackageJson: boolean, packageJsonContent: Object} {
+		let hasModifiedPackageJson = false;
+
+		if (packageJsonContent.devDependencies && packageJsonContent.devDependencies[dependency]) {
+			delete packageJsonContent.devDependencies[dependency];
+			hasModifiedPackageJson = true;
+		}
+
+		if (packageJsonContent.dependencies && packageJsonContent.dependencies[dependency]) {
+			delete packageJsonContent.dependencies[dependency];
+			hasModifiedPackageJson = true;
+		}
+
+		return {
+			hasModifiedPackageJson,
+			packageJsonContent
+		};
+	}
+
 	private getBasicPluginInformation(dependencies: any): IBasePluginData[] {
 		return _.map(dependencies, (version: string, key: string) => ({
 			name: key,