I'm fairly new to JavaScript and Gulp. I was following this article for creating my gulpfile, trying to follow best practices, but it seems to me that this peace of code can be optimized and written better.
Can you please give me some advice on how can I improve this code?
Git repo can be found here: [email protected]:Skok/jekyll-gulp.git
var gulp = require('gulp'),
sass = require('gulp-sass'),
browserSync = require('browser-sync'),
autoprefixer = require('gulp-autoprefixer'),
uglify = require('gulp-uglify'),
jshint = require('gulp-jshint'),
header = require('gulp-header'),
rename = require('gulp-rename'),
minifyCSS = require('gulp-minify-css'),
concat = require('gulp-concat'),
cp = require('child_process'),
notify = require('gulp-notify'),
size = require('gulp-size'),
imageResize = require('gulp-image-resize'),
imageMin = require('gulp-imagemin'),
cwebp = require('gulp-cwebp'),
plumber = require('gulp-plumber'),
beep = require('beepbeep'),
gutil = require('gulp-util'),
reload = browserSync.reload;
package = require('./package.json');
var banner = [
'/*!\n' +
' * <%= package.name %>\n' +
' * <%= package.title %>\n' +
' * <%= package.url %>\n' +
' * @author <%= package.author %>\n' +
' * @version <%= package.version %>\n' +
' * Copyright ' + new Date().getFullYear() + '. <%= package.license %> licensed.\n' +
' */',
'\n'
].join('');
// create system beeps & highlight error messages on error
var onError = function (err) {
beep([0, 0, 0]);
gutil.log(gutil.colors.green(err));
};
// copy task for copying bower assests
gulp.task('copy', function() {
return gulp.src('bower_components/bootstrap-sass-official/assets/stylesheets/**/*.scss')
.pipe(gulp.dest('assets/scss/vendor'))
.pipe(notify('copy task COMPLETE'));
});
// jshint task to lint all js files and output errors
gulp.task('jshint', function () {
return gulp.src('assets/js/**/*.js')
.pipe(reload({stream: true, once: true}))
.pipe(jshint())
.pipe(jshint.reporter('jshint-stylish'))
.pipe(notify('jshint task COMPLETE'));
});
// browser-sync task to start server.
gulp.task('browser-sync', function() {
browserSync({
server: {
baseDir: "html"
}
});
});
// sass task runs sass and auto-updates browser
gulp.task('sass', function () {
return gulp.src('assets/scss/**/*.scss')
.pipe(sass({errLogToConsole: true}))
.pipe(header(banner, {pkg : package}))
.pipe(autoprefixer(['last 15 versions', '> 1%', 'ie 8', 'ie 7'], {cascade: true}))
.pipe(size({showFiles: true}))
.pipe(gulp.dest('html/build/css'))
.pipe(reload({stream:true}))
.pipe(gulp.dest('build/css'))
.pipe(minifyCSS())
.pipe(rename({suffix: '.min'}))
.pipe(gulp.dest('build/css'))
.pipe(size({showFiles: true}))
.pipe(size({showFiles: true, gzip: true}))
.pipe(notify('sass task COMPLETE'));
});
// js task runs all js files with jshint and auto-updates browser
gulp.task('js', ['jshint'], function () {
return gulp.src('assets/js/**/*.js')
.pipe(plumber({
errorHandler: onError
}))
.pipe(gulp.dest('html/build/js'))
.pipe(reload({stream:true}))
.pipe(uglify())
.pipe(rename({suffix: '.min'}))
.pipe(header(banner, {pkg : package}))
.pipe(gulp.dest('build/js'))
.pipe(size({showFiles: true}))
.pipe(size({showFiles: true, gzip: true}))
.pipe(notify('js task COMPLETE'));
});
// img-opt task optimizes all images
gulp.task('img-opt', function () {
return gulp.src('assets/images/*')
.pipe(imageMin({optimizationLevel: 3, progressive: true, interlaced: true}))
.pipe(gulp.dest('build/images/img-opt'))
.pipe(notify('img-opt task COMPLETE'));
});
// image-resize task resizes all images
gulp.task('image-resize', ['img-opt'], function () {
gulp.src('build/images/img-opt/*')
.pipe(imageResize({
width : 480,
height : 320,
crop : true,
upscale : false,
}))
.pipe(rename({suffix: '-480w-320h'}))
.pipe(gulp.dest('build/images'))
gulp.src('build/images/img-opt/*')
.pipe(imageResize({
width : 768,
height : 576,
crop : true,
upscale : false
}))
.pipe(rename({suffix: '-768w-576h'}))
.pipe(gulp.dest('build/images'))
gulp.src('build/images/img-opt/*')
.pipe(imageResize({
width : 992,
height : 744,
crop : true,
upscale : false
}))
.pipe(rename({suffix: '-992w-774h'}))
.pipe(gulp.dest('build/images'))
gulp.src('build/images/img-opt/*')
.pipe(imageResize({
width : 1140,
height : 855,
crop : true,
upscale : false
}))
.pipe(rename({suffix: '-1140w-855h'}))
.pipe(gulp.dest('build/images'))
gulp.src('build/images/img-opt/*')
.pipe(imageResize({
width : 960,
height : 640,
crop : true,
upscale : false
}))
.pipe(rename({suffix: '-480w-320h@2x'}))
.pipe(gulp.dest('build/images'))
gulp.src('build/images/img-opt/*')
.pipe(imageResize({
width : 1536,
height : 1152,
crop : true,
upscale : false
}))
.pipe(rename({suffix: '-768w-576h@2x'}))
.pipe(gulp.dest('build/images'))
gulp.src('assets/images/img-opt/*')
.pipe(imageResize({
width : 1984,
height : 1488,
crop : true,
upscale : false
}))
.pipe(rename({suffix: '-992w-774h@2x'}))
.pipe(gulp.dest('build/images'))
return gulp.src('build/images/img-opt/*')
.pipe(imageResize({
width : 2880,
height : 1710,
crop : true,
upscale : false
}))
.pipe(rename({suffix: '-1140w-855h@2x'}))
.pipe(gulp.dest('build/images'))
.pipe(notify('images-resize task COMPLETE'));
// reload & notify when ready
// return gulp.src('build/images/*')
// .pipe(reload({stream:true}))
// .pipe(notify('images task..............................DONE !'));
});
// cwebp task convert all images to webp
gulp.task('cwebp', ['image-resize'], function () {
gulp.src('build/images/*')
.pipe(cwebp())
.pipe(gulp.dest('build/images'))
.pipe(notify('cwebp task COMPLETE'));
});
gulp.task('images', ['img-opt', 'image-resize', 'cwebp'], function () {
return gulp.src('build/images/*')
.pipe(reload({stream:true}));
//.pipe(notify('images task COMPLETE'));
});
// jekyll-build task regenerates jekyll assessets
gulp.task('jekyll-build', function (done) {
return cp.spawn('jekyll', ['build'], {stdio: 'inherit'})
.on('close', done),
// use notify
console.log('jekyll-build COMPLETE');
});
// jekyll-rebuild task regenerates jekyll assessets and auto-updates browser
gulp.task('jekyll-rebuild', ['jekyll-build'], function () {
browserSync.reload();
});
// default task to be run with `gulp`
gulp.task('default', ['sass', 'js', 'images', 'jekyll-build'], function () {
gulp('assets/scss/**/*.scss', ['sass']),
gulp('assets/js/**/*.js', ['js']),
gulp('assets/images/*', ['images']),
gulp(['*.html', '*.md', '_posts/*', '_includes/*', '_layouts/*'], ['jekyll-build']);
});
// watch task to be run with `gulp watch`
gulp.task('watch', ['sass', 'js', 'images', 'jekyll-build', 'browser-sync'], function () {
gulp.watch('assets/scss/**/*.scss', ['sass']),
gulp.watch('assets/js/**/*.js', ['js']),
gulp.watch('assets/images/*', ['images']),
gulp.watch(['*.html', '*.md', '_posts/*', '_includes/*', '_layouts/*'], ['jekyll-rebuild']);
});
-
\$\begingroup\$ Welcome to Code Review! To make life easier for reviewers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. See also this meta question \$\endgroup\$Simon Forsberg– Simon Forsberg2014年09月15日 22:38:53 +00:00Commented Sep 15, 2014 at 22:38
-
\$\begingroup\$ @SimonAndréForsberg Thanks for pointing out. I have added the git repo link for more info \$\endgroup\$iamskok– iamskok2014年09月15日 22:48:52 +00:00Commented Sep 15, 2014 at 22:48
-
\$\begingroup\$ I was more thinking that you could describe a bit more about what your code actually does. The git link is not necessary, but it might be helpful for some as well. I have to admit that I have no clue what gulp.js is though so perhaps those more aware about what that is and how it works will have an easier time understanding what problem your code solves. \$\endgroup\$Simon Forsberg– Simon Forsberg2014年09月15日 23:25:54 +00:00Commented Sep 15, 2014 at 23:25
1 Answer 1
Duplication
Some strings appear suspiciously often:
build/images
build/css
assets/images
assets/js
assets/scss
It might be a good idea to extract these to constants. Maybe you will never change their values, but variables would have the advantage of easy autocompletion in a descent IDE.
Also, this array appears twice, and might be useful to put it in a variable near the top where it's easier to find and edit:
['*.html', '*.md', '_posts/*', '_includes/*', '_layouts/*']
Semicolons
You're using semicolons inconsistently: sometimes you end statements with it, sometimes you don't. jshint.com recommends to use them. Or at least you can be consistent about it and either use them always or never.
Correctness
What's the point of the commas at the end of each statement inside the function:
gulp.task('default', ['sass', 'js', 'images', 'jekyll-build'], function () { gulp('assets/scss/**/*.scss', ['sass']), gulp('assets/js/**/*.js', ['js']), gulp('assets/images/*', ['images']), gulp(['*.html', '*.md', '_posts/*', '_includes/*', '_layouts/*'], ['jekyll-build']); });
I might be missing something but it seems pointless and confusing.
Same thing for the last one, gulp.task('watch', ...
Explore related questions
See similar questions with these tags.