From 7f708a44b9e70198a99b9b4b1c5275f035d7962e Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 3 Jul 2019 18:03:18 +0300 Subject: [PATCH 1/2] fix: preview and run do not work correctly In case there are multiple devices with the same platform, `run command` fails with error `Cannot read property hasNativeChanges of undefined`. The problem is that we return incorrect prepare data. Also we have a problem in `tns preview` when multiple platforms are used - all files are send multiple times to each device as we have too many started webpack processes (in case you scan fast with devices) and we are also adding handler of the prepare ready event per each device instead per platform. Fix this by handling the event only once per platform --- lib/controllers/prepare-controller.ts | 42 +++++++++++------------ lib/controllers/preview-app-controller.ts | 6 ++-- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/lib/controllers/prepare-controller.ts b/lib/controllers/prepare-controller.ts index 29b9be2b2a..65806d30a7 100644 --- a/lib/controllers/prepare-controller.ts +++ b/lib/controllers/prepare-controller.ts @@ -1,4 +1,3 @@ -import * as child_process from "child_process"; import * as choki from "chokidar"; import { hook } from "../common/helpers"; import { performanceLog } from "../common/decorators"; @@ -7,7 +6,7 @@ import * as path from "path"; import { PREPARE_READY_EVENT_NAME, WEBPACK_COMPILATION_COMPLETE, PACKAGE_JSON_FILE_NAME, PLATFORMS_DIR_NAME } from "../constants"; interface IPlatformWatcherData { - webpackCompilerProcess: child_process.ChildProcess; + hasWebpackCompilerProcess: boolean; nativeFilesWatcher: choki.FSWatcher; } @@ -63,9 +62,9 @@ export class PrepareController extends EventEmitter { this.watchersData[projectDir][platformLowerCase].nativeFilesWatcher = null; } - if (this.watchersData && this.watchersData[projectDir] && this.watchersData[projectDir][platformLowerCase] && this.watchersData[projectDir][platformLowerCase].webpackCompilerProcess) { + if (this.watchersData && this.watchersData[projectDir] && this.watchersData[projectDir][platformLowerCase] && this.watchersData[projectDir][platformLowerCase].hasWebpackCompilerProcess) { await this.$webpackCompilerService.stopWebpackCompiler(platform); - this.watchersData[projectDir][platformLowerCase].webpackCompilerProcess = null; + this.watchersData[projectDir][platformLowerCase].hasWebpackCompilerProcess = false; } } @@ -78,38 +77,39 @@ export class PrepareController extends EventEmitter { if (!this.watchersData[projectData.projectDir][platformData.platformNameLowerCase]) { this.watchersData[projectData.projectDir][platformData.platformNameLowerCase] = { nativeFilesWatcher: null, - webpackCompilerProcess: null + hasWebpackCompilerProcess: false }; - await this.startJSWatcherWithPrepare(platformData, projectData, prepareData); // -> start watcher + initial compilation - const hasNativeChanges = await this.startNativeWatcherWithPrepare(platformData, projectData, prepareData); // -> start watcher + initial prepare - const result = { platform: platformData.platformNameLowerCase, hasNativeChanges }; + } - const hasPersistedDataWithNativeChanges = this.persistedData.find(data => data.platform === result.platform && data.hasNativeChanges); - if (hasPersistedDataWithNativeChanges) { - result.hasNativeChanges = true; - } + await this.startJSWatcherWithPrepare(platformData, projectData, prepareData); // -> start watcher + initial compilation + const hasNativeChanges = await this.startNativeWatcherWithPrepare(platformData, projectData, prepareData); // -> start watcher + initial prepare + const result = { platform: platformData.platformNameLowerCase, hasNativeChanges }; - // TODO: Do not persist this in `this` context. Also it should be per platform. - this.isInitialPrepareReady = true; + const hasPersistedDataWithNativeChanges = this.persistedData.find(data => data.platform === result.platform && data.hasNativeChanges); + if (hasPersistedDataWithNativeChanges) { + result.hasNativeChanges = true; + } - if (this.persistedData && this.persistedData.length) { - this.emitPrepareEvent({ files: [], hasOnlyHotUpdateFiles: false, hasNativeChanges: result.hasNativeChanges, hmrData: null, platform: platformData.platformNameLowerCase }); - } + // TODO: Do not persist this in `this` context. Also it should be per platform. + this.isInitialPrepareReady = true; - return result; + if (this.persistedData && this.persistedData.length) { + this.emitPrepareEvent({ files: [], hasOnlyHotUpdateFiles: false, hasNativeChanges: result.hasNativeChanges, hmrData: null, platform: platformData.platformNameLowerCase }); } + + return result; } private async startJSWatcherWithPrepare(platformData: IPlatformData, projectData: IProjectData, prepareData: IPrepareData): Promise { - if (!this.watchersData[projectData.projectDir][platformData.platformNameLowerCase].webpackCompilerProcess) { + if (!this.watchersData[projectData.projectDir][platformData.platformNameLowerCase].hasWebpackCompilerProcess) { this.$webpackCompilerService.on(WEBPACK_COMPILATION_COMPLETE, data => { if (data.platform.toLowerCase() === platformData.platformNameLowerCase) { this.emitPrepareEvent({ ...data, hasNativeChanges: false }); } }); - const childProcess = await this.$webpackCompilerService.compileWithWatch(platformData, projectData, prepareData); - this.watchersData[projectData.projectDir][platformData.platformNameLowerCase].webpackCompilerProcess = childProcess; + this.watchersData[projectData.projectDir][platformData.platformNameLowerCase].hasWebpackCompilerProcess = true; + await this.$webpackCompilerService.compileWithWatch(platformData, projectData, prepareData); } } diff --git a/lib/controllers/preview-app-controller.ts b/lib/controllers/preview-app-controller.ts index c1883a76bf..cdd90f39de 100644 --- a/lib/controllers/preview-app-controller.ts +++ b/lib/controllers/preview-app-controller.ts @@ -79,8 +79,10 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon this.platformPrepareHandlers[device.platform] = true; // TODO: Remove the handler once the preview operation for this platform is stopped - this.$prepareController.on(PREPARE_READY_EVENT_NAME, async currentPrepareData => { - await this.handlePrepareReadyEvent(data, currentPrepareData.hmrData, currentPrepareData.files, device.platform); + this.$prepareController.on(PREPARE_READY_EVENT_NAME, async (currentPrepareData: IFilesChangeEventData) => { + if (currentPrepareData.platform.toLowerCase() === device.platform.toLowerCase()) { + await this.handlePrepareReadyEvent(data, currentPrepareData.hmrData, currentPrepareData.files, device.platform); + } }); } From 33c51925e6f2a08fa5dcaa77543bd8c61e1dc22e Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 3 Jul 2019 18:08:26 +0300 Subject: [PATCH 2/2] fix: remove incorrect spam message the message is shown every time when we want to show a line from device logs and we are with trace level --- lib/services/ios-log-filter.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/services/ios-log-filter.ts b/lib/services/ios-log-filter.ts index 4201ea6165..dfc0a7578c 100644 --- a/lib/services/ios-log-filter.ts +++ b/lib/services/ios-log-filter.ts @@ -13,13 +13,11 @@ export class IOSLogFilter implements Mobile.IPlatformLogFilter { private partialLine: string = null; - constructor(private $logger: ILogger, - private $loggingLevels: Mobile.ILoggingLevels) { + constructor(private $loggingLevels: Mobile.ILoggingLevels) { } public filterData(data: string, loggingOptions: Mobile.IDeviceLogOptions = {}): string { const specifiedLogLevel = (loggingOptions.logLevel || '').toUpperCase(); - this.$logger.trace("Logging options", loggingOptions); if (specifiedLogLevel !== this.$loggingLevels.info || !data) { return data;