Skip to content
This repository was archived by the owner on Aug 7, 2021. It is now read-only.

feat: add hook for preview command #649

Merged
merged 4 commits into from
Sep 13, 2018
Merged

feat: add hook for preview command #649

merged 4 commits into from
Sep 13, 2018

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Sep 10, 2018

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

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 Fatme changed the title Preview sync hook feat: add hook for preview command Sep 10, 2018
@Fatme
Copy link
Contributor Author

Fatme commented Sep 11, 2018

run ci

* Add event handler for "liveSyncStopped" event
* Persist webpack process per platform

Implements: #457
@Fatme
Copy link
Contributor Author

Fatme commented Sep 12, 2018

run ci

lib/compiler.js Outdated
@@ -43,6 +43,10 @@ exports.runWebpackCompiler = function runWebpackCompiler(config, $projectData, $
env["sourceMap"] = true;
}

if (hookArgs && hookArgs.externals) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it's a good idea to add a comment here that the externals are passed when we use the tns preview command only?

@sis0k0 sis0k0 merged commit b47da3c into master Sep 13, 2018
@sis0k0 sis0k0 deleted the fatme/preview branch September 13, 2018 14:02
sis0k0 pushed a commit that referenced this pull request Sep 27, 2018
`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.
sis0k0 pushed a commit that referenced this pull request Sep 28, 2018
`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.
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants