Skip to content

Conversation

@orangemug
Copy link
Owner

No description provided.

@orangemug
Copy link
Owner Author

@oliverbrooks maybe we should we remove support for callbacks all together... thoughts?

@oliverbrooks
Copy link
Collaborator

Looking good. Got a comment which is a matter of preference on how to use promises.

I like to have a coroutine which is uses purely promises or purely callbacks. Responding to both is then handled at the end of the function.

For example, with promises:

function readFile (filepath) {
  return new Promise(function (resolve, reject) {
    fs.readFile(filepath, function (err, raw) {
      if (err) {
        reject(err);
      } else {
        resolve(raw);
      }
    });
  });
}

module.exports = function (file, opts, done) {
  return readFile(file)
  .then(function (file) {
    // Do something promisey
  })
  .then(function (file) {
    // More promisey stuff
  })
  .then(function (result) {
    // Handle success for callback and promise
    if (done) {
      done(null, result);
    } else {
      return result;
    }
  })
  .catch(function (err) {
    if (done) {
      done(err)
    } else {
      throw err;
    }
  })}
}

@oliverbrooks
Copy link
Collaborator

Regarding removing support for callbacks. I haven't got anything against that anymore.

It's essentially that either you include the promise > callback boilerplate for everyone or those who need it add it to their code. Pretty much everyone is familiar with how to convert promise to callback nowadays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants