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

feat(livesync): enable webpack with watch #433

Merged
merged 5 commits into from
Feb 22, 2018

Conversation

Mitko-Kerezov
Copy link
Contributor

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

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.

@@ -8,7 +8,7 @@ module.exports = function ($mobileHelper, $projectData, hookArgs) {
env,
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
@@ -4,6 +4,7 @@ const { join, resolve: pathResolve } = require("path");
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
Copy link
Contributor Author

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

@SvetoslavTsenov
Copy link
Contributor

test

Dimitar Kerezov added 4 commits February 15, 2018 14:51
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
@Mitko-Kerezov
Copy link
Contributor Author

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

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

run ci

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

test branch_cli#kerezov/webpack+livesync

2 similar comments
@dtopuzov
Copy link
Contributor

test branch_cli#kerezov/webpack+livesync

@dtopuzov
Copy link
Contributor

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

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

@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

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

@lambourn
Copy link

ah, nevermind. Got it.

@Mitko-Kerezov
Copy link
Contributor Author

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

@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

@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

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

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

@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

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants