From ffa866da9dd80cea418af21718ab2b55a7c7369f Mon Sep 17 00:00:00 2001 From: Juuso Backman Date: Sat, 15 Nov 2014 00:57:54 +0200 Subject: [PATCH 1/4] clean up preprocess.js, add errback --- lib/modules/preprocess.js | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/lib/modules/preprocess.js b/lib/modules/preprocess.js index de55f93a..4af999ad 100644 --- a/lib/modules/preprocess.js +++ b/lib/modules/preprocess.js @@ -5,44 +5,24 @@ var concat = require('gulp-concat'), filter = require('gulp-filter'), less = require('gulp-less'), sass = require('gulp-sass'), - vfs = require('vinyl-fs'), - _ = require('lodash'), - proxyOnce = function(obj, key, fn) { - var original = obj[key]; - obj[key] = function proxy() { - var args = Array.prototype.slice.call(arguments); - if (_.isFunction(fn)) { - fn.apply(undefined, args); - } - if (_.isFunction(original)) { - original.apply(undefined, args); - } - obj[key] = original; - } - }; + vfs = require('vinyl-fs'); module.exports = { - // Processes all SASS/LESS/CSS files and combine to a single file - getStream: function(files, opt) { - var sassStream, - lessStream, - cssStream; - proxyOnce(opt.sass, 'onError', opt.onCompileError); - proxyOnce(opt.sass, 'onSuccess', opt.onCompileSuccess); + /** + * Process all SASS/LESS/CSS files and combine them into a single stream + */ + getStream: function(files, opt, errback) { + var sassStream, lessStream, cssStream; sassStream = vfs.src(opt.sass.src || files) .pipe(filter('**/*.scss')) - .pipe(sass(opt.sass)) + .pipe(sass(opt.sass).on('error', errback)) .pipe(concat('sass.css')); lessStream = vfs.src(opt.less.src || files) .pipe(filter('**/*.less')) - .pipe(less(opt.less)) - .on('error', function(err) { - console.error('An error occurred when compiling LESS'); - console.error(err.toString()); - }) + .pipe(less(opt.less).on('error', errback)) .pipe(concat('less.css')); cssStream = vfs.src(files) From d6efbb071a0e0b143676b4326b9d5183b9cb7e13 Mon Sep 17 00:00:00 2001 From: Juuso Backman Date: Sat, 15 Nov 2014 01:00:52 +0200 Subject: [PATCH 2/4] Refactor: promisify styleguide generation steps and move sub-functions outside the main function where possible --- lib/styleguide.js | 231 ++++++++++++++++++++++++---------------------- 1 file changed, 122 insertions(+), 109 deletions(-) diff --git a/lib/styleguide.js b/lib/styleguide.js index f70ffb3e..ee93393c 100644 --- a/lib/styleguide.js +++ b/lib/styleguide.js @@ -11,11 +11,8 @@ 'use strict'; var through = require('through2'), fs = require('fs'), - ncp = require('ncp'), gulp = require('gulp'), - chalk = require('chalk'), rename = require('gulp-rename'), - mkdirp = require('mkdirp'), markdown = require(__dirname + '/modules/markdown'), mustache = require('gulp-mustache'), parser = require(__dirname + '/modules/parser'), @@ -26,162 +23,178 @@ var through = require('through2'), path = require('path'), sgServer = require(__dirname + '/server'), File = require('vinyl'), + q = require('q'), distPath = __dirname + '/dist', - callbackCount, serverInstance; -module.exports = function(opt) { - if (!opt) opt = {}; - - var filesConfig = opt.filesConfig, - filesBuffer = {}, - compileErrors = 0, - onCompileError = function(err) { - compileErrors += 1; - }, - checkIfDone; - - opt = { +function sanitizeOptions(opt) { + return { title: opt.title || 'Styleguide Generator', sass: opt.sass || {}, less: opt.less || {}, + kssOpt: opt.kssOpt || {}, overviewPath: opt.overviewPath || __dirname + '/overview.md', - extraHead: opt.extraHead || '', + extraHead: (typeof opt.extraHead === 'object') ? opt.extraHead.join('\n') : '', appRoot: opt.appRoot || '', commonClass: opt.commonClass || '', styleVariables: opt.styleVariables || '', server: opt.server || false, port: opt.port, - rootPath: opt.rootPath + rootPath: opt.rootPath, + filesConfig: opt.filesConfig }; +} - // If we have already server running emit info that we started progress +function emitProgressStartIfServerIsRunning() { if (serverInstance && serverInstance.io) { serverInstance.io.emitProgressStart(); } +} - if (typeof opt.extraHead === 'object') opt.extraHead = opt.extraHead.join('\n'); - if (!opt.kssOpt) opt.kssOpt = {}; +function generateInheritedWrappers(json) { + json.sections = wrapperMarkup.generateSectionWrapperMarkup(json.sections); +} - // A stream through which each file will pass - // We have 4 different streams that we have to wait until the whole process is completed - callbackCount = 4; - - checkIfDone = function(callback) { - if (--callbackCount <= 0) { - // We are all done. Check if we want to start server - if (opt.server) { - startServer(opt); +function saveOptionsAsJsonConfig(opt, json) { + json.config = opt; +} + +function removeConfigOutputPath(json) { + if (json.config) { + delete json.config.outputPath; + } +} + +function appendUsedVariablesToEachBlock(opt, styleguide) { + if (opt.styleVariables) { + var syntax = path.extname(opt.styleVariables).substring(1); + // Parse variables from the defined file + styleguide.config.settings = parser.parseVariables(fs.readFileSync(opt.styleVariables, 'utf-8'), syntax); + // Go trough every styleguide style block and find used variables + styleguide.sections.forEach(function(section) { + if (section.css) { + section.variables = parser.findVariables(section.css, syntax); } - callback(); - } - }; + return section; + }); + } +} - return through( - { +module.exports = function(options) { + var opt = sanitizeOptions(options), + filesBuffer = {}, + compileErrors = 0, + throughOpts = { objectMode: true, allowHalfOpen: false - }, - // Buffer all filenames - function(file, enc, cb) { - if (file.isNull()) return; - if (file.isStream()) { - return console.error('Styleguide does not support streams!'); - } + }; + + function bufferFileContents(file, enc, done) { + if (file.isNull()) { + return; + } + if (file.isStream()) { + return console.error('Styleguide does not support streams!'); + } + + filesBuffer[file.path] = file.contents.toString('utf8'); - filesBuffer[file.path] = file.contents.toString('utf8'); + // Make sure file goes through the next gulp plugin + this.push(file); + done(); + } - // Make sure file goes through the next gulp plugin - this.push(file); + emitProgressStartIfServerIsRunning(); - // We're done with this file! - cb(); - }, - // After reading all files, parse them and generate data for styleguide - function(callback) { + // A stream through which each file will pass + return through(throughOpts, bufferFileContents, function(callback) { var _this = this; parseKSS(filesBuffer, opt.kssOpt, function(styleguide) { - var pushAllFiles = function() { + function pushAllFiles() { return through.obj(function(file, enc, cb) { _this.push(file); cb(); - }).on('finish', function() { - checkIfDone(callback); - }) - }; - - // Make inherited wrappers - styleguide.sections = wrapperMarkup.generateSectionWrapperMarkup(styleguide.sections); - - // Store settings inside the styleguide JSON - styleguide.config = opt; - - // Do not add output path to configuration - delete styleguide.config.outputPath - - // If settings file is found, generate settings object - if (opt.styleVariables) { - var syntax = path.extname(opt.styleVariables).substring(1), - variables; - // Parse variables from the defined file - styleguide.config.settings = parser.parseVariables(fs.readFileSync(opt.styleVariables, 'utf-8'), syntax); - // Go trough every styleguide style block and find used variables - styleguide.sections.forEach(function(section) { - if (section.css) { - section.variables = parser.findVariables(section.css, syntax); + }); + } + + function processOverviewMarkdown(opt) { + return q.Promise(function(resolve) { + if (!opt.overviewPath) { + resolve(); } - return section; + markdown.getStream(opt.overviewPath) + .pipe(rename(function(path) { + path.basename = 'overview'; + path.extname = '.html'; + })) + .pipe(pushAllFiles()) + .on('finish', resolve); }); } + generateInheritedWrappers(styleguide); + saveOptionsAsJsonConfig(opt, styleguide); + removeConfigOutputPath(styleguide); + appendUsedVariablesToEachBlock(opt, styleguide); + // Create JSON containing KSS data _this.push(new File({ path: 'styleguide.json', contents: new Buffer(JSON.stringify(styleguide)) })); - // Process markdown file - if (opt.overviewPath) { - markdown.getStream(opt.overviewPath) - .pipe(rename(function(path) { - path.basename = 'overview'; - path.extname = '.html'; - })) - .pipe(pushAllFiles()) - } + var overviewProcessed, + preProcessingDone, + filesCopied, + indexHtmlProcessed; + + overviewProcessed = processOverviewMarkdown(opt); // Preprocess all CSS files and combile then to a single file - opt.onCompileError = onCompileError; - preprocess.getStream(Object.keys(filesBuffer), opt) - .pipe(through.obj(function(file, enc, cb) { + preProcessingDone = q.Promise(function(resolve, reject) { + preprocess.getStream(Object.keys(filesBuffer), opt, reject) + .pipe(through.obj(function(file, enc, cb) { - // Create stylesheet that contains pseudo styles - _this.push(new File({ - path: 'styleguide_pseudo_styles.css', - contents: new Buffer(pseudoSelectors.stylesFromString(file.contents.toString())) - })); + // Create stylesheet that contains pseudo styles + _this.push(new File({ + path: 'styleguide_pseudo_styles.css', + contents: new Buffer(pseudoSelectors.stylesFromString(file.contents.toString())) + })); - _this.push(file); - cb(); - }).on('finish', function() { - checkIfDone(callback); - })); + _this.push(file); + cb(); + }).on('finish', resolve)); + }); // Copy all files (except index.html) from dist from to output stream - gulp.src([distPath + '/**', '!' + distPath + '/index.html']) - .pipe(pushAllFiles()); + filesCopied = q.Promise(function(resolve, reject) { + gulp.src([distPath + '/**', '!' + distPath + '/index.html']) + .pipe(pushAllFiles()) + .on('finish', resolve); + }); // Process index.html - gulp.src([distPath + '/index.html']) - .pipe(mustache({ - title: opt.title, - extraHead: opt.extraHead, - appRoot: opt.appRoot, - socketIo: opt.server, - filesConfig: JSON.stringify(filesConfig) - })) - .pipe(pushAllFiles()); + indexHtmlProcessed = q.Promise(function(resolve, reject) { + gulp.src([distPath + '/index.html']) + .pipe(mustache({ + title: opt.title, + extraHead: opt.extraHead, + appRoot: opt.appRoot, + socketIo: opt.server, + filesConfig: JSON.stringify(opt.filesConfig) + })) + .pipe(pushAllFiles()) + .on('finish', resolve); + }); + + q.all([overviewProcessed, preProcessingDone, filesCopied, indexHtmlProcessed]) + .then(function() { + if (opt.server) { + startServer(opt); + } + callback(); + }); }); } ).on('end', function() { From 2c23c78af9d1ee920c6fa11d531e017c7dab3e90 Mon Sep 17 00:00:00 2001 From: Juuso Backman Date: Sun, 16 Nov 2014 20:49:22 +0200 Subject: [PATCH 3/4] Don't fail gulp sass task; notify the UI if there are sass or less errors from preprocessing --- gulpfile.js | 3 ++- lib/styleguide.js | 45 ++++++++++++++++++++++++++++----------------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index b190ff70..c8d2206f 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -102,7 +102,8 @@ gulp.task('sass', function() { .pipe(plumber()) .pipe(sass({ // Include bourbon & neat - includePaths: neat.includePaths + includePaths: neat.includePaths, + errLogToConsole: true })) .pipe(sourcemaps.init()) .pipe(please({ diff --git a/lib/styleguide.js b/lib/styleguide.js index ee93393c..3863307c 100644 --- a/lib/styleguide.js +++ b/lib/styleguide.js @@ -45,12 +45,28 @@ function sanitizeOptions(opt) { }; } -function emitProgressStartIfServerIsRunning() { - if (serverInstance && serverInstance.io) { +function socketIsOpen() { + return serverInstance && serverInstance.io; +} + +function emitProgressStart() { + if (socketIsOpen()) { serverInstance.io.emitProgressStart(); } } +function emitCompileError(error) { + if (socketIsOpen()) { + serverInstance.io.emitCompileError(error); + } +} + +function emitCompileSuccess() { + if (socketIsOpen()) { + serverInstance.io.emitCompileSuccess(); + } +} + function generateInheritedWrappers(json) { json.sections = wrapperMarkup.generateSectionWrapperMarkup(json.sections); } @@ -104,7 +120,7 @@ module.exports = function(options) { done(); } - emitProgressStartIfServerIsRunning(); + emitProgressStart(); // A stream through which each file will pass return through(throughOpts, bufferFileContents, function(callback) { @@ -193,21 +209,16 @@ module.exports = function(options) { if (opt.server) { startServer(opt); } - callback(); - }); + emitCompileSuccess(); + }) + .catch(function(error) { + console.error('Style guide parsing failed:', error.message); + emitCompileError(error); + }) + .finally(callback); }); } - ).on('end', function() { - if (serverInstance && serverInstance.io) { - serverInstance.io.emitProgressEnd(); - - if (compileErrors > 0) { - serverInstance.io.emitCompileError(compileErrors); - } else { - serverInstance.io.emitCompileSuccess(); - } - } - }); + ); }; /* Built-in server */ @@ -226,4 +237,4 @@ function startServer(options) { }); } return serverInstance; -}; +} From 5e52dcd8af738498fd8c51501883e6036f11a9b9 Mon Sep 17 00:00:00 2001 From: Juuso Backman Date: Sun, 16 Nov 2014 20:50:45 +0200 Subject: [PATCH 4/4] Expose only socket compile error and success emits; progress end is emitted in both cases --- gulpfile.js | 2 +- lib/modules/io.js | 28 +++++++++++----------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index c8d2206f..e44f4a03 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -66,7 +66,7 @@ gulp.task('styleguide', function() { if (!fs.existsSync(__dirname + distPath)) { process.stderr.write(chalk.red.bold('Error:') + ' Directory ' + distPath + ' does not exist. You probably installed library by cloning repository directly instead of NPM repository.\n'); process.stderr.write('You need to run ' + chalk.green.bold('gulp build') + ' first\n'); - process.exit(1) + process.exit(1); return 1; } return gulp.src([sourcePath + '/**/*.scss']) diff --git a/lib/modules/io.js b/lib/modules/io.js index 28dbfc44..f1655198 100644 --- a/lib/modules/io.js +++ b/lib/modules/io.js @@ -6,12 +6,6 @@ module.exports = function(ioServer, options) { var loadVariables, saveVariables, - reloadStyles, - currentSocket, - emitProgressStart, - emitProgressEnd, - emitCompileError, - emitCompileSuccess, io = ioServer; if (options.styleVariables && !fs.existsSync(options.styleVariables)) { @@ -21,9 +15,8 @@ module.exports = function(ioServer, options) { loadVariables = function(socket) { return function() { - var syntax = path.extname(options.styleVariables).substring(1); + var values, syntax = path.extname(options.styleVariables).substring(1); fs.readFile(options.styleVariables, {encoding: 'utf8'}, function(err, data) { - var values = {}; if (err) { return console.error(err); } @@ -50,21 +43,23 @@ module.exports = function(ioServer, options) { }; }; - emitProgressStart = function() { + function emitProgressStart() { io.sockets.emit('styleguide progress start'); - }; + } - emitProgressEnd = function() { + function emitProgressEnd() { io.sockets.emit('styleguide progress end'); - }; + } - emitCompileError = function(err) { + function emitCompileError(err) { io.sockets.emit('styleguide compile error', err); - }; + emitProgressEnd(); + } - emitCompileSuccess = function() { + function emitCompileSuccess() { io.sockets.emit('styleguide compile success'); - }; + emitProgressEnd(); + } io.on('connection', function(socket) { console.log('Socket connection established (id:', socket.conn.id + ')'); @@ -74,7 +69,6 @@ module.exports = function(ioServer, options) { return { emitProgressStart: emitProgressStart, - emitProgressEnd: emitProgressEnd, emitCompileError: emitCompileError, emitCompileSuccess: emitCompileSuccess }