Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm looking for some code review of my usage of async. Mainly why this does not work without the done = this.async() method and why I don't have to invoke 'done()' on every request to download a file. This is working now but it is a bit of magic to me. Hoping to understand why it's working and if I'm abusing something.

   /*
   * grunt-esri-slurp
   * https://github.com/steveoh/grunt-esri-slurp
   *
   * Copyright (c) 2014 steveoh
   * Licensed under the MIT license.
   */

    'use strict';
    var fs = require('fs'),
    path = require('path'),

    async = require('async'),
    mkdirp = require('mkdirp'),
    request = require("request"),
    S = require('string'),

    unwind = require('./unwinder'),
    esriModules = require('./esriModules');

  module.exports = function(grunt) {
    grunt.registerTask('esri_slurp', 'download esri js api amd modules and create a package', function() {
      var options = this.options({
          packageLocation: 'src/esri/',
          version: '3.9'
        }),
        done = this.async();

      options.packageLocation = S(options.packageLocation).ensureRight('/').s;

      grunt.log.subhead('downloading esri version ' + options.version + ' modules');

      mkdirp.sync(options.packageLocation);

      var esriVersionBaseUrl = 'http://js.arcgis.com/' + options.version + 'amd/js/esri/';
      grunt.verbose.writeln('esri base url: ' + esriVersionBaseUrl);

      async.eachLimit(esriModules, 10, function(file, callback) {
        var subPath = file.substr(0, file.lastIndexOf('/') + 1),
          fileFolder = options.packageLocation + subPath,
          fileName = path.basename(file),
          httpUrl = esriVersionBaseUrl + subPath + fileName;

        if (!fs.existsSync(fileFolder)) {
          grunt.verbose.writeln(['creating folder ' + fileFolder]);

          mkdirp.sync(fileFolder);
        }

        grunt.verbose.writeln(['requesting ' + httpUrl]);

        request({
            uri: httpUrl,
            encoding: 'binary'
          },
          function(error, response, body) {
            if (body && body.length > 0) {
              grunt.verbose.or.write('.');
              grunt.verbose.writeln(['writing: ' + options.packageLocation + file]);

              var f = S(file);
              if (f.endsWith('.js') || f.endsWith('.css')) {
                body = unwind(body);
              }

              fs.writeFile(options.packageLocation + file, body, 'binary');
            }

            callback(error, body);
          });
      });
    });
  };
share|improve this question
    
i'm also working on using the path module to remove platform specific separators. –  Steve May 29 at 22:49

1 Answer 1

up vote 2 down vote accepted

this.async() tells grunt that you're doing something async and that it should wait until you call the done method. Usually it just goes to the next grunt task when the execution is done. Since you're using the async module you can call the done method in the third argument to async.eachLimit which is supposed to be a function that is called when all the work is done.

// tell grunt we're doing something async
var done = this.async();

async.eachLimit(['unicorn', 'rainbows'], 10, function(file, cb) {
    // do something async
    cb(); // tell `async` we're done doing work for this item
}, function (err) { // called when all the items have called their `cb()`
    if (err) {
        grunt.warn(err);
    }

    // all the items are done, let's tell grunt we are done
    done();
});
share|improve this answer
    
thanks @Sindre. I will add the third callback to each and done() it in there, but how is it that it works now as is? –  Steve May 29 at 22:51
1  
@Steve It shouldn't, as done is never called so grunt will just wait forever. Might be that grunt has an async timeout or something. Either way, it's just pure luck if it does work. –  Sindre Sorhus May 29 at 23:25
    
crazy how that works @Sindre. npmjs.org/package/grunt-esri-slurp is up and working great! thanks again. –  Steve May 30 at 0:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.