Merge pull request #4816 from NativeScript/tachev/improve-migration

Improve the `migrate` command based on the feedback

Author: Dimitar Tachev

lib/controllers/migrate-controller.ts

@@ -12,6 +12,7 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 		protected $platformsDataService: IPlatformsDataService,
 		protected $packageInstallationManager: IPackageInstallationManager,
 		protected $packageManager: IPackageManager,
+		private $androidResourcesMigrationService: IAndroidResourcesMigrationService,
 		private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
 		private $logger: ILogger,
 		private $errors: IErrors,
@@ -37,16 +38,15 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 	];
 
 	private migrationDependencies: IMigrationDependency[] = [
-		{ packageName: constants.TNS_CORE_MODULES_NAME, verifiedVersion: "6.0.0-rc-2019-06-28-175837-02" },
+		{ packageName: constants.TNS_CORE_MODULES_NAME, verifiedVersion: "6.0.0-rc-2019-07-08-111131-01" },
 		{ packageName: constants.TNS_CORE_MODULES_WIDGETS_NAME, verifiedVersion: "6.0.0" },
 		{ packageName: "tns-platform-declarations", isDev: true, verifiedVersion: "6.0.0-rc-2019-06-28-175837-02" },
 		{ 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, shouldAddIfMissing: true, verifiedVersion: "1.0.0-rc-2019-07-02-161545-02" },
+		{ packageName: "nativescript-dev-less", isDev: true, shouldRemove: true, warning: "LESS CSS is not supported out of the box. In order to enable it, follow the steps in this feature request: https://github.com/NativeScript/nativescript-dev-webpack/issues/967" },
+		{ packageName: constants.WEBPACK_PLUGIN_NAME, isDev: true, shouldAddIfMissing: true, verifiedVersion: "1.0.0-rc-2019-07-08-135456-03" },
 		{ packageName: "nativescript-camera", verifiedVersion: "4.5.0" },
 		{ packageName: "nativescript-geolocation", verifiedVersion: "5.1.0" },
 		{ packageName: "nativescript-imagepicker", verifiedVersion: "6.2.0" },
@@ -68,7 +68,7 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 		{ packageName: "nativescript-permissions", verifiedVersion: "1.3.0" },
 		{ packageName: "nativescript-cardview", verifiedVersion: "3.2.0" },
 		{
-			packageName: "nativescript-unit-test-runner", verifiedVersion: "0.6.3",
+			packageName: "nativescript-unit-test-runner", verifiedVersion: "0.6.4",
 			shouldMigrateAction: (projectData: IProjectData) => this.hasDependency({ packageName: "nativescript-unit-test-runner", isDev: false }, projectData),
 			migrateAction: this.migrateUnitTestRunner.bind(this)
 		}
@@ -103,6 +103,8 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 			this.$logger.trace(`Error during auto-generated files handling. ${(error && error.message) || error}`);
 		}
 
+		await this.migrateOldAndroidAppResources(projectData);
+
 		try {
 			await this.cleanUpProject(projectData);
 			await this.migrateDependencies(projectData);
@@ -112,6 +114,14 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 		}
 	}
 
+	private async migrateOldAndroidAppResources(projectData: IProjectData) {
+		const appResourcesPath = projectData.getAppResourcesDirectoryPath();
+		if (!this.$androidResourcesMigrationService.hasMigrated(appResourcesPath)) {
+			this.$logger.info("Migrate old Android App_Resources structure.");
+			await this.$androidResourcesMigrationService.migrate(appResourcesPath);
+		}
+	}
+
 	public async shouldMigrate({ projectDir }: IProjectDir): Promise<boolean> {
 		const projectData = this.$projectDataService.getProjectData(projectDir);
 
@@ -123,7 +133,7 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 				return true;
 			}
 
