From 55981a8b11ab3983ba8867969e12c8f8ae3b85d3 Mon Sep 17 00:00:00 2001 From: fatme Date: Tue, 20 Mar 2018 08:44:53 +0200 Subject: [PATCH 1/3] Fix node-sass watcher Currently node-sass watcher is used to compile sass files when some changes occurs. The problem is when file A is imported in file B. If some change occurs in file A, file A will be successfully compiled by `node-sass` but file B will not be updated. So when `livesync command` is executed, changes are not reflected in the app's UI. This PR removes `--watch` option from `node-sass` compiler and introduces chokidar watcher. Also fixes this https://github.com/NativeScript/nativescript-dev-sass/issues/70 --- src/lib/after-watch.js | 8 +- src/lib/converter.js | 212 +++++++++++++++++++++++++---------------- src/package.json | 1 + 3 files changed, 133 insertions(+), 88 deletions(-) diff --git a/src/lib/after-watch.js b/src/lib/after-watch.js index 7a3674d..e61f970 100644 --- a/src/lib/after-watch.js +++ b/src/lib/after-watch.js @@ -1,9 +1,9 @@ var converter = require('./converter'); module.exports = function ($logger) { - var sass = converter.getSassProcess(); - if (sass) { - $logger.info("Stopping sass watch"); - sass.kill("SIGINT") + var watcher = converter.getWatcher(); + if (watcher) { + $logger.info("Stopping nativescript-dev-sass watcher"); + watcher.close(); } } diff --git a/src/lib/converter.js b/src/lib/converter.js index de56f7e..cf38572 100644 --- a/src/lib/converter.js +++ b/src/lib/converter.js @@ -1,94 +1,138 @@ exports.convert = convert; -exports.getSassProcess = getSassProcess; +exports.getWatcher = getWatcher; var spawn = require('child_process').spawn; var fs = require('fs'); var path = require('path'); -var currentSassProcess = null; +var choki = require('chokidar'); +var watcher = null; function convert(logger, projectDir, appDir, options) { - return new Promise(function (resolve, reject) { - options = options || {}; - var sassPath = require.resolve('node-sass/bin/node-sass'); - var importerPath = path.join(__dirname, "importer.js"); - - if (fs.existsSync(sassPath)) { - try { - logger.info('Found peer node-sass'); - } catch (err) { } - } else { - throw Error('node-sass installation local to project was not found. Install by executing `npm install node-sass`.'); - } - - // Node SASS Command Line Args (https://github.com/sass/node-sass#command-line-interface) - // --ouput : Output directory - // --output-style : CSS output style (nested | expanded | compact | compresed) - // -q : Supress log output except on error - // --follow : Follow symlinked directories - // -r : Recursively watch directories or files - // --watch : Watch a directory or file - var nodeArgs = [sassPath, appDir, '--output', appDir, '--output-style', 'compressed', '-q', '--follow', '--importer', importerPath]; - if (options.watch) { - nodeArgs.push('-r', '--watch'); - } - - logger.trace(process.execPath, nodeArgs.join(' ')); - var env = Object.create( process.env ); - env.PROJECT_DIR = projectDir; - env.APP_DIR = appDir; - currentSassProcess = spawn(process.execPath, nodeArgs, { env: env }); - - var isResolved = false; - var watchResolveTimeout; - currentSassProcess.stdout.on('data', function (data) { - var stringData = data.toString(); - logger.info(stringData); - }); - - currentSassProcess.stderr.on('data', function (err) { - var message = ''; - var stringData = err.toString(); - - try { - var parsed = JSON.parse(stringData); - message = parsed.formatted || parsed.message || stringData; - } catch (e) { - renderMsg = true; - message = err.toString(); - } - - logger.info(message); - }); - - currentSassProcess.on('error', function (err) { - logger.info(err.message); - if (!isResolved) { - isResolved = true; - reject(err); - } - }); - - // TODO: Consider using close event instead of exit - currentSassProcess.on('exit', function (code, signal) { - currentSassProcess = null; - if (!isResolved) { - isResolved = true; - if (code === 0) { - resolve(); - } else { - reject(Error('SASS compiler failed with exit code ' + code)); - } - } - }); - - // SASS does not recompile on watch, so directly resolve. - if (options.watch && !isResolved) { - isResolved = true; - resolve(); - } - }); + options = options || {}; + var sassPath = getSassPath(); + var data = { + sassPath: sassPath, + projectDir: projectDir, + appDir: appDir, + logger: logger, + options: options + }; + + if (options.watch) { + createWatcher(data); + } + + return spawnNodeSass(data); +} + +function getWatcher() { + return watcher; +} + +function createWatcher(data) { + var appDir = data.appDir; + var watcherOptions = { + ignoreInitial: true, + cwd: appDir, + awaitWriteFinish: { + pollInterval: 100, + stabilityThreshold: 500 + }, + ignored: ['**/.*', '.*'] // hidden files + }; + + watcher = choki.watch(['**/*.scss', '**/*.sass'], watcherOptions) + .on('all', (event, filePath) => { + spawnNodeSass(data); + }); } -function getSassProcess() { - return currentSassProcess; +function getSassPath() { + var sassPath = require.resolve('node-sass/bin/node-sass'); + if (fs.existsSync(sassPath)) { + try { + logger.info('Found peer node-sass'); + } catch (err) { } + } else { + throw Error('node-sass installation local to project was not found. Install by executing `npm install node-sass`.'); + } + + return sassPath; +} + +function spawnNodeSass(data) { + return new Promise(function (resolve, reject) { + var sassPath = data.sassPath, + projectDir = data.projectDir, + appDir = data.appDir, + logger = data.logger, + options = data.options; + + var importerPath = path.join(__dirname, "importer.js"); + + // Node SASS Command Line Args (https://github.com/sass/node-sass#command-line-interface) + // --ouput : Output directory + // --output-style : CSS output style (nested | expanded | compact | compresed) + // -q : Supress log output except on error + // --follow : Follow symlinked directories + // -r : Recursively watch directories or files + // --watch : Watch a directory or file + var nodeArgs = [sassPath, appDir, '--output', appDir, '--output-style', 'compressed', '-q', '--follow', '--importer', importerPath]; + logger.trace(process.execPath, nodeArgs.join(' ')); + + var env = Object.create( process.env ); + env.PROJECT_DIR = projectDir; + env.APP_DIR = appDir; + + var currentSassProcess = spawn(process.execPath, nodeArgs, { env: env }); + + var isResolved = false; + var watchResolveTimeout; + + currentSassProcess.stdout.on('data', function (data) { + var stringData = data.toString(); + logger.info(stringData); + }); + + currentSassProcess.stderr.on('data', function (err) { + var message = ''; + var stringData = err.toString(); + + try { + var parsed = JSON.parse(stringData); + message = parsed.formatted || parsed.message || stringData; + } catch (e) { + message = err.toString(); + } + + logger.info(message); + }); + + currentSassProcess.on('error', function (err) { + logger.info(err.message); + if (!isResolved) { + isResolved = true; + reject(err); + } + }); + + // TODO: Consider using close event instead of exit + currentSassProcess.on('exit', function (code, signal) { + currentSassProcess = null; + if (!isResolved) { + isResolved = true; + if (code === 0) { + resolve(); + } else { + reject(Error('SASS compiler failed with exit code ' + code)); + } + } + }); + + // SASS does not recompile on watch, so directly resolve. + if (options.watch && !isResolved) { + isResolved = true; + resolve(); + } + }); } diff --git a/src/package.json b/src/package.json index e6a8d2a..1a81f4b 100644 --- a/src/package.json +++ b/src/package.json @@ -63,6 +63,7 @@ }, "dependencies": { "bluebird": "^3.4.6", + "chokidar": "2.0.2", "node-sass": "^4.7.1", "glob": "^7.1.2", "nativescript-hook": "^0.2.0" From 57ae2d9cb83391b0c5128b8236bd3a4fccaf903c Mon Sep 17 00:00:00 2001 From: fatme Date: Mon, 26 Mar 2018 13:09:03 +0300 Subject: [PATCH 2/3] Fix PR comments --- src/lib/converter.js | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/lib/converter.js b/src/lib/converter.js index cf38572..ca81e1a 100644 --- a/src/lib/converter.js +++ b/src/lib/converter.js @@ -6,16 +6,17 @@ var fs = require('fs'); var path = require('path'); var choki = require('chokidar'); var watcher = null; +var watchPromisesChain = Promise.resolve(); function convert(logger, projectDir, appDir, options) { options = options || {}; var sassPath = getSassPath(); var data = { - sassPath: sassPath, - projectDir: projectDir, - appDir: appDir, - logger: logger, - options: options + sassPath, + projectDir, + appDir, + logger, + options }; if (options.watch) { @@ -36,14 +37,14 @@ function createWatcher(data) { cwd: appDir, awaitWriteFinish: { pollInterval: 100, - stabilityThreshold: 500 + stabilityThreshold: 300 }, ignored: ['**/.*', '.*'] // hidden files }; watcher = choki.watch(['**/*.scss', '**/*.sass'], watcherOptions) .on('all', (event, filePath) => { - spawnNodeSass(data); + watchPromisesChain = watchPromisesChain.then(() => spawnNodeSass(data)); }); } @@ -54,7 +55,7 @@ function getSassPath() { logger.info('Found peer node-sass'); } catch (err) { } } else { - throw Error('node-sass installation local to project was not found. Install by executing `npm install node-sass`.'); + throw new Error('node-sass installation local to project was not found. Install by executing `npm install node-sass`.'); } return sassPath; @@ -87,7 +88,6 @@ function spawnNodeSass(data) { var currentSassProcess = spawn(process.execPath, nodeArgs, { env: env }); var isResolved = false; - var watchResolveTimeout; currentSassProcess.stdout.on('data', function (data) { var stringData = data.toString(); @@ -124,15 +124,9 @@ function spawnNodeSass(data) { if (code === 0) { resolve(); } else { - reject(Error('SASS compiler failed with exit code ' + code)); + reject(new Error('SASS compiler failed with exit code ' + code)); } } }); - - // SASS does not recompile on watch, so directly resolve. - if (options.watch && !isResolved) { - isResolved = true; - resolve(); - } }); } From e31808204ba287fa556425fbea0ada0745e2e758 Mon Sep 17 00:00:00 2001 From: fatme Date: Mon, 2 Apr 2018 09:30:40 +0300 Subject: [PATCH 3/3] Remove unneeded try/catch --- src/lib/converter.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/lib/converter.js b/src/lib/converter.js index ca81e1a..94bc290 100644 --- a/src/lib/converter.js +++ b/src/lib/converter.js @@ -10,7 +10,7 @@ var watchPromisesChain = Promise.resolve(); function convert(logger, projectDir, appDir, options) { options = options || {}; - var sassPath = getSassPath(); + var sassPath = getSassPath(logger); var data = { sassPath, projectDir, @@ -48,12 +48,10 @@ function createWatcher(data) { }); } -function getSassPath() { +function getSassPath(logger) { var sassPath = require.resolve('node-sass/bin/node-sass'); if (fs.existsSync(sassPath)) { - try { - logger.info('Found peer node-sass'); - } catch (err) { } + logger.info('Found peer node-sass'); } else { throw new Error('node-sass installation local to project was not found. Install by executing `npm install node-sass`.'); }