-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one minor query
|
||
map<T, R>(this: Promise<T[]>, iterator: (item: T, index: number) => R | PromiseLike<R>): Promise<R[]> { | ||
return this.then(items => { | ||
if (!Array.isArray(items)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps allow any Iterable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave this a shot but it looks like TypeScript has issues (e.g. microsoft/TypeScript#6842) using iterators when targeting ES5 code. Thanks for the suggestion though!
index.d.ts
Outdated
* @see http://bluebirdjs.com/docs/api/finally.html | ||
*/ | ||
finally(handler: (result?: T, error?: Error) => any): Promise<T>; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rm line
index.d.ts
Outdated
* | ||
* @see http://bluebirdjs.com/docs/api/finally.html | ||
*/ | ||
finally(handler: (result?: T, error?: Error) => any): Promise<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said before, this doesn't conform the bluebird implementation, that's generally fine, just add a note.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yep, wanted to fix that, forgot to
* @see http://bluebirdjs.com/docs/api/catch.html | ||
*/ | ||
catch<E extends Error, U>(errorCls: { new(...args: any[]): E }, onReject: (error: E) => U | PromiseLike<U>): Promise<U | T>; | ||
catch<E extends Error>(errorCls: { new(...args: any[]): E }, onReject: (error: E) => T | PromiseLike<T> | void | PromiseLike<void>): Promise<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the error predicate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not supported here, though I considered it. It would require a lot more code / dependencies, the polyfill right now is quite streamlined.
@@ -0,0 +1,45 @@ | |||
interface Promise<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this be overridden when index.ts
is being build?
Can we please stick to having a src
and a dist
folder? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, I told tsc not to emite declarations.
could have been easier to maintain (but slightly larger) if you just map bluebird methods to native promise |
@saadataz massive package size bloat. |
See the readme for info :smil