jprichardson / angular-bluebird

An AngularJS service for Bluebird promise library.
8 stars 0 forks source link

Promise.defer is deprecated, and $timeout already returns a promise #1

Open benjamingr opened 10 years ago

benjamingr commented 10 years ago

So this:


function asyncAskQuestion(question) {
    var deferred = $bluebird.defer()

    $timeout(function() { //simulate async
      var result = $window.prompt(question)
      if (typeof result == 'string') { //OK Pressed
        deferred.resolve({okPresssed: true, answer: result})
      } else { //'Canceled Pressed'
        deferred.reject({okPressed: false})
      }
    }, 250)

    return deferred.promise
  }

Can actually be this:

function asyncAskQuestion(question) {
    return $bluebird.delay(250).then(function(){
      var result = $window.prompt(question)
      if (typeof result === 'string') return ({okPresssed: true, answer: result});
      throw new Error(false); // or whatever for rejection
    });
  }
jprichardson commented 10 years ago

Thanks.

  1. The defer() in this example part comes from angular-bluebird. I wasn't aware that defer() was also in Bluebird.
  2. The $timeout was suppose to be a contrived example. But, lol, your example is much more succinct and arguably better.
  3. I'm starting to believe this library is actually a very bad idea. Promise.setScheduler (Bluebird) with Angular's $digest cycle has a lot of bad repercussions. Especially when you want to use Bluebird promises in non-ui code... like a database wrapper. Maybe it's just best to use Angular's $q for UI related code and Bluebird promises elsewhere??
benjamingr commented 10 years ago

One option would be to implement a disposer for the digest cycle and use Promise.using to "commit" actions to the digest cycle only when a whole chain is done.

joshperry commented 10 years ago

TL;DR: Bluebird's docs give this as an explicit example of using setScheduler.

I believe you're thinking about this from the wrong direction. When Bluebird needs to wait for the IO stack to do work before checking for a promise progressing, it defers that future check to the "scheduler". The "scheduler" is usually something like setTimeout or process.nextTick for browsers and node respectively. When using $rootScope.$evalAsync as the scheduler, it merely defers future scheduling of promise resolution to Angular's scheduler which, purposefully, coincides with the digest cycle.

  $evalAsync: function(expr) {
    // if we are outside of an $digest loop and this is the first time we are scheduling async
    // task also schedule async auto-flush
    if (!$rootScope.$$phase && !asyncQueue.length) {
      $browser.defer(function() {
        if (asyncQueue.length) {
          $rootScope.$digest();
        }
      });
    }

    asyncQueue.push({scope: this, expression: expr});
  },

This simply pushes "process this after the next async cycle" functions onto a queue. In the simple path, this code uses the $browser scheduler to call $digest during the browser's next async cycle. The $browser scheduler, oddly enough, uses the setTimeout(fn,0) trick to schedule the next async cycle.

self.defer = function(fn, delay) {
  var timeoutId;
  outstandingRequestCount++;
  timeoutId = setTimeout(function() {
    delete pendingDeferIds[timeoutId];
    completeOutstandingRequest(fn);
  }, delay || 0);
  pendingDeferIds[timeoutId] = true;
  return timeoutId;
};

On the next async cycle, $digest is called and at the top of that function is this loop:

      while (asyncQueue.length) {
        try {
          asyncTask = asyncQueue.shift();
          asyncTask.scope.$eval(asyncTask.expression);
        } catch (e) {
          $exceptionHandler(e);
        }
        lastDirtyWatch = null;
      }

It walks through the queue, processing any deferred functions one-at-a-time. Once the queue has been drained, $digest goes through and updates any modified watchers which, in turn, executes bindings, et al.

From Bluebird's perspective, it's async queue is getting flushed by setTimeout(fn,0), which naked Bluebird uses by default if process.nextTick or MutationObserver are not available anyway. From angular's perspective, all deferrals (including promise deferrals) are processed before the digest cycle updates watchers, giving it access to resolved values or rejections (assuming any async IO or DOM event processing has completed, as the case may be).

For reference, Bluebird's default scheduler selection:

if (typeof process === "object" && typeof process.version === "string") {
    schedule = function Promise$_Scheduler(fn) {
        process.nextTick(fn);
    };
}
else if ((typeof MutationObserver !== "undefined" &&
         (_MutationObserver = MutationObserver)) ||
         (typeof WebKitMutationObserver !== "undefined" &&
         (_MutationObserver = WebKitMutationObserver))) {
    schedule = (function() {
        // --snip-- Set up a MutationObserver to flush the queue
     })();
}
else if (typeof setTimeout !== "undefined") {
    schedule = function Promise$_Scheduler(fn) {
        setTimeout(fn, 0);
    };
}
spacepluk commented 10 years ago

This issue is very educational :) Would you care to elaborate on the bad repercussions of using bluebird for non-ui code?

benjamingr commented 10 years ago

@spacepluk since promises in Angular are intertwined with the digest cycle, promises resolving in Angular by default schedules a digest. If you setScheduler with Bluebird - you get the exact same deal. It's "as bad" as it is when doing it with Angular's $q.

joshperry commented 10 years ago

Promises in Angular are not "intertwined" with the digest cycle. The only difference when using Angular's scheduler is that Angular can be sure that the digest cycle runs after all other async deferrals. Angular's async deferral queue is processed by setTimeout(fn, 0) just like Bluebird's is when you don't call setScheduler. Take a look at the code.

In fact, if you were to run Angular and Bluebird on the same page (without calling setScheduler) there would actually be slightly more overhead since both frameworks would be calling setTimeout separately to schedule their deferral processing. The digest cycle and async deferral queue processing will not run any more or less frequently either way.

benjamingr commented 10 years ago

@joshperry What's stopping you from using $evalAsync in setScheduler ?

joshperry commented 10 years ago

@benjamingr nothing, that's exactly what this module does... Perhaps I misunderstand your question.