From 4ce08c90f3c94a4c046c17deb60aba5132d4b95d Mon Sep 17 00:00:00 2001 From: Petka Antonov Date: Tue, 4 Feb 2014 20:44:02 +0200 Subject: [PATCH] Fix progress not passed through from cast promises. Closes #88 --- src/progress.js | 3 +- src/promise.js | 11 ++--- src/thenables.js | 55 ++++++++++++++++++----- test/mocha/bluebird-multiple-instances.js | 24 ++++++++++ test/mocha/q_progress.js | 29 ++++++++++++ 5 files changed, 105 insertions(+), 17 deletions(-) diff --git a/src/progress.js b/src/progress.js index 893b5d9e4..7b9afcf7d 100644 --- a/src/progress.js +++ b/src/progress.js @@ -93,7 +93,8 @@ module.exports = function(Promise, isPromiseArrayProxy) { if (typeof handler === "function") { handler.call(receiver, progressValue, promise); } - else if (Promise.is(receiver) && receiver._isProxied()) { + else if (typeof receiver._isProxied === "function" && + receiver._isProxied()) { receiver._progressUnchecked(progressValue); } else if (isPromiseArrayProxy(receiver, promise)) { diff --git a/src/promise.js b/src/promise.js index 15ef7d5de..a9c163e3f 100644 --- a/src/promise.js +++ b/src/promise.js @@ -422,8 +422,9 @@ function Promise$_then( if (debugging && !haveInternalData) { var haveSameContext = this._peekContext() === this._traceParent; ret._traceParent = haveSameContext ? this._traceParent : this; - ret._setTrace(typeof caller === "function" ? - caller : this._then, this); + ret._setTrace(typeof caller === "function" + ? caller + : this._then, this); } if (!haveInternalData && this._isBound()) { @@ -759,7 +760,6 @@ function Promise$_settlePromiseFromHandler( Promise.prototype._follow = function Promise$_follow(promise) { ASSERT(arguments.length === 1); - ASSERT(isPromise(promise)); ASSERT(this._isFollowingOrFulfilledOrRejected() === false); ASSERT(promise !== this); this._setFollowing(); @@ -923,7 +923,8 @@ Promise.prototype._settlePromiseAt = function Promise$_settlePromiseAt(index) { //optimization when .then listeners on a promise are //just respective fate sealers on some other promise if (receiver !== void 0) { - if (receiver instanceof Promise && receiver._isProxied()) { + if (typeof receiver._isProxied === "function" && + receiver._isProxied()) { //Must be smuggled data if proxied ASSERT(!isPromise(promise)); receiver._unsetProxied(); @@ -1182,7 +1183,7 @@ if (!CapturedTrace.isSupported()) { Promise._makeSelfResolutionError = makeSelfResolutionError; require("./finally.js")(Promise, NEXT_FILTER); require("./direct_resolve.js")(Promise); -require("./thenables.js")(Promise); +require("./thenables.js")(Promise, INTERNAL); Promise.RangeError = RangeError; Promise.CancellationError = CancellationError; Promise.TimeoutError = TimeoutError; diff --git a/src/thenables.js b/src/thenables.js index dc503176c..25ff9cee1 100644 --- a/src/thenables.js +++ b/src/thenables.js @@ -1,10 +1,9 @@ "use strict"; -module.exports = function(Promise) { +module.exports = function(Promise, INTERNAL) { var ASSERT = require("./assert.js"); var util = require("./util.js"); var errorObj = util.errorObj; var isObject = util.isObject; - var tryCatch2 = util.tryCatch2; function getThen(obj) { try { @@ -38,21 +37,47 @@ module.exports = function(Promise) { return obj; } + function isAnyBluebirdPromise(obj) { + try { + return typeof obj._resolveFromSyncValue === "function"; + } + catch(ignore) { + return false; + } + } + function Promise$_doThenable(x, then, caller, originalPromise) { ASSERT(typeof then === "function"); ASSERT(arguments.length === 4); - var resolver = Promise.defer(caller); + //Make casting from another bluebird fast + if (isAnyBluebirdPromise(x)) { + var ret = new Promise(INTERNAL); + ret._follow(x); + ret._setTrace(caller, void 0); + return ret; + } + return Promise$_doThenableSlowCase(x, then, caller, originalPromise); + } + function Promise$_doThenableSlowCase(x, then, caller, originalPromise) { + var resolver = Promise.defer(caller); var called = false; - var ret = tryCatch2(then, x, - Promise$_resolveFromThenable, Promise$_rejectFromThenable); - - if (ret === errorObj && !called) { - called = true; - if (originalPromise !== void 0) { - originalPromise._attachExtraTrace(ret.e); + try { + then.call( + x, + Promise$_resolveFromThenable, + Promise$_rejectFromThenable, + Promise$_progressFromThenable + ); + } + catch(e) { + if (!called) { + called = true; + if (originalPromise !== void 0) { + originalPromise._attachExtraTrace(e); + } + resolver.promise._reject(e); } - resolver.promise._reject(ret.e); } return resolver.promise; @@ -81,6 +106,14 @@ module.exports = function(Promise) { resolver.promise._attachExtraTrace(r); resolver.promise._reject(r); } + + function Promise$_progressFromThenable(v) { + if (called) return; + var promise = resolver.promise; + if (typeof promise._progress === "function") { + promise._progress(v); + } + } } Promise._cast = Promise$_Cast; diff --git a/test/mocha/bluebird-multiple-instances.js b/test/mocha/bluebird-multiple-instances.js index b8eb2c778..d5fb00909 100644 --- a/test/mocha/bluebird-multiple-instances.js +++ b/test/mocha/bluebird-multiple-instances.js @@ -69,6 +69,30 @@ if( isNodeJS ) { }, 13); }); + specify("Should use fast cast", function(done) { + var a = Promise1.pending(); + var b = Promise2.cast(a.promise); + assert(b._isProxied()); + done(); + }); + + specify("Should pass through progress with fast cast", function(done){ + var a = Promise1.pending(); + var b = Promise2.cast(a.promise); + var test = 0; + b.then(function() { + test++; + }, null, function() { + test++; + }); + + a.progress(); + a.resolve(); + setTimeout(function(){ + assert.equal(test, 2); + done(); + }, 20); + }); }); } diff --git a/test/mocha/q_progress.js b/test/mocha/q_progress.js index 1e7432b56..1dc8d1dca 100644 --- a/test/mocha/q_progress.js +++ b/test/mocha/q_progress.js @@ -444,4 +444,33 @@ describe("progress", function () { done(); }, 13); }); + + specify("GH-88", function(done) { + var thenable = { + then: function(f, r, p) { + setTimeout(function(){ + var l = 10; + while(l--) { + p(4); + } + setTimeout(function(){ + f(3); + }, 13); + }, 13); + } + }; + + var promise = Promise.cast(thenable); + var count = 0; + promise.progressed(function(v){ + count++; + assert.equal(v, 4); + }); + promise.then(function(v) { + assert.equal(count, 10); + assert.equal(v, 3); + done(); + }); + + }); });