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

feat(livesync): enable webpack with watch #433

Merged

Mitko-Kerezov merged 5 commits into master from kerezov/watch+livesync

Feb 22, 2018

Merged

feat(livesync): enable webpack with watch #433

Mitko-Kerezov merged 5 commits into master from kerezov/watch+livesync

Feb 22, 2018

Conversation

Mitko-Kerezov

Copy link

Contributor

@Mitko-Kerezov Mitko-Kerezov commented Feb 1, 2018

Plug into NativeScript CLI's LiveSync pipeline in order to enable incremental webpacking with watch.
Includes:

Instruct CLI to watch App_Resources directory only
Launch webpack with --watch on before-watch hook
Do not launch multiple webpack processes

Merge after NativeScript/nativescript-cli#3350
Implements NativeScript/nativescript-cli#3349

Ping @sis0k0 @rosen-vladimirov @KristianDD

@Mitko-Kerezov Mitko-Kerezov self-assigned this

Feb 1, 2018

@Mitko-Kerezov Mitko-Kerezov force-pushed the kerezov/watch+livesync branch 4 times, most recently from d30bd90 to a6b98b3 Compare

February 7, 2018 14:49

sis0k0

sis0k0 reviewed

Feb 8, 2018

View reviewed changes

lib/before-watchPatterns.js Outdated

if (originalPatterns.indexOf(appResourcesDirectoryLocation) === -1) {

originalPatterns.push(appResourcesDirectoryLocation);

}

Copy link

Contributor

Choose a reason for hiding this comment

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

I think it would be better to keep the appResourcesDirectory in the original watch patterns, instead of adding it manually in the bundler.

lib/before-prepareJS.js Outdated

platform,

bundle: appFilesUpdaterOptions.bundle,

watch: false// TODO: Read from CLI options...

watch: false

Copy link

Contributor

Choose a reason for hiding this comment

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

We can drop this line since watch is false by default.

lib/compiler.js Outdated

const { existsSync } = require("fs");

const readline = require("readline");

const { messages } = require("../plugins/WatchStateLoggerPlugin");

const ProjectSnapshotGenerator = require("../snapshot/android/project-snapshot-generator");

Copy link

Contributor

Choose a reason for hiding this comment

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

Unused import

lib/compiler.js Outdated

}

