-
Notifications
You must be signed in to change notification settings - Fork 13
Make methods return bluebird promise instead native promise #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
84b8702
a327e54
1320acc
6cc65ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| const util = require("./util"); | ||
| module.exports = (Bluebird) => { | ||
| Bluebird.filter = (x, predicate, opts) => Bluebird.resolve(x).filter(predicate, opts); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change seems unnecessary.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it wasn't..
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because you resolve before you do the filter, you need to resolve the filter values.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are not,
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test so we can see? |
||
| Bluebird.filter = (x, predicate, opts) => Bluebird.resolve((async () => | ||
| Bluebird.resolve(x).filter(predicate, opts) | ||
| )()); | ||
|
|
||
| Bluebird.prototype.filter = async function(predicate, {concurrency} = {}) { | ||
| const values = await this.all(); | ||
| const predicateResults = await this.map(predicate, {concurrency}); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| const util = require("./util"); | ||
| module.exports = (Bluebird) => { | ||
| Bluebird.map = (x, mapper, opts) => Bluebird.resolve(x).map(mapper, opts); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change seems unnecessary. |
||
| Bluebird.map = (x, mapper, opts) => Bluebird.resolve((async () => | ||
| Bluebird.resolve(x).map(mapper, opts) | ||
| )()); | ||
|
|
||
| Bluebird.prototype.map = async function(mapper, {concurrency} = {}) { | ||
| const values = await Bluebird.all(await this); | ||
|
|
||
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.
This change seems unnecessary.
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.
it wasn'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.
But
Bluebird.resolvealready returns a bluebird instance, so why would wrapping help?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.
Because you resolve before you do the any, you need to resolve the any values.
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.
But
.any()already returns a bluebird promise.