Playground #3853

Fatme · 2018-08-30T11:37:36Z

PR Checklist

The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages.
There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
You have signed the CLA.
All existing tests are passing: https://github.com/NativeScript/nativescript-cli/blob/master/CONTRIBUTING.md#contribute-to-the-code-base
Tests for the changes are included.

Implements: #3813
Rel: NativeScript/nativescript-dev-webpack#649

DimitarTachev

Nice tests!

DimitarTachev · 2018-08-31T12:36:12Z

lib/services/livesync/playground/preview-app-livesync-service.ts

+					event: PreviewSdkEventNames.CHANGE_EVENT_NAME,
+					file: path.relative(platformsAppFolderPath, file),
+					fileContents: this.$fs.readText(file),
+					binary: false


maybe we should pass true for the images here

DimitarTachev · 2018-08-31T12:44:41Z

lib/services/livesync/playground/preview-sdk-service.ts

+				this.$logger.info(`LOG from device ${deviceName}: ${log}`);
+			},
+			onRestartMessage: () => {
+				console.log("ON RESTART MESSAGE!!!");


DimitarTachev · 2018-08-31T12:45:08Z

lib/services/livesync/playground/preview-sdk-service.ts

+	private getCallbacks(): SdkCallbacks {
+		return {
+			onLogSdkMessage: (log: string) => {
+				this.$logger.trace("onLogSdkMessage!!!", log);


set a better message here

DimitarTachev · 2018-08-31T12:46:22Z

lib/services/livesync/playground/preview-sdk-service.ts

+				console.log("ON RESTART MESSAGE!!!");
+			},
+			onUncaughtErrorMessage: () => {
+				this.$errors.failWithoutHelp("UncaughtErrorMessage while preview app!!");


we could also think about a better message here

DimitarTachev · 2018-08-31T12:59:04Z

lib/services/qr-code-terminal-service.ts

+		try {
+			qrcode.generate(url);
+		} catch (err) {
+			this.$logger.trace(`Failed to generate QR code for ${url}`, err);


maybe this should be info?

Fatme · 2018-09-04T05:18:38Z

run ci

rosen-vladimirov · 2018-09-03T14:21:32Z

lib/commands/command-base.ts

@@ -0,0 +1,34 @@
+export abstract class CommandBase implements ICommandBase {


You can rename this to ValidatePlatformCommandBase and do not implement ICommandBase

rosen-vladimirov · 2018-09-04T06:17:40Z

lib/definitions/livesync.d.ts

 	 */
 	timeout: string;
+
+	syncToPreviewApp?: boolean;


JSDocs here

rosen-vladimirov · 2018-09-04T06:18:59Z

lib/definitions/livesync.d.ts

 	 * @returns {string} The build output directory.
 	 */
 	getOutputDirectory?(options: IOutputDirectoryOptions): string;
+	syncToPreviewApp?: boolean;


You can extract an interface:

interface IHasSyncToPreviewAppOption { syncToPreviewApp?: boolean; }

And extend both ILiveSyncCommandHelperAdditionalOptions and ILiveSyncInfo interfaces

rosen-vladimirov · 2018-09-05T10:36:26Z

lib/commands/preview.ts

+	}
+
+	public async canExecute(args: string[]): Promise<boolean> {
+		return true;


this will allow the execution of the command even if you write tns preview someargs. In case the command does not accept args, just remove the canExecute method

rosen-vladimirov · 2018-09-05T10:56:56Z

lib/services/livesync/playground/preview-app-livesync-service.ts

+
+				if (filePayload.binary) {
+					const bitmap =  <string>this.$fs.readFile(file);
+					const base64 = new Buffer(bitmap).toString('base64');


new Buffer is deprecated, we should use Buffer.from or maybe this.$fs.readFile(file, { encoding: "base64" })

rosen-vladimirov · 2018-09-05T10:58:29Z

lib/services/livesync/playground/preview-app-livesync-service.ts

+
+	private showWarningsForNativeFiles(files: string[]): void {
+		if (files && files.length) {
+			for (const file of files) {


Instead of using if and for, you can use _.each - it handles cases where even undefined is passed

Or you can do:

_.filter(files, file => file.indexOf(APP_RESOURCES_FOLDER_NAME) > -1) .forEach(file => this.$logger.warn(`Unable to apply changes from ${APP_RESOURCES_FOLDER_NAME} folder. You need to build your application in order to make changes in ${APP_RESOURCES_FOLDER_NAME} folder.`));

`tns preview` command works without platform argument, so {N} CLI needs to start iOS webpack process when QR code is scanned with iOS device and android webpack process when QR code is scanned with Android device. Actually we need to have two separate webpack processes simultaneously. In order to do that we need to persist webpack process per platform. When `tns preview --bundle` command is executed and QR code is scanned for example with Android device, {N} CLI starts a webpack process in watch mode for Android platform. If the webpack process for Android is already started, {N} CLI doesn't do anything. In order to archive the above things, a new hook is introduced - "before-preview-sync". This hook is used only from `tns preview` command. ## PR Checklist - [x] The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages. - [ ] There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it. - [ ] You have signed the [CLA]. - [ ] All existing tests are passing: https://github.com/NativeScript/nativescript-dev-webpack/blob/master/CONTRIBUTING.md#testing-locally-by-running-e2e-tests - [ ] Tests for the changes are included. ## What is the current behavior? When `tns run --bundle` command is executed, the following error message is shown: "Bundling doesn't work with multiple platforms. Please specify platform to the run command." ## What is the new behavior? When `tns run --bundle` command is executed and for example the user has connected both iOS and Android devices, two webpack processes are started (one for Android and one for iOS). When the user makes some change in code, the changes are applied both on iOS and Android devices. NOTE: When a platform specific file is changed - for example `myfile.ios.js` only iOS webpack is started and the changes are applied only on iOS devices. When for example some iOS device is disconnected, we check if there are more connected iOS devices. If no more iOS devices are connected, we stopped the webpack process for iOS platform. We did it by adding an event handler for "liveSyncStopped" event. Implements: #457 Rel: NativeScript/nativescript-cli#3853 BREAKING CHANGES: The breaking changes are only for commands for which no platform argument is provided.

…onfigured

…r sync to preview app

…y configured and "Sync to Playground" option is selected

…int)

…ins-service

…ard are pressed.

…IValidatePlatformOutput

… and "Sync to Playground" option is selected

Fix "cannot read property .toLowerCase of undefined" error

…endencies and show warnings

…ipt-dev-webpack plugin

miroslavaivanova · 2018-09-14T08:59:07Z

run ci

`tns preview` command works without platform argument, so {N} CLI needs to start iOS webpack process when QR code is scanned with iOS device and android webpack process when QR code is scanned with Android device. Actually we need to have two separate webpack processes simultaneously. In order to do that we need to persist webpack process per platform. When `tns preview --bundle` command is executed and QR code is scanned for example with Android device, {N} CLI starts a webpack process in watch mode for Android platform. If the webpack process for Android is already started, {N} CLI doesn't do anything. In order to archive the above things, a new hook is introduced - "before-preview-sync". This hook is used only from `tns preview` command. ## PR Checklist - [x] The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages. - [ ] There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it. - [ ] You have signed the [CLA]. - [ ] All existing tests are passing: https://github.com/NativeScript/nativescript-dev-webpack/blob/master/CONTRIBUTING.md#testing-locally-by-running-e2e-tests - [ ] Tests for the changes are included. ## What is the current behavior? When `tns run --bundle` command is executed, the following error message is shown: "Bundling doesn't work with multiple platforms. Please specify platform to the run command." ## What is the new behavior? When `tns run --bundle` command is executed and for example the user has connected both iOS and Android devices, two webpack processes are started (one for Android and one for iOS). When the user makes some change in code, the changes are applied both on iOS and Android devices. NOTE: When a platform specific file is changed - for example `myfile.ios.js` only iOS webpack is started and the changes are applied only on iOS devices. When for example some iOS device is disconnected, we check if there are more connected iOS devices. If no more iOS devices are connected, we stopped the webpack process for iOS platform. We did it by adding an event handler for "liveSyncStopped" event. Implements: #457 Rel: NativeScript/nativescript-cli#3853 BREAKING CHANGES: The breaking changes are only for commands for which no platform argument is provided.

Fatme changed the title ~~Feature/playground~~ Playground Aug 30, 2018

Fatme mentioned this pull request Aug 30, 2018

Playground telerik/mobile-cli-lib#1134

Merged

DimitarTachev suggested changes Aug 31, 2018

View reviewed changes

Fatme added this to the 5.0.0 milestone Sep 3, 2018

DimitarTachev force-pushed the feature/playground branch from e88c5e4 to 349c445 Compare September 3, 2018 15:35

rosen-vladimirov suggested changes Sep 4, 2018

View reviewed changes

rosen-vladimirov reviewed Sep 5, 2018

View reviewed changes

Fatme mentioned this pull request Sep 10, 2018

feat: add hook for preview command NativeScript/nativescript-dev-webpack#649

Merged

5 tasks

Fatme force-pushed the feature/playground branch 2 times, most recently from 1ecdfdc to 0310ef0 Compare September 12, 2018 09:49

DimitarTachev force-pushed the feature/playground branch from 41ba075 to 0310ef0 Compare September 13, 2018 07:27

rosen-vladimirov approved these changes Sep 14, 2018

View reviewed changes

Fatme force-pushed the feature/playground branch from a61d739 to f847c8b Compare September 14, 2018 08:10

Fatme and others added 13 commits September 14, 2018 11:14

Initial version of preview command

6d2aaf9

Generate qr code

94f9acc

Add "Sync to Playground" option in prompts when env is not properly c…

f278202

…onfigured

Skip copy of app_resources and tns_modules when preparing platform fo…

5986b60

…r sync to preview app

Add warnings for plugins not presented in preview app on device

e7edf2d

Fix "Cannot find connected devices" error when the env is not properl…

c91958d

…y configured and "Sync to Playground" option is selected

Store only unique devices

ad822cd

Fix shouldStartWatcher check and fix lint errors

d46fec2

fix: handle bigger files upload (temporary reuse the playground endpo…

affbbea

…int)

Add warnings for native files and add unit tests for preview-app-plug…

04e741b

…ins-service

Show qr codes for current app and playground app when keys from keybo…

aac3668

…ard are pressed.

Add unit tests for preview-app-livesync-service

ec4d21c

Fix checkEnvironmentRequirements tests and rename IValidateOutput to …

efb4e00

…IValidatePlatformOutput

Fatme and others added 23 commits September 14, 2018 11:14

Stop execution of current command when env is not properly configured…

4967b2c

… and "Sync to Playground" option is selected

fix: fix binary files sync to the preview app

3ad200a

chore: regenerate shrinkwrap versions with the latest npm

d4f3d45

fix: fix the shortener endpoint usage

be9aa07

chore: use the preview sdk from npm

13643d2

chore: update the shrinkwrap after referring the preview SDK from npm

8a0e6c5

Fix messages

e9ed5d9

Fix "cannot read property .toLowerCase of undefined" error

chore: fix the shorURL casing from the shortener endpoint

7563753

fix: fix tslint errors

226060d

Generate QR code from command not inside preview-app-livesync-service.js

57cb647

Support for webpack

748f753

Fix tests and extract some messages as constants

c30fbef

Fix lint errors

d99972b

Fix PR comments

39622f6

Add base help for preview command

8349ae2

Integrate version 0.2.0 of nativescript-preview-sdk

37daa75

fix: compare the major versions between the local and preview app dep…

baa1295

…endencies and show warnings

fix: stop sending platform specific extensions in preview

d7bd815

Update preview sdk version

660f9fc

fix tests after removing the hack for platform specific files

b05ac25

Update nativescript-preview-sdk to 0.2.2 version

c2ca17f

chore: fix prepare

6dacef8

fix: Implement backwards compatibility with old versions of nativescr…

b7d42d7

…ipt-dev-webpack plugin

Fatme force-pushed the feature/playground branch from f847c8b to b7d42d7 Compare September 14, 2018 08:14

DimitarTachev approved these changes Sep 14, 2018

View reviewed changes

Fatme merged commit 3a783e1 into master Sep 14, 2018

Fatme deleted the feature/playground branch September 14, 2018 12:16

		@@ -0,0 +1,34 @@
		export abstract class CommandBase implements ICommandBase {

Uh oh!

Playground #3853

Playground #3853

Uh oh!

Conversation

Fatme commented Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

DimitarTachev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fatme commented Sep 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miroslavaivanova commented Sep 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fatme commented Aug 30, 2018 •

edited

Loading