Skip to content

Fix node-sass watcher #71

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 3 commits into from
Apr 2, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/lib/after-watch.js
Original file line number Diff line number Diff line change
@@ -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();
}
}
212 changes: 128 additions & 84 deletions src/lib/converter.js
Original file line number Diff line number Diff line change
@@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use short syntax for all properties:

var data = {
    sassPath,
    projectDir,
...
}

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
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't it way too long, maybe 300 ?

},
ignored: ['**/.*', '.*'] // hidden files
};

watcher = choki.watch(['**/*.scss', '**/*.sass'], watcherOptions)
.on('all', (event, filePath) => {
spawnNodeSass(data);
Copy link
Contributor

Choose a reason for hiding this comment

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

In case you apply several changes and save them one-by-one, you'll receive multiple events and several processes will be spawned. This is a problem, so I suggest chaining the calls to spawnNodeSass:

var watchPromisesChain = Promise.resolve();
watcher = choki.watch(['**/*.scss', '**/*.sass'], watcherOptions)
	.on('all', (event, filePath) => {
		watchPromiseChain = watchPromiseChain.then( () => 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) { }
Copy link
Contributor

Choose a reason for hiding this comment

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

When will the logger.info fail ?

} else {
throw Error('node-sass installation local to project was not found. Install by executing `npm install node-sass`.');
Copy link
Contributor

Choose a reason for hiding this comment

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

throw new Error

}

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

this variable is not used


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));
Copy link
Contributor

Choose a reason for hiding this comment

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

new Error

}
}
});

// SASS does not recompile on watch, so directly resolve.
if (options.watch && !isResolved) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need the check for options.watch here

Copy link
Contributor

Choose a reason for hiding this comment

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

In fact the whole if and the call to resolve is not needed here.

isResolved = true;
resolve();
}
});
}
1 change: 1 addition & 0 deletions src/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down