Skip to content

Commit 571e91d

Browse files
author
Fatme
authored
Merge pull request #3929 from NativeScript/fatme/skip-platform-add
fix: skip platform add on every change when `tns preview --bundle` command is executed
2 parents b59e85a + 598d22c commit 571e91d

File tree

4 files changed

+192
-16
lines changed

4 files changed

+192
-16
lines changed

‎lib/services/platform-service.ts‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ export class PlatformService extends EventEmitter implements IPlatformService {
4141
private $projectChangesService: IProjectChangesService,
4242
private $analyticsService: IAnalyticsService,
4343
private $terminalSpinnerService: ITerminalSpinnerService,
44-
private $pacoteService: IPacoteService
44+
private $pacoteService: IPacoteService,
45+
private $usbLiveSyncService: any
4546
) {
4647
super();
4748
}
@@ -726,7 +727,7 @@ export class PlatformService extends EventEmitter implements IPlatformService {
726727
platform = data[0],
727728
version = data[1];
728729

729-
if (this.isPlatformInstalled(platform, projectData)) {
730+
if (this.hasPlatformDirectory(platform, projectData)) {
730731
await this.updatePlatform(platform, version, platformTemplate, projectData, config);
731732
} else {
732733
await this.addPlatform(platformParam, platformTemplate, projectData, config);
@@ -769,7 +770,7 @@ export class PlatformService extends EventEmitter implements IPlatformService {
769770
public validatePlatformInstalled(platform: string, projectData: IProjectData): void {
770771
this.validatePlatform(platform, projectData);
771772

772-
if (!this.isPlatformInstalled(platform, projectData)) {
773+
if (!this.hasPlatformDirectory(platform, projectData)) {
773774
this.$errors.fail("The platform %s is not added to this project. Please use 'tns platform add <platform>'", platform);
774775
}
775776
}
@@ -779,31 +780,50 @@ export class PlatformService extends EventEmitter implements IPlatformService {
779780

780781
const platformData = this.$platformsData.getPlatformData(platform, projectData);
781782
const prepareInfo = this.$projectChangesService.getPrepareInfo(platform, projectData);
782-
// In case when no platform is added and webpack plugin is started it produces files in platforms folder. In this case {N} CLI needs to add platform and keeps the already produced files from webpack
783-
if (appFilesUpdaterOptions.bundle && this.isPlatformInstalled(platform, projectData) && !this.$fs.exists(platformData.configurationFilePath) && (!prepareInfo || !prepareInfo.nativePlatformStatus || prepareInfo.nativePlatformStatus !== constants.NativePlatformStatus.alreadyPrepared)) {
784-
const tmpDirectoryPath = path.join(projectData.projectDir, "platforms", `tmp-${platform}`);
785-
this.$fs.deleteDirectory(tmpDirectoryPath);
786-
this.$fs.ensureDirectoryExists(tmpDirectoryPath);
787-
this.$fs.copyFile(path.join(platformData.appDestinationDirectoryPath, "*"), tmpDirectoryPath);
788-
await this.addPlatform(platform, platformTemplate, projectData, config, "", nativePrepare);
789-
this.$fs.copyFile(path.join(tmpDirectoryPath, "*"), platformData.appDestinationDirectoryPath);
790-
this.$fs.deleteDirectory(tmpDirectoryPath);
783+
784+
// In case when no platform is added and webpack plugin is started it produces files in platforms folder.
785+
// In this case {N} CLI needs to add platform and keeps the already produced files from webpack
786+
const shouldPersistWebpackFiles = this.shouldPersistWebpackFiles(platform, projectData, prepareInfo, appFilesUpdaterOptions, nativePrepare);
787+
if (shouldPersistWebpackFiles) {
788+
await this.persistWebpackFiles(platform, platformTemplate, projectData, config, platformData, nativePrepare);
791789
return;
792790
}
793791

794-
if (!this.isPlatformInstalled(platform, projectData)) {
795-
await this.addPlatform(platform, platformTemplate, projectData, config, "", nativePrepare);
796-
} else {
792+
const hasPlatformDirectory = this.hasPlatformDirectory(platform, projectData);
793+
if (hasPlatformDirectory) {
797794
const shouldAddNativePlatform = !nativePrepare || !nativePrepare.skipNativePrepare;
798795
// In case there's no prepare info, it means only platform add had been executed. So we've come from CLI and we do not need to prepare natively.
799796
requiresNativePlatformAdd = prepareInfo && prepareInfo.nativePlatformStatus === constants.NativePlatformStatus.requiresPlatformAdd;
800797
if (requiresNativePlatformAdd && shouldAddNativePlatform) {
801798
await this.addPlatform(platform, platformTemplate, projectData, config, "", nativePrepare);
802799
}
800+
} else {
801+
await this.addPlatform(platform, platformTemplate, projectData, config, "", nativePrepare);
803802
}
804803
}
805804

806-
private isPlatformInstalled(platform: string, projectData: IProjectData): boolean {
805+
private shouldPersistWebpackFiles(platform: string, projectData: IProjectData, prepareInfo: IPrepareInfo, appFilesUpdaterOptions: IAppFilesUpdaterOptions, nativePrepare: INativePrepare): boolean {
806+
const hasPlatformDirectory = this.hasPlatformDirectory(platform, projectData);
807+
const isWebpackWatcherStarted = this.$usbLiveSyncService.isInitialized;
808+
const hasNativePlatformStatus = !prepareInfo || !prepareInfo.nativePlatformStatus;
809+
const requiresPlatformAdd = prepareInfo && prepareInfo.nativePlatformStatus === constants.NativePlatformStatus.requiresPlatformAdd;
810+
const shouldAddNativePlatform = !nativePrepare || !nativePrepare.skipNativePrepare;
811+
const shouldAddPlatform = hasNativePlatformStatus || (requiresPlatformAdd && shouldAddNativePlatform);
812+
const result = appFilesUpdaterOptions.bundle && isWebpackWatcherStarted && hasPlatformDirectory && shouldAddPlatform;
813+
return result;
814+
}
815+
816+
private async persistWebpackFiles(platform: string, platformTemplate: string, projectData: IProjectData, config: IPlatformOptions, platformData: IPlatformData, nativePrepare?: INativePrepare): Promise<void> {
817+
const tmpDirectoryPath = path.join(projectData.projectDir, "platforms", `tmp-${platform}`);
818+
this.$fs.deleteDirectory(tmpDirectoryPath);
819+
this.$fs.ensureDirectoryExists(tmpDirectoryPath);
820+
this.$fs.copyFile(path.join(platformData.appDestinationDirectoryPath, "*"), tmpDirectoryPath);
821+
await this.addPlatform(platform, platformTemplate, projectData, config, "", nativePrepare);
822+
this.$fs.copyFile(path.join(tmpDirectoryPath, "*"), platformData.appDestinationDirectoryPath);
823+
this.$fs.deleteDirectory(tmpDirectoryPath);
824+
}
825+
826+
private hasPlatformDirectory(platform: string, projectData: IProjectData): boolean {
807827
return this.$fs.exists(path.join(projectData.platformsDir, platform.toLowerCase()));
808828
}
809829

‎test/npm-support.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ function createTestInjector(): IInjector {
106106
testInjector.register("pacoteService", {
107107
extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise<void> => undefined
108108
});
109+
testInjector.register("usbLiveSyncService", () => ({}));
109110

110111
return testInjector;
111112
}

‎test/platform-commands.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ function createTestInjector() {
179179
testInjector.register("pacoteService", {
180180
extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise<void> => undefined
181181
});
182+
testInjector.register("usbLiveSyncService", ({}));
182183

183184
return testInjector;
184185
}

‎test/platform-service.ts‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { Messages } from "../lib/common/messages/messages";
2525
import { SettingsService } from "../lib/common/test/unit-tests/stubs";
2626
import { INFO_PLIST_FILE_NAME, MANIFEST_FILE_NAME } from "../lib/constants";
2727
import { mkdir } from "shelljs";
28+
import * as constants from "../lib/constants";
2829

2930
require("should");
3031
const temp = require("temp");
@@ -121,6 +122,7 @@ function createTestInjector() {
121122
}));
122123
}
123124
});
125+
testInjector.register("usbLiveSyncService", () => ({}));
124126

125127
return testInjector;
126128
}
@@ -1004,4 +1006,156 @@ describe('Platform Service Tests', () => {
10041006
});
10051007
});
10061008
});
1009+
1010+
describe("ensurePlatformInstalled", () => {
1011+
const platform = "android";
1012+
const platformTemplate = "testPlatformTemplate";
1013+
const appFilesUpdaterOptions = { bundle: true };
1014+
let isPlatformAdded = false;
1015+
let areWebpackFilesPersisted = false;
1016+
1017+
let projectData: IProjectData = null;
1018+
let usbLiveSyncService: any = null;
1019+
let projectChangesService: IProjectChangesService = null;
1020+
1021+
beforeEach(() => {
1022+
reset();
1023+
1024+
(<any>platformService).addPlatform = () => isPlatformAdded = true;
1025+
(<any>platformService).persistWebpackFiles = () => areWebpackFilesPersisted = true;
1026+
1027+
projectData = testInjector.resolve("projectData");
1028+
usbLiveSyncService = testInjector.resolve("usbLiveSyncService");
1029+
projectChangesService = testInjector.resolve("projectChangesService");
1030+
1031+
usbLiveSyncService.isInitialized = true;
1032+
});
1033+
1034+
function reset() {
1035+
isPlatformAdded = false;
1036+
areWebpackFilesPersisted = false;
1037+
}
1038+
1039+
function mockPrepareInfo(prepareInfo: any) {
1040+
projectChangesService.getPrepareInfo = () => prepareInfo;
1041+
}
1042+
1043+
const testCases = [
1044+
{
1045+
name: "should persist webpack files when prepareInfo is null (first execution of `tns run --bundle`)",
1046+
areWebpackFilesPersisted: true
1047+
},
1048+
{
1049+
name: "should persist webpack files when prepareInfo is null and skipNativePrepare is true (first execution of `tns preview --bundle`)",
1050+
nativePrepare: { skipNativePrepare: true },
1051+
areWebpackFilesPersisted: true
1052+
},
1053+
{
1054+
name: "should not persist webpack files when requires platform add",
1055+
prepareInfo: { nativePlatformStatus: constants.NativePlatformStatus.requiresPlatformAdd },
1056+
areWebpackFilesPersisted: true
1057+
},
1058+
{
1059+
name: "should persist webpack files when requires platform add and skipNativePrepare is true",
1060+
prepareInfo: { nativePlatformStatus: constants.NativePlatformStatus.requiresPlatformAdd },
1061+
nativePrepare: { skipNativePrepare: true },
1062+
areWebpackFilesPersisted: false
1063+
},
1064+
{
1065+
name: "should persist webpack files when platform is already prepared",
1066+
prepareInfo: { nativePlatformStatus: constants.NativePlatformStatus.alreadyPrepared },
1067+
areWebpackFilesPersisted: false
1068+
},
1069+
{
1070+
name: "should not persist webpack files when platform is already prepared and skipNativePrepare is true",
1071+
prepareInfo: { nativePlatformStatus: constants.NativePlatformStatus.alreadyPrepared },
1072+
areWebpackFilesPersisted: false
1073+
},
1074+
{
1075+
name: "should not persist webpack files when no webpack watcher is started (first execution of `tns build --bundle`)",
1076+
isWebpackWatcherStarted: false,
1077+
areWebpackFilesPersisted: false
1078+
},
1079+
{
1080+
name: "should not persist webpack files when no webpack watcher is started and skipNativePrepare is true (local JS prepare from cloud command)",
1081+
isWebpackWatcherStarted: false,
1082+
nativePrepare: { skipNativePrepare: true },
1083+
areWebpackFilesPersisted: false
1084+
}
1085+
];
1086+
1087+
_.each(testCases, (testCase: any) => {
1088+
it(`${testCase.name}`, async () => {
1089+
usbLiveSyncService.isInitialized = testCase.isWebpackWatcherStarted === undefined ? true : testCase.isWebpackWatcherStarted;
1090+
mockPrepareInfo(testCase.prepareInfo);
1091+
1092+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions, testCase.nativePrepare);
1093+
assert.deepEqual(areWebpackFilesPersisted, testCase.areWebpackFilesPersisted);
1094+
});
1095+
});
1096+
1097+
it("should not persist webpack files after the second execution of `tns preview --bundle` or `tns cloud run --bundle`", async () => {
1098+
// First execution of `tns preview --bundle`
1099+
mockPrepareInfo(null);
1100+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions, { skipNativePrepare: true });
1101+
assert.isTrue(areWebpackFilesPersisted);
1102+
1103+
// Second execution of `tns preview --bundle`
1104+
reset();
1105+
mockPrepareInfo({ nativePlatformStatus: constants.NativePlatformStatus.requiresPlatformAdd });
1106+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions, { skipNativePrepare: true });
1107+
assert.isFalse(areWebpackFilesPersisted);
1108+
});
1109+
1110+
it("should not persist webpack files after the second execution of `tns run --bundle`", async () => {
1111+
// First execution of `tns run --bundle`
1112+
mockPrepareInfo(null);
1113+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions);
1114+
assert.isTrue(areWebpackFilesPersisted);
1115+
1116+
// Second execution of `tns run --bundle`
1117+
reset();
1118+
mockPrepareInfo({ nativePlatformStatus: constants.NativePlatformStatus.alreadyPrepared });
1119+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions);
1120+
assert.isFalse(areWebpackFilesPersisted);
1121+
});
1122+
1123+
it("should handle correctly the following sequence of commands: `tns preview --bundle`, `tns run --bundle` and `tns preview --bundle`", async () => {
1124+
// First execution of `tns preview --bundle`
1125+
mockPrepareInfo(null);
1126+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions, { skipNativePrepare: true });
1127+
assert.isTrue(areWebpackFilesPersisted);
1128+
1129+
// Execution of `tns run --bundle`
1130+
reset();
1131+
mockPrepareInfo({ nativePlatformStatus: constants.NativePlatformStatus.requiresPlatformAdd });
1132+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions);
1133+
assert.isTrue(areWebpackFilesPersisted);
1134+
1135+
// Execution of `tns preview --bundle`
1136+
reset();
1137+
mockPrepareInfo({ nativePlatformStatus: constants.NativePlatformStatus.alreadyPrepared });
1138+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions, { skipNativePrepare: true });
1139+
assert.isFalse(areWebpackFilesPersisted);
1140+
});
1141+
1142+
it("should handle correctly the following sequence of commands: `tns preview --bundle`, `tns run --bundle` and `tns build --bundle`", async () => {
1143+
// Execution of `tns preview --bundle`
1144+
mockPrepareInfo(null);
1145+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions, { skipNativePrepare: true });
1146+
assert.isTrue(areWebpackFilesPersisted);
1147+
1148+
// Execution of `tns run --bundle`
1149+
reset();
1150+
mockPrepareInfo({ nativePlatformStatus: constants.NativePlatformStatus.requiresPlatformAdd });
1151+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions);
1152+
assert.isTrue(areWebpackFilesPersisted);
1153+
1154+
// Execution of `tns build --bundle`
1155+
reset();
1156+
mockPrepareInfo({ nativePlatformStatus: constants.NativePlatformStatus.alreadyPrepared });
1157+
await (<any>platformService).ensurePlatformInstalled(platform, platformTemplate, projectData, config, appFilesUpdaterOptions);
1158+
assert.isFalse(areWebpackFilesPersisted);
1159+
});
1160+
});
10071161
});

0 commit comments

Comments
 (0)