7
\$\begingroup\$

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']);
});
asked Sep 15, 2014 at 22:01
\$\endgroup\$
3
  • \$\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\$ Commented Sep 15, 2014 at 22:38
  • \$\begingroup\$ @SimonAndréForsberg Thanks for pointing out. I have added the git repo link for more info \$\endgroup\$ Commented 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\$ Commented Sep 15, 2014 at 23:25

1 Answer 1

3
\$\begingroup\$

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', ...

answered Sep 15, 2014 at 22:51
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.