Skip to content

fix: fix issues with Sidekick, cloud builds and multiple projects/devices #4806

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 5 commits into from
Jul 5, 2019

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Jul 5, 2019

Fix attaching/detaching from PREPARE_READY_EVENT

Currently we're attaching on PREPARE_READY_EVENT when run method is called. As Sidekick is long living process and run on device can be called more than once, we'll have more than one handlers for PREPARE_READY_EVENT. This means that we'll sync the changed files more than once. This PR fixes this behavior as ensures that we're attаching only once on PREPARE_READY_EVENT event and detaching from it on stop method.

Don't provide device descriptors initially on syncChangedDataOnDevices

Currently CLI provides deviceDescriptors only initially when syncChangedDataOnDevices method is called. As Sidekick can add additional deviceDescriptors, CLI doesn't respect them when files are changed and syncs only initially set deviceDescriptors.

Fix shouldBuild method for release cloud builds

Currently CLI throws an error for release cloud build as this.$projectChangesService.currentChanges is undefined and CLI is not able to read .hasChanges of undefined. this.$projectChangesService.currentChanges is populated when this.$projectChangesService.checkForChanges method is called. This method is called from nativePrepeare method which is not called for cloud builds as skipNativePrepare flag is true. Due to these reasons this.$projectChangesService.currentChanges is undefined. This PR fixes this behavior as ensures that this.$projectChangesService.checkForChanges will be called and this.$projectChangesService.currentChanges will be populated.

Fix tns run command when there is both iOS and android device and at least one platform is not added

When tns run command is executed and there is both iOS and android device and at least one platform is not added, CLI adds the missing platform and writes the version in package.json. As the native watcher watches the package.json file, it reports that there is a native change. After that CLI rebuilds again the project as the native change is reported. In order to avoid such behavior, this PR call checkForChanges method before syncing the changed files when there is a native change.

PR Checklist

What is the current behavior?

What is the new behavior?

Fixes/Implements/Closes #[Issue Number].

@cla-bot cla-bot bot added the cla: yes label Jul 5, 2019
@rosen-vladimirov
Copy link
Contributor

test cli-templates package_version#rc

@Fatme
Copy link
Contributor Author

Fatme commented Jul 5, 2019

test cli-templates package_version#rc

@Fatme
Copy link
Contributor Author

Fatme commented Jul 5, 2019

test cli-templates package_version#rc

@Fatme
Copy link
Contributor Author

Fatme commented Jul 5, 2019

test cli-device package_version#rc

1 similar comment
@rosen-vladimirov
Copy link
Contributor

test cli-device package_version#rc

@rosen-vladimirov rosen-vladimirov merged commit 5a74db5 into release Jul 5, 2019
@rosen-vladimirov rosen-vladimirov deleted the fatme/fix-prepare-ready-event branch July 5, 2019 15:08
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.

2 participants