-			if (hasDependency && dependency.replaceWith) {
+			if (hasDependency && (dependency.replaceWith || dependency.shouldRemove)) {
 				return true;
 			}
 
@@ -134,6 +144,10 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 			if (!hasDependency && dependency.shouldAddIfMissing) {
 				return true;
 			}
+
+			if (!this.$androidResourcesMigrationService.hasMigrated(projectData.getAppResourcesDirectoryPath())) {
+				return true;
+			}
 		}
 
 		for (const platform in this.$devicePlatformsConstants) {
@@ -146,6 +160,7 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 
 	private async cleanUpProject(projectData: IProjectData): Promise<void> {
 		this.$logger.info("Clean old project artefacts.");
+		this.$projectDataService.removeNSConfigProperty(projectData.projectDir, "useLegacyWorkflow");
 		this.$fs.deleteDirectory(path.join(projectData.projectDir, constants.HOOKS_DIR_NAME));
 		this.$fs.deleteDirectory(path.join(projectData.projectDir, constants.PLATFORMS_DIR_NAME));
 		this.$fs.deleteDirectory(path.join(projectData.projectDir, constants.NODE_MODULES_FOLDER_NAME));
@@ -244,15 +259,21 @@ export class MigrateController extends UpdateControllerBase implements IMigrateC
 
 	private async migrateDependency(dependency: IMigrationDependency, projectData: IProjectData): Promise<void> {
 		const hasDependency = this.hasDependency(dependency, projectData);
+		if (dependency.warning) {
+			this.$logger.warn(dependency.warning);
+		}
 
-		if (hasDependency && dependency.replaceWith) {
+		if (hasDependency && (dependency.replaceWith || dependency.shouldRemove)) {
 			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.");
+			if (dependency.replaceWith) {
+				const replacementDep = _.find(this.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);
 			}
-			this.$logger.info(`Replacing '${dependency.packageName}' with '${replacementDep.packageName}'.`);
-			this.$pluginsService.addToPackageJson(replacementDep.packageName, replacementDep.verifiedVersion, replacementDep.isDev, projectData.projectDir);
+
 			return;
 		}
 

lib/definitions/migrate.d.ts

@@ -11,6 +11,7 @@ interface IDependency {
 interface IMigrationDependency extends IDependency {
 	shouldRemove?: boolean;
 	replaceWith?: string;
+	warning?: string;
 	verifiedVersion?: string;
 	shouldAddIfMissing?: boolean;
 	shouldMigrateAction?: (projectData: IProjectData) => boolean;

lib/definitions/project.d.ts

@@ -74,7 +74,6 @@ interface INsConfig {
 	appPath?: string;
 	appResourcesPath?: string;
 	shared?: boolean;
-	useLegacyWorkflow?: boolean;
 	previewAppSchema?: string;
 }
 
@@ -101,11 +100,6 @@ interface IProjectData extends ICreateProjectData {
 	 */
 	isShared: boolean;
 
-	/**
-	 * Defines if the project has hmr enabled by default
-	 */
-	useLegacyWorkflow: boolean;
-
 	/**
 	 * Defines the schema for the preview app
 	 */
@@ -150,6 +144,14 @@ interface IProjectDataService {
 	 */
 	removeNSProperty(projectDir: string, propertyName: string): void;
 
+	/**
+	 * Removes a property from `nsconfig.json`.
+	 * @param {string} projectDir The project directory - the place where the `nsconfig.json` is located.
+	 * @param {string} propertyName The name of the property to be removed.
+	 * @returns {void}
+	 */
+	removeNSConfigProperty(projectDir: string, propertyName: string): void;
+
 	/**
 	 * Removes dependency from package.json
 	 * @param {string} projectDir The project directory - the place where the root package.json is located.
@@ -192,12 +194,12 @@ interface IProjectDataService {
 	 */
 	getAppExecutableFiles(projectDir: string): string[];
 
-		/**
-	 * Returns a value from `nativescript` key in project's package.json.
-	 * @param {string} jsonData The project directory - the place where the root package.json is located.
-	 * @param {string} propertyName The name of the property to be checked in `nativescript` key.
-	 * @returns {any} The value of the property.
-	 */
+	/**
+ * Returns a value from `nativescript` key in project's package.json.
+ * @param {string} jsonData The project directory - the place where the root package.json is located.
+ * @param {string} propertyName The name of the property to be checked in `nativescript` key.
+ * @returns {any} The value of the property.
+ */
 	getNSValueFromContent(jsonData: Object, propertyName: string): any;
 }
 
@@ -496,7 +498,7 @@ interface IRemoveExtensionsOptions {
 	pbxProjPath: string
 }
 
-interface IRemoveWatchAppOptions extends IRemoveExtensionsOptions{}
+interface IRemoveWatchAppOptions extends IRemoveExtensionsOptions { }
 
 interface IRubyFunction {
 	functionName: string;

lib/project-data.ts

@@ -61,7 +61,6 @@ export class ProjectData implements IProjectData {
 	public buildXcconfigPath: string;
 	public podfilePath: string;
 	public isShared: boolean;
-	public useLegacyWorkflow: boolean;
 	public previewAppSchema: string;
 
 	constructor(private $fs: IFileSystem,
@@ -137,7 +136,6 @@ export class ProjectData implements IProjectData {
 			this.buildXcconfigPath = path.join(this.appResourcesDirectoryPath, this.$devicePlatformsConstants.iOS, constants.BUILD_XCCONFIG_FILE_NAME);
 			this.podfilePath = path.join(this.appResourcesDirectoryPath, this.$devicePlatformsConstants.iOS, constants.PODFILE_NAME);
 			this.isShared = !!(this.nsConfig && this.nsConfig.shared);
-			this.useLegacyWorkflow = this.nsConfig && this.nsConfig.useLegacyWorkflow;
 			this.previewAppSchema = this.nsConfig && this.nsConfig.previewAppSchema;
 			return;
 		}

lib/services/project-data-service.ts

@@ -1,5 +1,7 @@
 import * as path from "path";
 import { ProjectData } from "../project-data";
+import * as constants from "../constants";
+import { parseJson } from "../common/helpers";
 import { exported } from "../common/decorators";
 import {
 	NATIVESCRIPT_PROPS_INTERNAL_DELIMITER,
@@ -125,6 +127,12 @@ export class ProjectDataService implements IProjectDataService {
 		};
 	}
 
+	public removeNSConfigProperty(projectDir: string, propertyName: string): void {
+		this.$logger.trace(`Removing "${propertyName}" property from nsconfig.`);
+		this.updateNsConfigValue(projectDir, null, [propertyName]);
+		this.$logger.trace(`"${propertyName}" property successfully removed.`);
+	}
+
 	@exported("projectDataService")
 	public async getAndroidAssetsStructure(opts: IProjectDir): Promise<IAssetGroup> {
 		// TODO: Use image-size package to get the width and height of an image.
@@ -180,6 +188,43 @@ export class ProjectDataService implements IProjectDataService {
 
 		return files;
 	}
+	private refreshProjectData(projectDir: string) {
+		if (this.projectDataCache[projectDir]) {
+			this.projectDataCache[projectDir].initializeProjectData(projectDir);
+		}
+	}
+
+	private updateNsConfigValue(projectDir: string, updateObject?: INsConfig, propertiesToRemove?: string[]): void {
+		const nsConfigPath = path.join(projectDir, constants.CONFIG_NS_FILE_NAME);
+		const currentNsConfig = this.getNsConfig(nsConfigPath);
+		let newNsConfig = currentNsConfig;
+		if (updateObject) {
+			newNsConfig = _.assign(newNsConfig || this.getNsConfigDefaultObject(), updateObject);
+		}
+
+		if (newNsConfig && propertiesToRemove && propertiesToRemove.length) {
+			newNsConfig = _.omit(newNsConfig, propertiesToRemove);
+		}
+
+		if (newNsConfig) {
+			this.$fs.writeJson(nsConfigPath, newNsConfig);
+			this.refreshProjectData(projectDir);
+		}
+	}
+
+	private getNsConfig(nsConfigPath: string): INsConfig {
+		let result: INsConfig = null;
+		if (this.$fs.exists(nsConfigPath)) {
+			const nsConfigContent = this.$fs.readText(nsConfigPath);
+			try {
+				result = <INsConfig>parseJson(nsConfigContent);
+			} catch (e) {
+				this.$logger.trace("The `nsconfig` content is not a valid JSON. Parse error: ", e);
+			}
+		}
+
+		return result;
+	}
 
 	private getImageDefinitions(): IImageDefinitionsStructure {
 		const pathToImageDefinitions = path.join(__dirname, "..", "..", CLI_RESOURCES_DIR_NAME, AssetConstants.assets, AssetConstants.imageDefinitionsFileName);
@@ -334,7 +379,7 @@ export class ProjectDataService implements IProjectDataService {
 	}
 
 	private getNsConfigDefaultObject(data?: Object): INsConfig {
-		const config: INsConfig = { useLegacyWorkflow: false };
+		const config: INsConfig = {};
 		Object.assign(config, data);
 
 		return config;

test/options.ts

@@ -263,73 +263,6 @@ describe("options", () => {
 	});
 
 	describe("setupOptions", () => {
-		const testCases = [
-			{
-				name: "no options are provided",
-				args: [],
-				data: [
-					{ useLegacyWorkflow: undefined, expectedHmr: true, expectedBundle: true },
-					{ useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true },
-					{ useLegacyWorkflow: true, expectedHmr: true, expectedBundle: true }
-				]
-			},
-			{
-				name: " --hmr is provided",
-				args: ["--hmr"],
-				data: [
-					{ useLegacyWorkflow: undefined, expectedHmr: true, expectedBundle: true },
-					{ useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true },
-					{ useLegacyWorkflow: true, expectedHmr: true, expectedBundle: true }
-				]
-			},
-			{
-				name: " --no-hmr is provided",
-				args: ["--no-hmr"],
-				data: [
-					{ useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: true },
-					{ useLegacyWorkflow: false, expectedHmr: false, expectedBundle: true },
-					{ useLegacyWorkflow: true, expectedHmr: false, expectedBundle: true }
-				]
-			},
-			{
-				name: " --bundle is provided",
-				args: ["--bundle"],
-				data: [
-					{ useLegacyWorkflow: undefined, expectedHmr: true, expectedBundle: true },
-					{ useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true },
-					{ useLegacyWorkflow: true, expectedHmr: true, expectedBundle: true }
-				]
-			},
-			{
-				name: " --release is provided",
-				args: ["--release"],
-				data: [
-					{ useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: true },
-					{ useLegacyWorkflow: false, expectedHmr: false, expectedBundle: true },
-					{ useLegacyWorkflow: true, expectedHmr: false, expectedBundle: true }
-				]
-			}
-		];
-
-		_.each([undefined, false, true], useLegacyWorkflow => {
-			_.each(testCases, testCase => {
-				it(`should pass correctly when ${testCase.name} and useLegacyWorkflow is ${useLegacyWorkflow}`, () => {
-					(testCase.args || []).forEach(arg => process.argv.push(arg));
-
-					const options: any = createOptions(testInjector);
-					const projectData = <IProjectData>{ useLegacyWorkflow };
-					options.setupOptions(projectData);
-
-					(testCase.args || []).forEach(arg => process.argv.pop());
-
-					const data = testCase.data.find(item => item.useLegacyWorkflow === useLegacyWorkflow);
-
-					assert.equal(!!options.argv.hmr, !!data.expectedHmr);
-					assert.equal(!!options.argv.bundle, !!data.expectedBundle);
-				});
-			});
-		});
-
 		const testCasesExpectingToThrow = [
 			{
 				name: "--release --hmr",