Skip to content

Commit 89a549b

Browse files
lfacdomenic
lfac
authored andcommitted
Make isPromise always return a boolean.
`isPromise` returned the object to test when it was falsy, instead of returning false. For example `isPromise(null)` would return `null`. For the sake of consistency `isPromise` now always returns a boolean. Also updates `isPromiseAlike` and adds tests for both. Finally this also fixes an edge-case bug with `isPromise` and `isPromiseAlike` where truthy non-objects who had `promiseDispatch` or `then` methods would be false positives. For example, if someone added a `then` method to `Boolean.prototype`, then `isPromiseAlike(true)` would return `true`.
1 parent 02040f9 commit 89a549b

File tree

3 files changed

+52
-2
lines changed

3 files changed

+52
-2
lines changed

CHANGES.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<!-- vim:ts=4:sts=4:sw=4:et:tw=60 -->
22

3+
## 0.9.4 (unreleased)
4+
- `isPromise` and `isPromiseAlike` now always returns a boolean
5+
(even for falsy values). #284 @lfac-pt
6+
37
## 0.9.3
48

59
- Add the ability to give `Q.timeout`'s errors a custom error message. #270

q.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,10 @@ var object_keys = Object.keys || function (object) {
253253

254254
var object_toString = uncurryThis(Object.prototype.toString);
255255

256+
function isObject(value) {
257+
return value === Object(value);
258+
}
259+
256260
// generator related shims
257261

258262
function isStopIteration(exception) {
@@ -643,12 +647,12 @@ function valueOf(value) {
643647
*/
644648
Q.isPromise = isPromise;
645649
function isPromise(object) {
646-
return object && typeof object.promiseDispatch === "function";
650+
return isObject(object) && typeof object.promiseDispatch === "function";
647651
}
648652

649653
Q.isPromiseAlike = isPromiseAlike;
650654
function isPromiseAlike(object) {
651-
return object && typeof object.then === "function";
655+
return isObject(object) && typeof object.then === "function";
652656
}
653657

654658
/**

spec/q-spec.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2132,6 +2132,48 @@ describe("node support", function () {
21322132

21332133
});
21342134

2135+
describe("isPromise", function () {
2136+
it("returns true if passed a promise", function () {
2137+
expect(Q.isPromise(Q.resolve(10))).toBe(true);
2138+
});
2139+
2140+
it("returns false if not passed a promise", function () {
2141+
expect(Q.isPromise(undefined)).toBe(false);
2142+
expect(Q.isPromise(null)).toBe(false);
2143+
expect(Q.isPromise(10)).toBe(false);
2144+
expect(Q.isPromise("str")).toBe(false);
2145+
expect(Q.isPromise("")).toBe(false);
2146+
expect(Q.isPromise(true)).toBe(false);
2147+
expect(Q.isPromise(false)).toBe(false);
2148+
expect(Q.isPromise({})).toBe(false);
2149+
expect(Q.isPromise({
2150+
then: function () {}
2151+
})).toBe(false);
2152+
expect(Q.isPromise(function () {})).toBe(false);
2153+
});
2154+
});
2155+
2156+
describe("isPromiseAlike", function () {
2157+
it("returns true if passed a promise like object", function () {
2158+
expect(Q.isPromiseAlike(Q.resolve(10))).toBe(true);
2159+
expect(Q.isPromiseAlike({
2160+
then: function () {}
2161+
})).toBe(true);
2162+
});
2163+
2164+
it("returns false if not passed a promise like object", function () {
2165+
expect(Q.isPromiseAlike(undefined)).toBe(false);
2166+
expect(Q.isPromiseAlike(null)).toBe(false);
2167+
expect(Q.isPromiseAlike(10)).toBe(false);
2168+
expect(Q.isPromiseAlike("str")).toBe(false);
2169+
expect(Q.isPromiseAlike("")).toBe(false);
2170+
expect(Q.isPromiseAlike(true)).toBe(false);
2171+
expect(Q.isPromiseAlike(false)).toBe(false);
2172+
expect(Q.isPromiseAlike({})).toBe(false);
2173+
expect(Q.isPromiseAlike(function () {})).toBe(false);
2174+
});
2175+
});
2176+
21352177
if (typeof require === "function") {
21362178
var domain;
21372179
try {

0 commit comments

Comments
 (0)