Coder Social home page Coder Social logo

Comments (13)

alexanderGugel avatar alexanderGugel commented on June 25, 2024 3

@STRML Yup. RxJS and observables are the future :). I don't really want to start a massive promise vs no promises discussion TBH. I don't want to use them. Reasons:

  1. They don't really simplify control flow IMO (compared to async).
  2. If you have super nested callbacks, something is wrong with your code in the first place IMO
  3. Name your callbacks, split them out and they're already more readable.
  4. Callbacks are easier to explain to people (I actually made the experience with a couple of grads that it's much easier for people to understand and explain Observables then promises). This is an important selling point IMO.
  5. Adding another promise library (such as Bluebird) as a dependency should be avoided if possible.

That being said, I don't feel religious about it, but I just don't see the need to use promises TBH.

from ied.

rstacruz avatar rstacruz commented on June 25, 2024

Example:

// install_cmd.js
  async.series([
    mkdirp.bind(null, path.join(cwd, 'node_modules', '.bin')),
    mkdirp.bind(null, path.join(config.cacheDir, '.tmp')),
    function (cb) {
      var installAll = async.map.bind(null, deps, function (target, cb) {
        var name = target[0]
        var version = target[1]
        install(node_modules, name, version, function (err, pkg) {
          cb(err && err.code === 'LOCKED' ? null : err, pkg)
        })
      })

      var exposeAll = expose.bind(null, node_modules)
      var waterfall = [ installAll, exposeAll ]

      if (argv.save) {
        waterfall.push(save.bind(null, cwd, 'dependencies'))
      }

      if (argv['save-dev']) {
        waterfall.push(save.bind(null, cwd, 'devDependencies'))
      }

      async.waterfall(waterfall, function (err) {
        if (err) throw err
        cb()
      })
    }
  ], cb)

Can be rewritten something like so:

Promise.resolve()
  .then(_ => mkdirp(path.join(cwd, 'node_modules', '.bin')))
  .then(_ => mkdirp(path.join(config.cacheDir, '.tmp')))
  .then(_ => installAll(node_modules, deps, _))
  .then(_ => exposeAll(node_modules, _))
  .then(_ => saveDeps(argv, _))
  /* might want to use `.bind` equivalents instead to support node 0.11 */

function installAll (node_modules, deps) {
  return Promise.all(deps.map(function (dep) {
    return install(node_modules, dep)
      .catch(function (err) {
        if (err.code === 'LOCKED') return err.pkg
        throw err
      })
  }))
}

function saveDeps (argv, result) {
  if (argv.save) {
    return save.bin(null, cwd, 'dependencies', result)
  } else if (argv['save-dev']) {
    return save.bin(null, cwd, 'devDependencies', result)
  } else {
    return result
  }
}

from ied.

alexanderGugel avatar alexanderGugel commented on June 25, 2024

No. I don't like promises. Sorry. Spoiler alert: I'm working on a partial rewrite of ied-install in rx though (more like a PoC). async is going to go away, but I just don't like Promises. Sorry 😞

from ied.

just-boris avatar just-boris commented on June 25, 2024

@alexanderGugel well, look forward to see your results then.

from ied.

rstacruz avatar rstacruz commented on June 25, 2024

curious, care to share why the dislike for promises?

from ied.

alexanderGugel avatar alexanderGugel commented on June 25, 2024

taste

from ied.

alexanderGugel avatar alexanderGugel commented on June 25, 2024

ahh whatever. I'm not religious about it. If you feel strongly about it, feel free to send a PR. Happy to merge if it simplifies stuff.

from ied.

mstade avatar mstade commented on June 25, 2024

This is just noise, but to give at least one reason why promises are unpalatable to some (including yours truly) is that if errors aren't handled you just won't see them, and then waste time chasing red herrings. It's also not always clear how control flows through handlers, and if you forget to return proper values from handlers you may just end up not propagating values that really should have been. Promises being async by spec as well makes it difficult to properly trace execution, since you lose the stack.

There's a lot of apparent magic in promises and using them for control flow, as opposed to the original intent – the promise of a future value, where syntactically they have limited purpose in current javascript, case in point: async/await which hides them entirely – is likely going to cause more harm than good, unless you're intimately familiar with the pitfalls.

from ied.

STRML avatar STRML commented on June 25, 2024

Bluebird catches uncaught errors, and in combination with async/await leads to really nice, idiomatic code.

from ied.

just-boris avatar just-boris commented on June 25, 2024

If you are using async library, you will get usual exception.

Unlike promises, async callbacks are not wrapped in try/catch, so you don't need do anything special here. For me, it looks like advantage of async, not promises

from ied.

STRML avatar STRML commented on June 25, 2024

Automatic try/catch is a feature, not a bug, intended to prevent unintentional error swallowing if you forget to write yet another if (err) return cb(err), or crashing if you truck right along expecting a value. They instead bubble up to the nearest .catch() handler, or are logged. It's easy to set a policy to exit the process at any uncaught exception as well.

It appears moot anyhow as there's an open RxJS rewrite PR.

from ied.

STRML avatar STRML commented on June 25, 2024

@alexanderGugel I'll support RxJS, I've been interested in figuring out what the fuss is about. Happy to help you finish the conversion. Please let me know what still needs addressing.

My hunch is that we don't really need the kind of event-based flow that RxJS provides, and that the simpler flow Promises handle (single action flowing through a number of async functions) is fine. But there's only one way to be sure, and that's to finish the rewrite.

Once the RxJS rewrite is finished and Babel is in master it would be trivial to rewrite the async parts to use async/await if we determine it's simpler.

from ied.

alexanderGugel avatar alexanderGugel commented on June 25, 2024

RxJS rewrite complete and merged into master. 🎆

from ied.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.