Skip to content

Commit 520ce8d

Browse files
committed
refactor: fix migrate command PR comments
1 parent bb21c3d commit 520ce8d

File tree

6 files changed

+83
-83
lines changed

6 files changed

+83
-83
lines changed

lib/commands/migrate.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export class MigrateCommand implements ICommand {
1414

1515
public async canExecute(args: string[]): Promise<boolean> {
1616
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".');
1818
}
1919

2020
return true;

lib/controllers/migrate-controller.ts

Lines changed: 50 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import * as path from "path";
22
import * as semver from "semver";
33
import * as constants from "../constants";
4-
import { BaseUpdateController } from "./base-update-controller";
4+
import { UpdateControllerBase } from "./update-controller-base";
55

6-
export class MigrateController extends BaseUpdateController implements IMigrateController {
6+
export class MigrateController extends UpdateControllerBase implements IMigrateController {
77
constructor(
88
protected $fs: IFileSystem,
99
protected $platformCommandHelper: IPlatformCommandHelper,
@@ -12,6 +12,7 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
1212
protected $packageManager: IPackageManager,
1313
private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
1414
private $logger: ILogger,
15+
private $errors: IErrors,
1516
private $addPlatformService: IAddPlatformService,
1617
private $pluginsService: IPluginsService,
1718
private $projectDataService: IProjectDataService) {
@@ -28,15 +29,15 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
2829
];
2930

3031
static readonly migrationDependencies : IMigrationDependency[] = [
31-
{ packageName: constants.TNS_CORE_MODULES_NAME, isDev: false, verifiedVersion: "6.0.0-next-2019-06-10-092158-01"},
32-
{ packageName: constants.TNS_CORE_MODULES_WIDGETS_NAME, isDev: false, verifiedVersion: "6.0.0-next-2019-06-10-092158-01"},
32+
{ packageName: constants.TNS_CORE_MODULES_NAME, verifiedVersion: "6.0.0-next-2019-06-10-092158-01"},
33+
{ packageName: constants.TNS_CORE_MODULES_WIDGETS_NAME, verifiedVersion: "6.0.0-next-2019-06-10-092158-01"},
3334
{ packageName: "node-sass", isDev: true, verifiedVersion: "4.12.0"},
3435
{ packageName: "typescript", isDev: true, verifiedVersion: "3.4.1"},
3536
{ packageName: "less", isDev: true, verifiedVersion: "3.9.0"},
3637
{ packageName: "nativescript-dev-sass", isDev: true, replaceWith: "node-sass"},
3738
{ packageName: "nativescript-dev-typescript", isDev: true, replaceWith: "typescript"},
3839
{ packageName: "nativescript-dev-less", isDev: true, replaceWith: "less"},
39-
{ packageName: constants.WEBPACK_PLUGIN_NAME, isDev: true, shouldAdd: true, verifiedVersion: "0.25.0-webpack-2019-06-11-105349-01"},
40+
{ packageName: constants.WEBPACK_PLUGIN_NAME, isDev: true, shouldAddIfMissing: true, verifiedVersion: "0.25.0-webpack-2019-06-11-105349-01"},
4041
{ packageName: "nativescript-camera", verifiedVersion: "4.5.0"},
4142
{ packageName: "nativescript-geolocation", verifiedVersion: "5.1.0"},
4243
{ packageName: "nativescript-imagepicker", verifiedVersion: "6.2.0"},
@@ -50,16 +51,16 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
5051
{ packageName: "nativescript-ui-autocomplete", verifiedVersion: "5.0.0-androidx-110619"},
5152
{ packageName: "nativescript-datetimepicker", verifiedVersion: "1.1.0"},
5253
//TODO update with compatible with webpack only hooks
53-
{ packageName: "kinvey-nativescript-sdk", verifiedVersion: "4.1.0"},
54+
{ packageName: "kinvey-nativescript-sdk", verifiedVersion: "4.2.1"},
5455
//TODO update with compatible with webpack only hooks
55-
{ packageName: "nativescript-plugin-firebase", verifiedVersion: "8.3.2"},
56+
{ packageName: "nativescript-plugin-firebase", verifiedVersion: "9.0.1"},
5657
//TODO update with no prerelease version compatible with webpack only hooks
5758
{ packageName: "nativescript-vue", verifiedVersion: "2.3.0-rc.0"},
5859
{ packageName: "nativescript-permissions", verifiedVersion: "1.3.0"},
5960
{ packageName: "nativescript-cardview", verifiedVersion: "3.2.0"}
6061
];
6162

62-
static readonly tempFolder: string = ".migration_backup";
63+
static readonly backupFolder: string = ".migration_backup";
6364
static readonly migrateFailMessage: string = "Could not migrate the project!";
6465
static readonly backupFailMessage: string = "Could not backup project folders!";
6566

@@ -72,42 +73,50 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
7273

7374
public async migrate({projectDir}: {projectDir: string}): Promise<void> {
7475
const projectData = this.$projectDataService.getProjectData(projectDir);
75-
const tmpDir = path.join(projectDir, MigrateController.tempFolder);
76+
const backupDir = path.join(projectDir, MigrateController.backupFolder);
7677

7778
try {
7879
this.$logger.info("Backup project configuration.");
79-
this.backup(MigrateController.folders, tmpDir, projectData);
80+
this.backup(MigrateController.folders, backupDir, projectData.projectDir);
8081
this.$logger.info("Backup project configuration complete.");
8182
} catch (error) {
8283
this.$logger.error(MigrateController.backupFailMessage);
83-
this.$fs.deleteDirectory(tmpDir);
84+
this.$fs.deleteDirectory(backupDir);
8485
return;
8586
}
8687

8788
try {
8889
await this.cleanUpProject(projectData);
8990
await this.migrateDependencies(projectData);
9091
} catch (error) {
91-
this.restoreBackup(MigrateController.folders, tmpDir, projectData);
92+
this.restoreBackup(MigrateController.folders, backupDir, projectData.projectDir);
9293
this.$logger.error(MigrateController.migrateFailMessage);
9394
}
9495
}
9596

9697
public async shouldMigrate({projectDir}: IProjectDir): Promise<boolean> {
9798
const projectData = this.$projectDataService.getProjectData(projectDir);
99+
98100
for (let i = 0; i < MigrateController.migrationDependencies.length; i++) {
99101
const dependency = MigrateController.migrationDependencies[i];
100-
const collection = dependency.isDev ? projectData.devDependencies : projectData.dependencies;
101-
if (dependency.replaceWith && collection && collection[dependency.packageName]) {
102+
const hasDependency = this.hasDependency(dependency, projectData);
103+
104+
if (hasDependency && dependency.replaceWith) {
105+
return true;
106+
}
107+
108+
if (hasDependency && await this.shouldMigrateDependencyVersion(dependency, projectData)) {
102109
return true;
103110
}
104111

105-
if (!this.shouldSkipMigrationDependency(dependency, projectData) && await this.shouldMigrateDependencyVersion(dependency, projectData)) {
112+
if (!hasDependency && dependency.shouldAddIfMissing) {
106113
return true;
107114
}
108115
}
116+
109117
for (const platform in this.$devicePlatformsConstants) {
110-
if (await this.shouldUpdateRuntimeVersion({ targetVersion: this.verifiedPlatformVersions[platform.toLowerCase()], platform, projectData, shouldAdd: true})) {
118+
const hasRuntimeDependency = this.hasRuntimeDependency({platform, projectData});
119+
if (!hasRuntimeDependency || await this.shouldUpdateRuntimeVersion({ targetVersion: this.verifiedPlatformVersions[platform.toLowerCase()], platform, projectData})) {
111120
return true;
112121
}
113122
}
@@ -125,27 +134,39 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
125134
}
126135

127136
private async migrateDependencies(projectData: IProjectData): Promise<void> {
128-
this.$logger.info("Start migrating dependencies.");
137+
this.$logger.info("Start dependencies migration.");
129138
for (let i = 0; i < MigrateController.migrationDependencies.length; i++) {
130139
const dependency = MigrateController.migrationDependencies[i];
131-
if (this.shouldSkipMigrationDependency(dependency, projectData)) {
132-
continue;
133-
}
140+
const hasDependency = this.hasDependency(dependency, projectData);
134141

135-
if (dependency.replaceWith) {
142+
if (hasDependency && dependency.replaceWith) {
136143
this.$pluginsService.removeFromPackageJson(dependency.packageName, dependency.isDev, projectData.projectDir);
137144
const replacementDep = _.find(MigrateController.migrationDependencies, migrationPackage => migrationPackage.packageName === dependency.replaceWith);
145+
if (!replacementDep) {
146+
this.$errors.failWithoutHelp("Failed to find replacement dependency.");
147+
}
138148
this.$logger.info(`Replacing '${dependency.packageName}' with '${replacementDep.packageName}'.`, );
139149
this.$pluginsService.addToPackageJson(replacementDep.packageName, replacementDep.verifiedVersion, replacementDep.isDev, projectData.projectDir);
140-
} else if (await this.shouldMigrateDependencyVersion(dependency, projectData)) {
150+
continue;
151+
}
152+
153+
if (hasDependency && await this.shouldMigrateDependencyVersion(dependency, projectData)) {
141154
this.$logger.info(`Updating '${dependency.packageName}' to compatible version '${dependency.verifiedVersion}'`);
142155
this.$pluginsService.addToPackageJson(dependency.packageName, dependency.verifiedVersion, dependency.isDev, projectData.projectDir);
156+
continue;
157+
}
158+
159+
if (!hasDependency && dependency.shouldAddIfMissing) {
160+
this.$logger.info(`Adding '${dependency.packageName}' with version '${dependency.verifiedVersion}'`);
161+
this.$pluginsService.addToPackageJson(dependency.packageName, dependency.verifiedVersion, dependency.isDev, projectData.projectDir);
162+
continue;
143163
}
144164
}
145165

146166
for (const platform in this.$devicePlatformsConstants) {
147167
const lowercasePlatform = platform.toLowerCase();
148-
if (await this.shouldUpdateRuntimeVersion({targetVersion: this.verifiedPlatformVersions[lowercasePlatform], platform, projectData, shouldAdd: true})) {
168+
const hasRuntimeDependency = this.hasRuntimeDependency({platform, projectData});
169+
if (!hasRuntimeDependency || await this.shouldUpdateRuntimeVersion({targetVersion: this.verifiedPlatformVersions[lowercasePlatform], platform, projectData})) {
149170
const verifiedPlatformVersion = this.verifiedPlatformVersions[lowercasePlatform];
150171
const platformData = this.$platformsDataService.getPlatformData(lowercasePlatform, projectData);
151172
this.$logger.info(`Updating ${platform} platform to version '${verifiedPlatformVersion}'.`);
@@ -166,38 +187,15 @@ export class MigrateController extends BaseUpdateController implements IMigrateC
166187

167188
private async shouldMigrateDependencyVersion(dependency: IMigrationDependency, projectData: IProjectData): Promise<boolean> {
168189
const collection = dependency.isDev ? projectData.devDependencies : projectData.dependencies;
169-
if (
170-
collection &&
171-
collection[dependency.packageName]
172-
) {
173-
const maxSatisfyingVersion = await this.getMaxDependencyVersion(dependency.packageName, collection[dependency.packageName]);
174-
if (maxSatisfyingVersion) {
175-
const isPrereleaseVersion = semver.prerelease(maxSatisfyingVersion);
176-
const coerceMaxSatisfying = semver.coerce(maxSatisfyingVersion).version;
177-
const coerceVerifiedVersion = semver.coerce(dependency.verifiedVersion).version;
178-
//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
179-
if (isPrereleaseVersion) {
180-
if (semver.gt(coerceMaxSatisfying, coerceVerifiedVersion)) {
181-
return false;
182-
}
183-
184-
//TODO This should be removed once we update the verified versions to no prerelease versions
185-
if (semver.eq(maxSatisfyingVersion, dependency.verifiedVersion)) {
186-
return false;
187-
}
188-
} else if (semver.gte(coerceMaxSatisfying, coerceVerifiedVersion)) {
189-
return false;
190-
}
191-
}
192-
}
190+
const maxSatisfyingVersion = await this.getMaxDependencyVersion(dependency.packageName, collection[dependency.packageName]);
193191

194-
return true;
192+
return !(maxSatisfyingVersion && semver.gte(maxSatisfyingVersion, dependency.verifiedVersion));
195193
}
196194

197-
private shouldSkipMigrationDependency(dependency: IMigrationDependency, projectData: IProjectData) {
198-
if (!dependency.shouldAdd) {
199-
return this.shouldSkipDependency(dependency, projectData);
200-
}
195+
protected async shouldUpdateRuntimeVersion({targetVersion, platform, projectData}: {targetVersion: string, platform: string, projectData: IProjectData}): Promise<boolean> {
196+
const maxRuntimeVersion = await this.getMaxRuntimeVersion({platform, projectData});
197+
198+
return !(maxRuntimeVersion && semver.gte(maxRuntimeVersion, targetVersion));
201199
}
202200
}
203201

lib/controllers/base-update-controller.ts renamed to lib/controllers/update-controller-base.ts

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,63 @@
11
import * as path from "path";
22
import * as semver from "semver";
33

4-
export class BaseUpdateController {
4+
export class UpdateControllerBase {
55
constructor(protected $fs: IFileSystem,
66
protected $platformCommandHelper: IPlatformCommandHelper,
77
protected $platformsDataService: IPlatformsDataService,
88
protected $packageInstallationManager: IPackageInstallationManager,
99
protected $packageManager: IPackageManager) {
1010
}
1111

12-
protected restoreBackup(folders: string[], tmpDir: string, projectData: IProjectData): void {
12+
protected restoreBackup(folders: string[], backupDir: string, projectDir: string): void {
1313
for (const folder of folders) {
14-
this.$fs.deleteDirectory(path.join(projectData.projectDir, folder));
14+
this.$fs.deleteDirectory(path.join(projectDir, folder));
1515

16-
const folderToCopy = path.join(tmpDir, folder);
16+
const folderToCopy = path.join(backupDir, folder);
1717

1818
if (this.$fs.exists(folderToCopy)) {
19-
this.$fs.copyFile(folderToCopy, projectData.projectDir);
19+
this.$fs.copyFile(folderToCopy, projectDir);
2020
}
2121
}
2222
}
2323

24-
protected backup(folders: string[], tmpDir: string, projectData: IProjectData): void {
25-
this.$fs.deleteDirectory(tmpDir);
26-
this.$fs.createDirectory(tmpDir);
24+
protected backup(folders: string[], backupDir: string, projectDir: string): void {
25+
this.$fs.deleteDirectory(backupDir);
26+
this.$fs.createDirectory(backupDir);
2727
for (const folder of folders) {
28-
const folderToCopy = path.join(projectData.projectDir, folder);
28+
const folderToCopy = path.join(projectDir, folder);
2929
if (this.$fs.exists(folderToCopy)) {
30-
this.$fs.copyFile(folderToCopy, tmpDir);
30+
this.$fs.copyFile(folderToCopy, backupDir);
3131
}
3232
}
3333
}
3434

35-
protected shouldSkipDependency(dependency: IDependency, projectData: IProjectData): boolean {
35+
protected hasDependency(dependency: IDependency, projectData: IProjectData): boolean {
3636
const collection = dependency.isDev ? projectData.devDependencies : projectData.dependencies;
37-
if (!collection[dependency.packageName]) {
37+
if (collection && collection[dependency.packageName]) {
3838
return true;
3939
}
4040
}
4141

42-
protected async shouldUpdateRuntimeVersion({targetVersion, platform, projectData, shouldAdd}: {targetVersion: string, platform: string, projectData: IProjectData, shouldAdd: boolean}) {
42+
protected hasRuntimeDependency({platform, projectData}: {platform: string, projectData: IProjectData}): boolean {
4343
const lowercasePlatform = platform.toLowerCase();
4444
const currentPlatformVersion = this.$platformCommandHelper.getCurrentPlatformVersion(lowercasePlatform, projectData);
45-
const platformData = this.$platformsDataService.getPlatformData(lowercasePlatform, projectData);
45+
4646
if (currentPlatformVersion) {
47-
const maxPlatformSatisfyingVersion = await this.getMaxDependencyVersion(platformData.frameworkPackageName, currentPlatformVersion) || currentPlatformVersion;
48-
if (semver.gte(maxPlatformSatisfyingVersion, targetVersion)) {
49-
return false;
50-
}
51-
} else if (!shouldAdd) {
52-
return false;
47+
return true;
5348
}
49+
}
5450

55-
return true;
51+
protected async getMaxRuntimeVersion({platform, projectData}: {platform: string, projectData: IProjectData}) {
52+
const lowercasePlatform = platform.toLowerCase();
53+
const currentPlatformVersion = this.$platformCommandHelper.getCurrentPlatformVersion(lowercasePlatform, projectData);
54+
const platformData = this.$platformsDataService.getPlatformData(lowercasePlatform, projectData);
55+
if (currentPlatformVersion) {
56+
return await this.getMaxDependencyVersion(platformData.frameworkPackageName, currentPlatformVersion) || currentPlatformVersion;
57+
}
5658
}
5759

58-
protected async getMaxDependencyVersion(dependency: string, version: string) {
60+
protected async getMaxDependencyVersion(dependency: string, version: string): Promise<string> {
5961
let maxDependencyVersion;
6062
if (semver.valid(version)) {
6163
maxDependencyVersion = version;

lib/definitions/migrate.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ interface IDependency {
99
}
1010

1111
interface IMigrationDependency extends IDependency {
12-
mustRemove?: boolean;
12+
shouldRemove?: boolean;
1313
replaceWith?: string;
1414
verifiedVersion?: string;
15-
shouldAdd?: boolean;
15+
shouldAddIfMissing?: boolean;
1616
}

lib/definitions/plugins.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
interface IPluginsService {
22
add(plugin: string, projectData: IProjectData): Promise<void>; // adds plugin by name, github url, local path and et.
33
remove(pluginName: string, projectData: IProjectData): Promise<void>; // removes plugin only by name
4-
addToPackageJson(plugin: string, version: string, dev: boolean, projectDir: string): void;
5-
removeFromPackageJson(plugin: string, dev: boolean, projectDir: string): void;
4+
addToPackageJson(plugin: string, version: string, isDev: boolean, projectDir: string): void;
5+
removeFromPackageJson(plugin: string, isDev: boolean, projectDir: string): void;
66
getAllInstalledPlugins(projectData: IProjectData): Promise<IPluginData[]>;
77
ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise<void>;
88

lib/services/plugins-service.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,21 +88,21 @@ export class PluginsService implements IPluginsService {
8888
}
8989
}
9090

91-
public addToPackageJson(plugin: string, version: string, dev: boolean, projectDir: string) {
91+
public addToPackageJson(plugin: string, version: string, isDev: boolean, projectDir: string) {
9292
const packageJsonPath = this.getPackageJsonFilePath(projectDir);
9393
const packageJsonContent = this.$fs.readJson(packageJsonPath);
94-
const collectionKey = dev ? "devDependencies" : "dependencies";
94+
const collectionKey = isDev ? "devDependencies" : "dependencies";
9595

9696
packageJsonContent[collectionKey] = packageJsonContent[collectionKey] || {};
9797
packageJsonContent[collectionKey][plugin] = version;
9898

9999
this.$fs.writeJson(packageJsonPath, packageJsonContent);
100100
}
101101

102-
public removeFromPackageJson(plugin: string, dev: boolean, projectDir: string) {
102+
public removeFromPackageJson(plugin: string, isDev: boolean, projectDir: string) {
103103
const packageJsonPath = this.getPackageJsonFilePath(projectDir);
104104
const packageJsonContent = this.$fs.readJson(packageJsonPath);
105-
const collection = dev ? packageJsonContent.devDependencies : packageJsonContent.dependencies;
105+
const collection = isDev ? packageJsonContent.devDependencies : packageJsonContent.dependencies;
106106

107107
if (collection && collection[plugin]) {
108108
delete collection[plugin];

0 commit comments

Comments
 (0)