Skip to content

fix: preview shouldn't start native watch #5016

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

Merged
merged 2 commits into from
Sep 12, 2019

Conversation

KristianDD
Copy link
Contributor

PR Checklist

What is the current behavior?

When the user scans the preview barcode with both device platforms, the second one modifies the package.json to set the runtime version, which triggers a native change event. This causes an error as preview controller doesn't understand and shouldn't understand of native changes.

What is the new behavior?

Disable the native changes watch when running tns preview.

Fixes/Implements/Closes #4893

@cla-bot cla-bot bot added the cla: yes label Sep 11, 2019
@KristianDD
Copy link
Contributor Author

test cli-smoke cli-preview cli-device

@KristianDD KristianDD force-pushed the kddimitrov/fix-preview-watches-native-changes branch from 02e7ca1 to f763264 Compare September 11, 2019 14:58
@KristianDD
Copy link
Contributor Author

test cli-smoke cli-preview cli-device

if (_.isBoolean(data.watchNative)) {
this.watchNative = data.watchNative;
}
this.watch = data.watch;
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicated line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. :)

@@ -127,6 +127,21 @@ export class PrepareController extends EventEmitter {
}

private async startNativeWatcherWithPrepare(platformData: IPlatformData, projectData: IProjectData, prepareData: IPrepareData): Promise<boolean> {
let newWatchStarted = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

newWatchStarted -> newNativeWatchStarted

@KristianDD
Copy link
Contributor Author

test cli-smoke cli-preview cli-device

@KristianDD KristianDD merged commit 1daa7aa into release Sep 12, 2019
@KristianDD KristianDD deleted the kddimitrov/fix-preview-watches-native-changes branch September 12, 2019 16:09
@KristianDD KristianDD added this to the 6.1.1 milestone Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants