Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't prepare the project on preview command #4142

Merged
merged 4 commits into from
Nov 23, 2018

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Nov 19, 2018

PR Checklist

What is the current behavior?

Project is prepared when tns preview command is executed

What is the new behavior?

Project is not prepared when tns preview command is executed

Fixes/Implements/Closes #[Issue Number].
#4141

Prepare is needed only to compile typescript and sass files. `preview-sync` hook is added to `nativescript-dev-typescript` and `nativescript-dev-sass` plugins in order to handle the compilation. This way the preview command will be faster as no prepare will be executed.
@Fatme
Copy link
Contributor Author

Fatme commented Nov 19, 2018

run ci

@@ -8,6 +8,16 @@ declare global {
stopLiveSync(): Promise<void>;
}

interface IPreviewAppFilesService {
getInitialFilesPayload(data: IPreviewAppLiveSyncData, platform: string, deviceId?: string): FilesPayload;
getFilesPayload(data: IPreviewAppLiveSyncData, filesData: IPreviewAppFilesData, platform: string, deviceId?: string): FilesPayload;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data -> liveSyncData

.filter(file => !_.includes(this.excludedFiles, path.basename(file)))
.filter(file => !_.includes(this.excludedFileExtensions, path.extname(file)));

this.$logger.trace(`Transferring ${filesToTransfer.join("\n")}.`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transferring is not the proper verb for this method

@@ -78,7 +66,7 @@ export class PreviewAppLiveSyncService implements IPreviewAppLiveSyncService {
const hookArgs = this.getHookArgs(data, device);
await this.$hooksService.executeBeforeHooks("preview-sync", { hookArgs });
await this.$previewAppPluginsService.comparePluginsOnDevice(data, device);
const payloads = await this.syncFilesForPlatformSafe(data, device.platform, { isInitialSync: true, useHotModuleReload: data.useHotModuleReload });
const payloads = await this.syncInitialFilesForPlatformSafe(data, device.platform);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syncInitialFilesForPlatformSafe -> getInitialFilesForPlatformSafe

and

initializePreviewForDevice -> getInitialFilesForDevice


try {
const payloads = this.$previewAppFilesService.getInitialFilesPayload(data, platform);
this.$logger.info(`Successfully synced changes for platform ${platform}.`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully synced changes for platform... -> Successfully sent initial files for platform...

@@ -106,14 +94,45 @@ export class PreviewAppLiveSyncService implements IPreviewAppLiveSyncService {
return result;
}

private async syncInitialFilesForPlatformSafe(data: IPreviewAppLiveSyncData, platform: string): Promise<FilesPayload> {
this.$logger.info(`Start syncing changes for platform ${platform}.`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start syncing changes for platform... -> Start sending initial files for platform...

this.$logger.info(`Successfully synced changes for platform ${platform}.`);
return payloads;
} catch (err) {
this.$logger.warn(`Unable to apply changes for platform ${platform}. Error is: ${err}, ${JSON.stringify(err, null, 2)}.`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${JSON.stringify(err, null, 2)} -> helper

@Fatme Fatme added this to the 5.0.2 milestone Nov 20, 2018
@Fatme Fatme self-assigned this Nov 20, 2018
@Fatme Fatme modified the milestones: 5.0.2, 5.1.0 Nov 21, 2018
@DimitarTachev DimitarTachev changed the base branch from release to master November 21, 2018 13:55
@Fatme
Copy link
Contributor Author

Fatme commented Nov 22, 2018

run ci

@DimitarTachev DimitarTachev merged commit e2b55b2 into master Nov 23, 2018
@DimitarTachev DimitarTachev deleted the fatme/remove-prepare-from-preview branch November 23, 2018 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants