Merge pull request #5163 from NativeScript/tachev/fix-hmr-restart

Dimitar Tachev · web-flow · commit 65278fafa0eb · 2019-12-05T16:45:56.000+02:00
fix: sync all files when a HMR update is not possible in order to avoid multiple HMR syncs after restart
diff --git a/Gruntfile.js b/Gruntfile.js
@@ -138,7 +138,6 @@ module.exports = function (grunt) {
 						"isWindows": true,
 						"isMacOS": true,
 						"isLinux": true,
-						"formatListOfNames": () => { },
 						"constants": ""
 					}
 				},
diff --git a/docs/man_pages/project/testing/test-init.md b/docs/man_pages/project/testing/test-init.md
@@ -17,7 +17,7 @@ General | `$ tns test init [--framework <Framework>]`
 
 ### Options
 
-* `--framework <Framework>` - Sets the unit testing framework to install. The following frameworks are available: <%= formatListOfNames(getUnitTestingFrameworkNames(), 'and') %>.
+* `--framework <Framework>` - Sets the unit testing framework to install. The following frameworks are available: mocha, jasmine and qunit.
 
 <% if(isHtml) { %>
 
diff --git a/lib/common/services/micro-templating-service.ts b/lib/common/services/micro-templating-service.ts
@@ -1,13 +1,11 @@
 import * as util from "util";
 import * as os from "os";
 import * as constants from "../../constants";
-import { formatListOfNames } from '../helpers';
 
 export class MicroTemplateService implements IMicroTemplateService {
 	private dynamicCallRegex: RegExp;
 
-	constructor(private $injector: IInjector,
-		private $testInitializationService: ITestInitializationService) {
+	constructor(private $injector: IInjector) {
 		// Injector's dynamicCallRegex doesn't have 'g' option, which we need here.
 		// Use ( ) in order to use $1 to get whole expression later
 		this.dynamicCallRegex = new RegExp(util.format("(%s)", this.$injector.dynamicCallRegex.source), "g");
@@ -37,8 +35,6 @@ export class MicroTemplateService implements IMicroTemplateService {
 		localVariables["isMacOS"] = isHtml || this.isPlatform("darwin");
 		localVariables["isConsole"] = !isHtml;
 		localVariables["isHtml"] = isHtml;
-		localVariables["formatListOfNames"] = formatListOfNames;
-		localVariables["getUnitTestingFrameworkNames"] = this.$testInitializationService.getFrameworkNames;
 		localVariables["isJekyll"] = false;
 
 		return localVariables;
diff --git a/lib/controllers/preview-app-controller.ts b/lib/controllers/preview-app-controller.ts
@@ -141,7 +141,7 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon
 		const platformHmrData = _.cloneDeep(hmrData) || <IPlatformHmrData>{};
 		const connectedDevices = this.$previewDevicesService.getDevicesForPlatform(platform);
 		if (!connectedDevices || !connectedDevices.length) {
-			this.$logger.warn(`Unable to find any connected devices for platform '${platform}'. In order to execute live sync, open your Preview app and optionally re-scan the QR code using the Playground app.`);
+			this.$logger.warn(`Unable to find any connected devices for platform '${platform}'. In order to execute livesync, open your Preview app and optionally re-scan the QR code using the Playground app.`);
 			return;
 		}
 
diff --git a/lib/controllers/run-controller.ts b/lib/controllers/run-controller.ts
@@ -144,10 +144,10 @@ export class RunController extends EventEmitter implements IRunController {
 		return this.$liveSyncProcessDataService.getDeviceDescriptors(data.projectDir);
 	}
 
-	protected async refreshApplication(projectData: IProjectData, liveSyncResultInfo: ILiveSyncResultInfo, filesChangeEventData: IFilesChangeEventData, deviceDescriptor: ILiveSyncDeviceDescriptor): Promise<IRestartApplicationInfo> {
+	protected async refreshApplication(projectData: IProjectData, liveSyncResultInfo: ILiveSyncResultInfo, filesChangeEventData: IFilesChangeEventData, deviceDescriptor: ILiveSyncDeviceDescriptor, fullSyncAction?: () => Promise<void>): Promise<IRestartApplicationInfo> {
 		const result = deviceDescriptor.debuggingEnabled ?
 			await this.refreshApplicationWithDebug(projectData, liveSyncResultInfo, filesChangeEventData, deviceDescriptor) :
-			await this.refreshApplicationWithoutDebug(projectData, liveSyncResultInfo, filesChangeEventData, deviceDescriptor);
+			await this.refreshApplicationWithoutDebug(projectData, liveSyncResultInfo, filesChangeEventData, deviceDescriptor, undefined, fullSyncAction);
 
 		const device = liveSyncResultInfo.deviceAppData.device;
 
@@ -181,14 +181,15 @@ export class RunController extends EventEmitter implements IRunController {
 	}
 
 	@performanceLog()
-	protected async refreshApplicationWithoutDebug(projectData: IProjectData, liveSyncResultInfo: ILiveSyncResultInfo, filesChangeEventData: IFilesChangeEventData, deviceDescriptor: ILiveSyncDeviceDescriptor, settings?: IRefreshApplicationSettings): Promise<IRestartApplicationInfo> {
+	protected async refreshApplicationWithoutDebug(projectData: IProjectData, liveSyncResultInfo: ILiveSyncResultInfo, filesChangeEventData: IFilesChangeEventData, deviceDescriptor: ILiveSyncDeviceDescriptor, settings?: IRefreshApplicationSettings, fullSyncAction?: () => Promise<void>): Promise<IRestartApplicationInfo> {
 		const result = { didRestart: false };
 		const platform = liveSyncResultInfo.deviceAppData.platform;
 		const applicationIdentifier = projectData.projectIdentifiers[platform.toLowerCase()];
 		const platformLiveSyncService = this.$liveSyncServiceResolver.resolveLiveSyncService(platform);
 
 		try {
-			let shouldRestart = filesChangeEventData && (filesChangeEventData.hasNativeChanges || !filesChangeEventData.hasOnlyHotUpdateFiles);
+			const isFullSync = filesChangeEventData && (filesChangeEventData.hasNativeChanges || !filesChangeEventData.hasOnlyHotUpdateFiles);
+			let shouldRestart = isFullSync;
 			if (!shouldRestart) {
 				shouldRestart = await platformLiveSyncService.shouldRestart(projectData, liveSyncResultInfo);
 			}
@@ -197,6 +198,12 @@ export class RunController extends EventEmitter implements IRunController {
 				shouldRestart = !await platformLiveSyncService.tryRefreshApplication(projectData, liveSyncResultInfo);
 			}
 
+			if (!isFullSync && shouldRestart && fullSyncAction) {
+				this.$logger.trace(`Syncing all files as the current app state does not support hot updates.`);
+				liveSyncResultInfo.didRecover = true;
+				await fullSyncAction();
+			}
+
 			if (shouldRestart) {
 				this.emit(DEBUGGER_DETACHED_EVENT_NAME, { deviceIdentifier: liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier });
 				await platformLiveSyncService.restartApplication(projectData, liveSyncResultInfo);
@@ -360,11 +367,14 @@ export class RunController extends EventEmitter implements IRunController {
 
 			try {
 				const platformLiveSyncService = this.$liveSyncServiceResolver.resolveLiveSyncService(device.deviceInfo.platform);
+				const allAppFiles = (data.hmrData && data.hmrData.fallbackFiles && data.hmrData.fallbackFiles.length) ?
+					data.hmrData.fallbackFiles : data.files;
+				const filesToSync = data.hasOnlyHotUpdateFiles ? data.files : allAppFiles;
 				const watchInfo = {
 					liveSyncDeviceData: deviceDescriptor,
 					projectData,
 					filesToRemove: <any>[],
-					filesToSync: data.files,
+					filesToSync,
 					hmrData: data.hmrData,
 					useHotModuleReload: liveSyncInfo.useHotModuleReload,
 					force: liveSyncInfo.force,
@@ -391,16 +401,21 @@ export class RunController extends EventEmitter implements IRunController {
 					}
 
 					const watchAction = async (): Promise<void> => {
-						let liveSyncResultInfo = await platformLiveSyncService.liveSyncWatchAction(device, watchInfo);
-						await this.refreshApplication(projectData, liveSyncResultInfo, data, deviceDescriptor);
+						const liveSyncResultInfo = await platformLiveSyncService.liveSyncWatchAction(device, watchInfo);
+						const fullSyncAction = async () => {
+							watchInfo.filesToSync = allAppFiles;
+							const fullLiveSyncResultInfo = await platformLiveSyncService.liveSyncWatchAction(device, watchInfo);
+							// IMPORTANT: keep the same instance as we rely on side effects
+							_.assign(liveSyncResultInfo, fullLiveSyncResultInfo);
+						};
+
+						await this.refreshApplication(projectData, liveSyncResultInfo, data, deviceDescriptor, fullSyncAction);
 
 						if (!liveSyncResultInfo.didRecover && isInHMRMode) {
 							const status = await this.$hmrStatusService.getHmrStatus(device.deviceInfo.identifier, data.hmrData.hash);
-							// error or timeout
-							if (status !== HmrConstants.HMR_SUCCESS_STATUS) {
-								watchInfo.filesToSync = data.hmrData.fallbackFiles;
-								liveSyncResultInfo = await platformLiveSyncService.liveSyncWatchAction(device, watchInfo);
-								// We want to force a restart of the application.
+							// the timeout is assumed OK as the app could be blocked on a breakpoint
+							if (status === HmrConstants.HMR_ERROR_STATUS) {
+								await fullSyncAction();
 								liveSyncResultInfo.isFullSync = true;
 								await this.refreshApplication(projectData, liveSyncResultInfo, data, deviceDescriptor);
 							}
diff --git a/lib/options.ts b/lib/options.ts
@@ -44,6 +44,10 @@ export class Options {
 			// on each livesync in order to stop and allow debugging on app start
 			this.argv.hmr = false;
 		}
+
+		if (this.argv.justlaunch) {
+			this.argv.hmr = false;
+		}
 	}
 
 	constructor(private $errors: IErrors,
diff --git a/lib/services/livesync/android-livesync-service.ts b/lib/services/livesync/android-livesync-service.ts
@@ -25,21 +25,6 @@ export class AndroidLiveSyncService extends PlatformLiveSyncServiceBase implemen
 
 	@performanceLog()
 	public async liveSyncWatchAction(device: Mobile.IDevice, liveSyncInfo: ILiveSyncWatchInfo): Promise<IAndroidLiveSyncResultInfo> {
-		let result = await this.liveSyncWatchActionCore(device, liveSyncInfo);
-
-		// When we use hmr, there is only one case when result.didRefresh is false.
-		// This is the case when the app has crashed and is in ErrorActivity.
-		// As the app might not have time to apply the patches, we will send the whole bundle.js(fallbackFiles)
-		if (liveSyncInfo.useHotModuleReload && !result.didRefresh && liveSyncInfo.hmrData && liveSyncInfo.hmrData.hash) {
-			liveSyncInfo.filesToSync = liveSyncInfo.hmrData.fallbackFiles;
-			result = await this.liveSyncWatchActionCore(device, liveSyncInfo);
-			result.didRecover = true;
-		}
-
-		return result;
-	}
-
-	private async liveSyncWatchActionCore(device: Mobile.IDevice, liveSyncInfo: ILiveSyncWatchInfo): Promise<IAndroidLiveSyncResultInfo> {
 		const liveSyncResult = await super.liveSyncWatchAction(device, liveSyncInfo);
 		const result = await this.finalizeSync(device, liveSyncInfo.projectData, liveSyncResult);
 
diff --git a/lib/services/livesync/ios-device-livesync-service.ts b/lib/services/livesync/ios-device-livesync-service.ts
@@ -76,7 +76,7 @@ export class IOSDeviceLiveSyncService extends DeviceLiveSyncServiceBase implemen
 			shouldRestart = true;
 		} else {
 			const canExecuteFastSync = this.canExecuteFastSyncForPaths(liveSyncInfo, localToDevicePaths, projectData, deviceAppData.platform);
-			const isRefreshConnectionSetup = this.canRefreshWithNotification(projectData, liveSyncInfo) || (!this.device.isOnlyWiFiConnected && await this.setupSocketIfNeeded(projectData));
+			const isRefreshConnectionSetup = this.canRefreshWithNotification(projectData, liveSyncInfo) || (!this.device.isOnlyWiFiConnected && this.socket);
 			if (!canExecuteFastSync || !isRefreshConnectionSetup) {
 				shouldRestart = true;
 			}
@@ -137,6 +137,11 @@ export class IOSDeviceLiveSyncService extends DeviceLiveSyncServiceBase implemen
 			waitForDebugger: liveSyncInfo.waitForDebugger,
 			projectDir: projectData.projectDir
 		});
+
+		if (liveSyncInfo.useHotModuleReload) {
+			// enable HOT updates
+			await this.setupSocketIfNeeded(projectData);
+		}
 	}
 
 	private async reloadPage(): Promise<void> {
diff --git a/lib/services/webpack/webpack-compiler-service.ts b/lib/services/webpack/webpack-compiler-service.ts
@@ -272,8 +272,9 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp
 		this.expectedHashes[platform] = nextHash;
 
 		const emittedHotUpdatesAndAssets = isHashValid ? _.difference(allEmittedFiles, chunkFiles) : allEmittedFiles;
+		const fallbackFiles = chunkFiles.concat(emittedHotUpdatesAndAssets.filter(f => f.indexOf("hot-update") === -1));
 
-		return { emittedFiles: emittedHotUpdatesAndAssets, fallbackFiles: chunkFiles, hash: currentHash };
+		return { emittedFiles: emittedHotUpdatesAndAssets, fallbackFiles, hash: currentHash };
 	}
 
 	private getCurrentHotUpdateHash(emittedFiles: string[]) {
diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json
diff --git a/package.json b/package.json
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -138,7 +138,6 @@ module.exports = function (grunt) {`
`138`	`138`	`"isWindows": true,`
`139`	`139`	`"isMacOS": true,`
`140`	`140`	`"isLinux": true,`
`141`		`- "formatListOfNames": () => { },`
`142`	`141`	`"constants": ""`
`143`	`142`	`}`
`144`	`143`	`},`
Original file line number	Diff line number	Diff line change
`@@ -141,7 +141,7 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon`
`141`	`141`	`const platformHmrData = _.cloneDeep(hmrData) \|\| <IPlatformHmrData>{};`
`142`	`142`	`const connectedDevices = this.$previewDevicesService.getDevicesForPlatform(platform);`
`143`	`143`	`if (!connectedDevices \|\| !connectedDevices.length) {`
`144`		- this.$logger.warn(`Unable to find any connected devices for platform '${platform}'. In order to execute live sync, open your Preview app and optionally re-scan the QR code using the Playground app.`);
	`144`	+ this.$logger.warn(`Unable to find any connected devices for platform '${platform}'. In order to execute livesync, open your Preview app and optionally re-scan the QR code using the Playground app.`);
`145`	`145`	`return;`
`146`	`146`	`}`
`147`	`147`
Original file line number	Diff line number	Diff line change
`@@ -44,6 +44,10 @@ export class Options {`
`44`	`44`	`// on each livesync in order to stop and allow debugging on app start`
`45`	`45`	`this.argv.hmr = false;`
`46`	`46`	`}`
	`47`	`+`
	`48`	`+ if (this.argv.justlaunch) {`
	`49`	`+ this.argv.hmr = false;`
	`50`	`+ }`
`47`	`51`	`}`
`48`	`52`
`49`	`53`	`constructor(private $errors: IErrors,`
Original file line number	Diff line number	Diff line change
`@@ -272,8 +272,9 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp`
`272`	`272`	`this.expectedHashes[platform] = nextHash;`
`273`	`273`
`274`	`274`	`const emittedHotUpdatesAndAssets = isHashValid ? _.difference(allEmittedFiles, chunkFiles) : allEmittedFiles;`
	`275`	`+ const fallbackFiles = chunkFiles.concat(emittedHotUpdatesAndAssets.filter(f => f.indexOf("hot-update") === -1));`
`275`	`276`
`276`		`- return { emittedFiles: emittedHotUpdatesAndAssets, fallbackFiles: chunkFiles, hash: currentHash };`
	`277`	`+ return { emittedFiles: emittedHotUpdatesAndAssets, fallbackFiles, hash: currentHash };`
`277`	`278`	`}`
`278`	`279`
`279`	`280`	`private getCurrentHotUpdateHash(emittedFiles: string[]) {`