1

I've written a node script which generates a set of files and populates them automatically. However I've noticed in the output that the success messages come quite a bit after the call message.

I am not very familiar with promises or callbacks apart from when I use them in pre-existing code. How would I go about writing the following to run in a step-by-step manner?

var fileGen = function(componentName) {
    // Each createFile instance should run only after the previous one has completed.
    createFile('view.php', componentName)
    createFile('style.styl', componentName)
    createFile('script.js', componentName)
    createFile('style.print.styl', componentName)
    // finishUp should only run when all createFile instances have completed.
    finishUp()
}

var createFile = function (fileName, componentName, doc) {
  // Tell the user what is happening
  console.log(chalk.blue('\nCreating', fileName, '...'))
  // Bring in the view.php scaffold file
  var scaffold = './scaffold/' + fileName
  fs.readFile(scaffold, 'utf8', function (err, data) {
    if (err) return console.log(chalk.red(err))
    var result = data.replace(/%cname%/g, componentName)
    if (doc) {
      var d = new Date()
      result = result.replace(/%cfname%/g, doc.componentName)
      result = result.replace(/%cdesc%/g, doc.componentDesc)
      result = result.replace(/%cauthor%/g, doc.userName)
      result = result.replace(/%cagithub%/g, doc.userGithub)
      result = result.replace(/%creationdate%/g, d.getDate() + '/' + d.getMonth() + '/' + d.getFullYear())
    }

    fs.writeFile('./src/components/' + componentName + '/' + fileName, result, function (err) {
      if (err) return console.log(chalk.red(err))
      console.log(chalk.green(fileName, 'created!'))
    })
  })
}

I feel like this is something I should be able to do (and probably have done before) but for some reason I can't get my brain around it today.

Andreas
  • 21,535
  • 7
  • 47
  • 56
Alex Foxleigh
  • 1,784
  • 2
  • 21
  • 47
  • [Using Promises with fs.readFile in a loop](http://stackoverflow.com/questions/34628305/using-promises-with-fs-readfile-in-a-loop), [fs-es6-promise](https://www.npmjs.com/package/fs-es6-promise) – Andreas Mar 30 '17 at 16:31
  • Ah, I've heard of Bluebird but didn't know what it was. Thank you, I'll take a look. – Alex Foxleigh Mar 30 '17 at 16:33
  • What do you mean by "*when I use them in pre-existing code*"? You might want to have a look at [my rules of thumb](http://stackoverflow.com/a/25756564/1048572) – Bergi Mar 30 '17 at 16:37

1 Answers1

-1

Promise: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Promise

var fileGen = function(componentName) {
    // Each createFile instance should run only after the previous one has completed.
    createFile('view.php', componentName)
    .then(() => {
        createFile('style.styl', componentName);
    })
    .then(() => {
        createFile('script.js', componentName)
    })
    .then(() => {
        createFile('style.print.styl', componentName)
    })
    .then(() => {
        // finishUp should only run when all createFile instances have completed.
        finishUp()    
    });
}

var createFile = function (fileName, componentName, doc) {
  // Tell the user what is happening
  console.log(chalk.blue('\nCreating', fileName, '...'))
  // Bring in the view.php scaffold file
  var scaffold = './scaffold/' + fileName

  return new Promise((resolve, reject) => {
    fs.readFile(scaffold, 'utf8', function (err, data) {
      if (err) {
        console.log(chalk.red(err))
        reject(err)
        return
      }
      var result = data.replace(/%cname%/g, componentName)
      if (doc) {
        var d = new Date()
        result = result.replace(/%cfname%/g, doc.componentName)
        result = result.replace(/%cdesc%/g, doc.componentDesc)
        result = result.replace(/%cauthor%/g, doc.userName)
        result = result.replace(/%cagithub%/g, doc.userGithub)
        result = result.replace(/%creationdate%/g, d.getDate() + '/' + d.getMonth() + '/' + d.getFullYear())
      }

      fs.writeFile('./src/components/' + componentName + '/' + fileName, result, function (err) {
        if (err) {
            console.log(chalk.red(err))
            reject(err)
            return
        } else {
          console.log(chalk.green(fileName, 'created!'))
          resolve(); // promise is now fulfilled, the next chained `then` will be called   }
      })
    })
  });
}
Amos Wong
  • 192
  • 1
  • 10
  • Thank you so much! That's one of the first times I've seen promises make complete sense to me. – Alex Foxleigh Mar 30 '17 at 16:36
  • Well, please don't do it like this. Promisify on the lowest level, i.e. `readFile` and `writeFile` separately. Also notice that `reject` is not a `return`, the rest of the code is still executed! Use `else` – Bergi Mar 30 '17 at 16:38
  • Also `Promise.all` seems to be more appropriate here than chaining – Bergi Mar 30 '17 at 16:39
  • @Bergi Wouldn't `Promise.all` make all the `createFile` functions to execute asynchronously (e.g. not in sequence as what Alex Ward wanted)? – Amos Wong Mar 30 '17 at 16:43
  • @Bergi, also if someone is not using Bluebird or similar library that provide `Promisify`, is this answer still acceptable? – Amos Wong Mar 30 '17 at 16:46
  • @AmosWong `createFile` *always* executes asynchronously. Yes, you could call them so that they would run in parallel, you only want to use `Promise.all` to sequence the `finishUp` function after all files are created. – Bergi Mar 30 '17 at 17:15
  • @AmosWong Promisification has nothing to do with Bluebird. You should *always* wrap the primitive asynchronous callback-taking functions (`fs.readFile`, `fs.writeFile`) in a small helper function that returns a promise instead of taking a callback. Then call those. Bluebird just simplifies the creation of the helpers a lots. – Bergi Mar 30 '17 at 17:17
  • @Bergi, ok now I understood your idea about promisification. Regarding to the `promise.all`, Alex Ward doesn't want `createFile` to run in parallel. He mentioned that _"// Each createFile instance should run only after the previous one has completed."_. So `Promise.all` shouldn't be used here right? – Amos Wong Mar 30 '17 at 17:25
  • Ah, I missed that comment (and can't see why it would be necessary, as they don't seem to depend on each other - so why not get the speedup for free?), I only saw "*finishUp should only run when all createFile instances have completed.*" – Bergi Mar 30 '17 at 17:29