if (hookArgs.filesToSync && hookArgs.startSyncFilesTimeout) {

// TODO: Might need to get the "app" constant from config file in the future

Copy link

Contributor

Choose a reason for hiding this comment

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

Let's remove the 'TODO's and create a separate issue, tracking all the changes that may be needed in the plugin after NativeScript/android#944 is merged.

@Mitko-Kerezov Mitko-Kerezov mentioned this pull request

Feb 9, 2018

Get app directory location from CLI #436

Closed

@Mitko-Kerezov

Copy link

Contributor Author

Mitko-Kerezov commented Feb 9, 2018

Ping @sis0k0 with fixed comments and a follow up issue #436

@SvetoslavTsenov

Copy link

Contributor

SvetoslavTsenov commented Feb 13, 2018

test

@Mitko-Kerezov Mitko-Kerezov force-pushed the kerezov/watch+livesync branch from 06fe266 to 79e274f Compare

February 15, 2018 12:39

Dimitar Kerezov added 4 commits

February 15, 2018 14:51


 feat(livesync): enable webpack with watch

603eb9d

Plug into NativeScript CLI's LiveSync pipeline in order to enable incremental webpacking with watch.
Includes:
* Instruct CLI to watch `App_Resources` directory only
* Launch `webpack` with `--watch` on before-watch hook
* Do not launch multiple `webpack` processes


 refactor(livesync): extract app as constant

e56de67


 chore(livesync): make code synchronous

b726388


 fix: do not rebuild every time

4fc6deb

@Mitko-Kerezov Mitko-Kerezov force-pushed the kerezov/watch+livesync branch from 79e274f to 4fc6deb Compare

February 15, 2018 12:51

@Mitko-Kerezov

Copy link

Contributor Author

Mitko-Kerezov commented Feb 15, 2018

Ping @sis0k0, rebased and addressed some issues found off line

sis0k0

sis0k0 reviewed

Feb 16, 2018

View reviewed changes

Copy link

Contributor

@sis0k0 sis0k0 left a comment

Choose a reason for hiding this comment

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

Just an idea -
since we have access to all watch patterns from the CLI, can we use them as follows:

patterns that webpack should use for watching -> initialize the webpack compiler (and watcher) with them.
patterns that webpack should ignore -> add them to the 'ignore' options when starting the webpack compiler. That way we can remove the ignore option (./app/App_Resources) from the webpack config.

@dtopuzov

Copy link

Contributor

dtopuzov commented Feb 16, 2018

run ci


 fix: emit absolute paths

3919f61

sis0k0

sis0k0 approved these changes

Feb 19, 2018

View reviewed changes

Copy link

Contributor

@sis0k0 sis0k0 left a comment

Choose a reason for hiding this comment

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

Great work!

@SvetoslavTsenov

Copy link

Contributor

SvetoslavTsenov commented Feb 19, 2018

test branch_cli#kerezov/webpack+livesync

2 similar comments

@dtopuzov

Copy link

Contributor

dtopuzov commented Feb 19, 2018

test branch_cli#kerezov/webpack+livesync

@dtopuzov

Copy link

Contributor

dtopuzov commented Feb 19, 2018

test branch_cli#kerezov/webpack+livesync

@Mitko-Kerezov Mitko-Kerezov merged commit 847a56f into master

Feb 22, 2018

@Mitko-Kerezov Mitko-Kerezov deleted the kerezov/watch+livesync branch

February 22, 2018 15:19

@lambourn

Copy link

lambourn commented Feb 28, 2018

nice - do we need to pass --watch to the tns run --bundle call now or is watch active by default?

@Mitko-Kerezov

Copy link

Contributor Author

Mitko-Kerezov commented Feb 28, 2018

@lambourn watch is active by default - you only need execute
tns run <platform> --bundle.

As prerequisites for this to work, though, you would need the @next versions of both nativescript and nativescript-dev-webpack

@lambourn

Copy link

lambourn commented Feb 28, 2018

@Mitko-Kerezov thanmks. The nativescript-dev-webpack is at 0.9.2 at npmjs.org, is this identical to @next?

@lambourn

Copy link

lambourn commented Feb 28, 2018

ah, nevermind. Got it.

@Mitko-Kerezov

Copy link

Contributor Author

Mitko-Kerezov commented Feb 28, 2018

As a reference here:
On every commit in the master branch of this plugin a new version is published to npm.
You can use the latest of these versions by executing npm install nativescript-dev-webpack@next

@lambourn

Copy link

lambourn commented Feb 28, 2018

@Mitko-Kerezov yep, thanks. I Installed @next versions of nativescript and nativescript-dev-webpack and got it running successfully. Very nice.

However, my vendor.ts file gets recompiled every time, even though nothing changed on that file. It merely lists all 3rd party imports, e.g. angular etc.

Is that the expected behaviour?

@Mitko-Kerezov

Copy link

Contributor Author

Mitko-Kerezov commented Feb 28, 2018

@lambourn first off, thank you for testing this functionality - this feedback is very valuable to us.
As to your question - it is expected the vendor chunk be recompiled whenever you make changes to .css or .scss files or to app/vendor.ts itself. This is caused by the fact that css contents are embedded into the vendor chunk.

What kind of files do you modify to cause this behavior?

@darkyelox

Copy link

darkyelox commented Feb 28, 2018 •

edited

Loading

I can test this with a project already created using Nativescript CLI 3.4.2? it's an Angular project; i updated the webpack package and the Nativescript CLI to @next but when i try to run using tns run android --bundle i get the error:

Executing before-watchPatterns hook from /home/diego/Documents/development/mobile/bridge/NativeScript/Modules/shared-module/hooks/before-watchPatterns/nativescript-dev-webpack.js Cannot read property 'getAppDirectoryRelativePath' of undefined

@darkyelox

Copy link

darkyelox commented Feb 28, 2018

I test installing the latest nativescript cli @next and creating an empty project with latest nativescript-dev-webpack @next plugin installed and running gives me the same error, the app compiles and runs in the emulator but the cli crashes when trying to watch.

@lambourn

Copy link

lambourn commented Mar 1, 2018

@Mitko-Kerezov I'll recheck. Presumably, it was either just a component.html or a component.ts file, for sure it was not a .css. The component is part of the bundle.js

In this case, recompilation of the vendor.ts should not happen, right?

btw, is it ok to hijack this PR comment thread or should I create an issue for it?

@Mitko-Kerezov

Copy link

Contributor Author

Mitko-Kerezov commented Mar 1, 2018 •

edited

Loading

@darkyelox as I said earlier @next versions are published on every commit in the master branch - the issue you are describing is already resolved in the current @next of nativescript-dev-webpack. You can safely try updating the plugin and giving it another go.

@lambourn I tried changing component.ts as well as component.html and only my bundle chunk got recompiled. If you still experience have trouble could you create an issue in this repository for this and list the steps to reproduce as detailed as possible and we'll look into it. Providing a demo for reproduction purposes would be even better.

@Tronix117

Copy link

Tronix117 commented Apr 4, 2018 •

edited

Loading

Is this expected behavior than the app is entirely reloaded only when css change ?

I see data transfering from webpack (app.css.js, package.json, starter.js, tns-java-classes.js), then I see the CSS changes immediately in the app, and after that the app restart.

I'd like to prevent last step, any solution ?

Note: I added app.scss to the entry in webpack, if I don't do that, the css is not in the app.

@NickIliev NickIliev mentioned this pull request

Apr 25, 2018

Console logs not working #414

Closed

Labels

None yet

7 participants

@Mitko-Kerezov @SvetoslavTsenov @dtopuzov @lambourn @darkyelox @Tronix117 @sis0k0

Uh oh!

feat(livesync): enable webpack with watch #433

feat(livesync): enable webpack with watch #433

Uh oh!

Conversation

@Mitko-Kerezov Mitko-Kerezov commented Feb 1, 2018

Uh oh!

@sis0k0 sis0k0 Feb 8, 2018

Choose a reason for hiding this comment

Uh oh!

@sis0k0 sis0k0 Feb 8, 2018

Choose a reason for hiding this comment

Uh oh!

@sis0k0 sis0k0 Feb 8, 2018

Choose a reason for hiding this comment

Uh oh!

@sis0k0 sis0k0 Feb 8, 2018

Choose a reason for hiding this comment

Uh oh!

Mitko-Kerezov commented Feb 9, 2018

Uh oh!

SvetoslavTsenov commented Feb 13, 2018

Uh oh!

Mitko-Kerezov commented Feb 15, 2018

Uh oh!

@sis0k0 sis0k0 left a comment

Choose a reason for hiding this comment

Uh oh!

dtopuzov commented Feb 16, 2018

Uh oh!

@sis0k0 sis0k0 left a comment

Choose a reason for hiding this comment

Uh oh!

SvetoslavTsenov commented Feb 19, 2018

Uh oh!

dtopuzov commented Feb 19, 2018

Uh oh!

dtopuzov commented Feb 19, 2018

Uh oh!

lambourn commented Feb 28, 2018

Uh oh!

Mitko-Kerezov commented Feb 28, 2018

Uh oh!

lambourn commented Feb 28, 2018

Uh oh!

lambourn commented Feb 28, 2018

Uh oh!

Mitko-Kerezov commented Feb 28, 2018

Uh oh!

lambourn commented Feb 28, 2018

Uh oh!

Mitko-Kerezov commented Feb 28, 2018

Uh oh!

darkyelox commented Feb 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darkyelox commented Feb 28, 2018

Uh oh!

lambourn commented Mar 1, 2018

Uh oh!

Mitko-Kerezov commented Mar 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tronix117 commented Apr 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

darkyelox commented Feb 28, 2018 •

edited

Loading

Mitko-Kerezov commented Mar 1, 2018 •

edited

Loading

Tronix117 commented Apr 4, 2018 •

edited

Loading