From 62aecdd83167108d5b98ad110b46e3546169ecfb Mon Sep 17 00:00:00 2001 From: fatme Date: Thu, 18 Jul 2019 22:46:21 +0300 Subject: [PATCH 1/8] fix(cloud, sidekick): don't stop livesync on iOS device without developer disk image when app is built in cloud on windows The livesync process is stopped when a native change is made in app built in cloud on windows OS. This happens as CLI builds the app and after that tries to restart it. As the `platformLiveSyncService.restartApplication` is without try/catch, the CLI is not able to start the application due to missing developer disk image mounted on device. After that an error is thrown and the livesync process is stopped. This PR replaces `platformLiveSyncService.restartApplication` with `this.refreshApplication`. The `this.refreshApplication` method has try/catch and shows a warning `Unable to start application...` when an error is thrown during restart process. On the other side, this method has a check if a naive change occurs, so no additional checks will be executed from the method in order to determine if the application should be refreshed or restarted. (https://github.com/NativeScript/nativescript-cli/blob/24b21c4c3ec46ecf05f3b3aa1d2ed5b7d2301424/lib/controllers/run-controller.ts#L187) --- lib/controllers/run-controller.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/controllers/run-controller.ts b/lib/controllers/run-controller.ts index e4a0c637d2..fdb07e6557 100644 --- a/lib/controllers/run-controller.ts +++ b/lib/controllers/run-controller.ts @@ -199,7 +199,7 @@ export class RunController extends EventEmitter implements IRunController { result.didRestart = true; } } catch (err) { - this.$logger.info(`Error while trying to start application ${applicationIdentifier} on device ${liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier}. Error is: ${err.message || err}`); + this.$logger.trace(`Error while trying to start application ${applicationIdentifier} on device ${liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier}. Error is: ${err.message || err}`); const msg = `Unable to start application ${applicationIdentifier} on device ${liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier}. Try starting it manually.`; this.$logger.warn(msg); @@ -377,7 +377,7 @@ export class RunController extends EventEmitter implements IRunController { await this.$deviceInstallAppService.installOnDevice(device, deviceDescriptor.buildData, rebuiltInformation[platformData.platformNameLowerCase].packageFilePath); await platformLiveSyncService.syncAfterInstall(device, watchInfo); - await platformLiveSyncService.restartApplication(projectData, { deviceAppData, modifiedFilesData: [], isFullSync: false, useHotModuleReload: liveSyncInfo.useHotModuleReload }); + await this.refreshApplication(projectData, { deviceAppData, modifiedFilesData: [], isFullSync: false, useHotModuleReload: liveSyncInfo.useHotModuleReload }, data, deviceDescriptor); } else { const isInHMRMode = liveSyncInfo.useHotModuleReload && data.hmrData && data.hmrData.hash; if (isInHMRMode) { From b7a32dab5b5b0b09217306da3775f49a7c170769 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 19 Jul 2019 17:17:45 +0300 Subject: [PATCH 2/8] fix(run): remove prepareReadyEvent handler correctly on run `tns run` (and using SK) should remove its handler for prepareControllers' `prepareReadyEvent` handler. However, this does not happend due to two reasons: - we try to remove the handler from runControllers' event, but this handler is attached to `preparController`, so it actually remains and causes multiple issues in long living processes, when CLI is used as a library - the event handler is not cached correctly as we attach its bound version, but cache it without the binding. So you cannot remove it. This causes issues when running on device in Sidekick, closing the current app, open it again and try to livesync a change - we have many handlers for prepareReadyEvent and this causes multiple livesync operations. --- lib/controllers/run-controller.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/controllers/run-controller.ts b/lib/controllers/run-controller.ts index fdb07e6557..d6cc35912e 100644 --- a/lib/controllers/run-controller.ts +++ b/lib/controllers/run-controller.ts @@ -51,7 +51,7 @@ export class RunController extends EventEmitter implements IRunController { } if (!this.prepareReadyEventHandler) { - this.prepareReadyEventHandler = async (data: IFilesChangeEventData) => { + const handler = async (data: IFilesChangeEventData) => { if (data.hasNativeChanges) { const platformData = this.$platformsDataService.getPlatformData(data.platform, projectData); const prepareData = this.$prepareDataService.getPrepareData(liveSyncInfo.projectDir, data.platform, { ...liveSyncInfo, watch: !liveSyncInfo.skipWatcher }); @@ -63,7 +63,9 @@ export class RunController extends EventEmitter implements IRunController { await this.syncChangedDataOnDevices(data, projectData, liveSyncInfo); } }; - this.$prepareController.on(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler.bind(this)); + + this.prepareReadyEventHandler = handler.bind(this); + this.$prepareController.on(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler); } await this.syncInitialDataOnDevices(projectData, liveSyncInfo, deviceDescriptorsForInitialSync); @@ -113,7 +115,7 @@ export class RunController extends EventEmitter implements IRunController { liveSyncProcessInfo.deviceDescriptors = []; if (this.prepareReadyEventHandler) { - this.removeListener(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler); + this.$prepareController.removeListener(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler); this.prepareReadyEventHandler = null; } @@ -199,7 +201,7 @@ export class RunController extends EventEmitter implements IRunController { result.didRestart = true; } } catch (err) { - this.$logger.trace(`Error while trying to start application ${applicationIdentifier} on device ${liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier}. Error is: ${err.message || err}`); + this.$logger.info(`Error while trying to start application ${applicationIdentifier} on device ${liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier}. Error is: ${err.message || err}`); const msg = `Unable to start application ${applicationIdentifier} on device ${liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier}. Try starting it manually.`; this.$logger.warn(msg); From 713a8174a6d3604f1f436b0545fdc61a67858151 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 19 Jul 2019 17:23:51 +0300 Subject: [PATCH 3/8] fix(preview): remove prepareReadyEvent handler correctly on preview `tns preview` (and using SK) should remove its handler for prepareControllers' `prepareReadyEvent` handler. However, this does not happend due to two reasons: - we try to remove the handler from prepareControllers' event, but this handler is attached to `preparController`, so it actually remains and causes multiple issues in long living processes, when CLI is used as a library - the event handler is not cached correctly as we attach its bound version, but cache it without the binding. So you cannot remove it. This causes issues when using preview in Sidekick, closing the current app, open it again and try to livesync a change - we have many handlers for prepareReadyEvent and this causes multiple livesync operations. --- lib/controllers/preview-app-controller.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/controllers/preview-app-controller.ts b/lib/controllers/preview-app-controller.ts index 9634718d99..74f522467d 100644 --- a/lib/controllers/preview-app-controller.ts +++ b/lib/controllers/preview-app-controller.ts @@ -43,7 +43,7 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon this.$previewSdkService.stop(); this.$previewDevicesService.updateConnectedDevices([]); if (this.prepareReadyEventHandler) { - this.removeListener(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler); + this.$prepareController.removeListener(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler); this.prepareReadyEventHandler = null; } } @@ -83,10 +83,12 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon await this.$previewAppPluginsService.comparePluginsOnDevice(data, device); if (!this.prepareReadyEventHandler) { - this.prepareReadyEventHandler = async (currentPrepareData: IFilesChangeEventData) => { + const handler = async (currentPrepareData: IFilesChangeEventData) => { await this.handlePrepareReadyEvent(data, currentPrepareData); }; - this.$prepareController.on(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler.bind(this)); + + this.prepareReadyEventHandler = handler.bind(this); + this.$prepareController.on(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler); } data.env = data.env || {}; From b7595b5ccfcb388635b4e902d30b3de4514fe7b7 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 19 Jul 2019 17:26:12 +0300 Subject: [PATCH 4/8] fix: remove webpackCompilationComplete handler correctly on prepare Commands using prepare of project (and using SK) should remove their handlers for webpackCompilerService's `webpackCompilationComplete` event. This is not happening and whenever we start new prepartion of the project, we attach new handler. This causes issues when running on device in Sidekick, closing the current app, open it again and try to livesync a change - we have many handlers for prepareReadyEvent and this causes multiple livesync operations. --- lib/controllers/prepare-controller.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/controllers/prepare-controller.ts b/lib/controllers/prepare-controller.ts index 8361c686c0..9d6a85195d 100644 --- a/lib/controllers/prepare-controller.ts +++ b/lib/controllers/prepare-controller.ts @@ -14,6 +14,7 @@ export class PrepareController extends EventEmitter { private watchersData: IDictionary> = {}; private isInitialPrepareReady = false; private persistedData: IFilesChangeEventData[] = []; + private webpackCompilerHandler: any = null; constructor( private $platformController: IPlatformController, @@ -46,7 +47,8 @@ export class PrepareController extends EventEmitter { } if (this.watchersData && this.watchersData[projectDir] && this.watchersData[projectDir][platformLowerCase] && this.watchersData[projectDir][platformLowerCase].hasWebpackCompilerProcess) { - await this.$webpackCompilerService.stopWebpackCompiler(platform); + await this.$webpackCompilerService.stopWebpackCompiler(platformLowerCase); + this.$webpackCompilerService.removeListener(WEBPACK_COMPILATION_COMPLETE, this.webpackCompilerHandler); this.watchersData[projectDir][platformLowerCase].hasWebpackCompilerProcess = false; } } @@ -110,11 +112,14 @@ export class PrepareController extends EventEmitter { private async startJSWatcherWithPrepare(platformData: IPlatformData, projectData: IProjectData, prepareData: IPrepareData): Promise { if (!this.watchersData[projectData.projectDir][platformData.platformNameLowerCase].hasWebpackCompilerProcess) { - this.$webpackCompilerService.on(WEBPACK_COMPILATION_COMPLETE, data => { + const handler = (data: any) => { if (data.platform.toLowerCase() === platformData.platformNameLowerCase) { this.emitPrepareEvent({ ...data, hasNativeChanges: false }); } - }); + }; + + this.webpackCompilerHandler = handler.bind(this); + this.$webpackCompilerService.on(WEBPACK_COMPILATION_COMPLETE, this.webpackCompilerHandler); this.watchersData[projectData.projectDir][platformData.platformNameLowerCase].hasWebpackCompilerProcess = true; await this.$webpackCompilerService.compileWithWatch(platformData, projectData, prepareData); From f0e79b3cfed004debab806153e39b3553313bdc0 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 19 Jul 2019 17:28:11 +0300 Subject: [PATCH 5/8] fix: webpackCompilerService should not cache childProcesses forever Currently the webpackCompilerService caches the started webpack processes, but it removes them from the cache only when someone calls `stopWebpackCompiler` method. This is a major problem as we cache the first instance of the child process and we do not remove it, even when the process dies. For example, when you run just build, we start the webpack process without watch mode, the webpackCompilerService caches the child process, it exits, but the instance remains cached. Trying to run the application on device does not start new webpack process and different failures occur. --- lib/services/webpack/webpack-compiler-service.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/services/webpack/webpack-compiler-service.ts b/lib/services/webpack/webpack-compiler-service.ts index e0af115380..6a6fbcb94a 100644 --- a/lib/services/webpack/webpack-compiler-service.ts +++ b/lib/services/webpack/webpack-compiler-service.ts @@ -72,6 +72,7 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp childProcess.on("error", (err) => { this.$logger.trace(`Unable to start webpack process in watch mode. Error is: ${err}`); + delete this.webpackProcesses[platformData.platformNameLowerCase]; reject(err); }); @@ -82,6 +83,7 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp this.$logger.trace(`Webpack process exited with code ${exitCode} when we expected it to be long living with watch.`); const error = new Error(`Executing webpack failed with exit code ${exitCode}.`); error.code = exitCode; + delete this.webpackProcesses[platformData.platformNameLowerCase]; reject(error); }); }); @@ -97,12 +99,14 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp const childProcess = await this.startWebpackProcess(platformData, projectData, prepareData); childProcess.on("error", (err) => { this.$logger.trace(`Unable to start webpack process in non-watch mode. Error is: ${err}`); + delete this.webpackProcesses[platformData.platformNameLowerCase]; reject(err); }); childProcess.on("close", async (arg: any) => { await this.$cleanupService.removeKillProcess(childProcess.pid.toString()); + delete this.webpackProcesses[platformData.platformNameLowerCase]; const exitCode = typeof arg === "number" ? arg : arg && arg.code; if (exitCode === 0) { resolve(); From 7c83e3c52c6d463d28b5a219f6330d00d75bf919 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 19 Jul 2019 17:32:33 +0300 Subject: [PATCH 6/8] fix: stopPreview does not stop running webpack compilers `stopPreview` method does not stop running webpack compilers, so it is no longer possible to run the same application on device (no matter preview, build or run). This happens in long living processes, like Sidekick. To fix this, add new parameter to the method - an object containing the projectDir and use it to stop the processes. --- lib/controllers/preview-app-controller.ts | 7 ++++++- lib/definitions/preview-app-livesync.d.ts | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/controllers/preview-app-controller.ts b/lib/controllers/preview-app-controller.ts index 74f522467d..b53ca164e3 100644 --- a/lib/controllers/preview-app-controller.ts +++ b/lib/controllers/preview-app-controller.ts @@ -15,6 +15,7 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon constructor( private $analyticsService: IAnalyticsService, + private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, private $errors: IErrors, private $hmrStatusService: IHmrStatusService, private $logger: ILogger, @@ -39,9 +40,13 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon return result; } - public async stopPreview(): Promise { + public async stopPreview(data: IProjectDir): Promise { this.$previewSdkService.stop(); this.$previewDevicesService.updateConnectedDevices([]); + + await this.$prepareController.stopWatchers(data.projectDir, this.$devicePlatformsConstants.Android); + await this.$prepareController.stopWatchers(data.projectDir, this.$devicePlatformsConstants.iOS); + if (this.prepareReadyEventHandler) { this.$prepareController.removeListener(PREPARE_READY_EVENT_NAME, this.prepareReadyEventHandler); this.prepareReadyEventHandler = null; diff --git a/lib/definitions/preview-app-livesync.d.ts b/lib/definitions/preview-app-livesync.d.ts index 414b4cffe7..6c8f125aed 100644 --- a/lib/definitions/preview-app-livesync.d.ts +++ b/lib/definitions/preview-app-livesync.d.ts @@ -77,6 +77,6 @@ declare global { interface IPreviewAppController { startPreview(data: IPreviewAppLiveSyncData): Promise; - stopPreview(): Promise; + stopPreview(data: IProjectDir): Promise; } } \ No newline at end of file From a55591dbaab9e0db55a902de227d7ce66805e2aa Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 19 Jul 2019 17:51:03 +0300 Subject: [PATCH 7/8] chore: fix run-controllers unit tests --- test/controllers/run-controller.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/controllers/run-controller.ts b/test/controllers/run-controller.ts index e33fbc4688..6b0c200278 100644 --- a/test/controllers/run-controller.ts +++ b/test/controllers/run-controller.ts @@ -96,7 +96,8 @@ function createTestInjector() { prepareData = currentPrepareData; return { platform: prepareData.platform, hasNativeChanges: false }; }, - on: () => ({}) + on: () => ({}), + removeListener: (): void => undefined }); injector.register("prepareNativePlatformService", {}); injector.register("projectChangesService", {}); From 99676a4ee792ecfdfb7aaf2236d215de44bb9637 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 19 Jul 2019 17:51:45 +0300 Subject: [PATCH 8/8] chore: set version to 6.0.2 --- npm-shrinkwrap.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index fa4d54cd5c..2aeebbccc8 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "nativescript", - "version": "6.0.1", + "version": "6.0.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 63c06551b4..468664b837 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "nativescript", "preferGlobal": true, - "version": "6.0.1", + "version": "6.0.2", "author": "Telerik ", "description": "Command-line interface for building NativeScript projects", "bin": {