-
Couldn't load subscription status.
- Fork 2
Add more *Async APIs for Collection and Cursor methods #386
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -5,16 +5,27 @@ const ERROR_CODES = Object.freeze({ | |
| }); | ||
|
|
||
| // $FlowFixMe: meteor/mongo doesn't have nice Flow types | ||
| function extend(Mongo: Object) { | ||
| function extendCollection(Mongo: Object) { | ||
| if (!Mongo) { | ||
| throw new Error('Mongo should be truthy!'); | ||
| throw new Error('Mongo object must exist'); | ||
| } | ||
|
|
||
| if (!Mongo.Collection || typeof Mongo.Collection !== 'function') { | ||
| throw new Error('Mongo.Collection is not a prototypable constructor!'); | ||
| throw new Error('Mongo.Collection must be a function/class'); | ||
| } | ||
|
|
||
| Object.assign(Mongo.Collection.prototype, { | ||
| findOneAsync(selector, options) { | ||
| return new Promise((resolve, reject) => { | ||
| try { | ||
| // $FlowExpectedError[object-this-reference] | ||
| resolve(this.findOne(selector, options)); | ||
| } catch (error) { | ||
| reject(error); | ||
| } | ||
| }); | ||
| }, | ||
|
|
||
| updateAsync(selector, modifier, options) { | ||
| return new Promise((resolve, reject) => { | ||
| // $FlowExpectedError[object-this-reference] | ||
|
|
@@ -128,4 +139,67 @@ function extend(Mongo: Object) { | |
| }); | ||
| } | ||
|
|
||
| module.exports = { extend }; | ||
| // $FlowFixMe: meteor/mongo doesn't have nice Flow types | ||
|
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. We should migrate to TypeScript at some point 😏 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. You're right -- Flow seems especially useless for this package since we aren't even explicitly typing the inputs/outputs of these extended functions, and we are just wildly suppressing everything. Buuuuuut I didn't want to bring that into scope here, so I left it alone and put more band-aids for now 😅 |
||
| function extendCursor(anyCursor: Object) { | ||
| /* | ||
| On the server, meteor/mongo does not export its Cursor type directly. | ||
| So, the only reliable way to get the prototype is to actually find to create a cursor. | ||
| We ask the consumer to inject this so that we don't depend on any particular collection existing. | ||
| */ | ||
| const cursorPrototype = anyCursor && Object.getPrototypeOf(anyCursor); | ||
| if (!anyCursor || !cursorPrototype) { | ||
| throw new Error('Cursor and its prototype must exist'); | ||
| } | ||
|
|
||
| Object.assign(cursorPrototype, { | ||
| forEachAsync(callback, thisArg) { | ||
| return new Promise((resolve, reject) => { | ||
| try { | ||
| // $FlowExpectedError[object-this-reference] | ||
| this.forEach(callback, thisArg); | ||
| resolve(); | ||
| } catch (error) { | ||
| reject(error); | ||
| } | ||
| }); | ||
| }, | ||
|
|
||
| mapAsync(callback, thisArg) { | ||
| return new Promise((resolve, reject) => { | ||
| try { | ||
| // $FlowExpectedError[object-this-reference] | ||
| resolve(this.map(callback, thisArg)); | ||
| } catch (error) { | ||
| reject(error); | ||
| } | ||
| }); | ||
| }, | ||
|
|
||
| fetchAsync() { | ||
| return new Promise((resolve, reject) => { | ||
| try { | ||
| // $FlowExpectedError[object-this-reference] | ||
| resolve(this.fetch()); | ||
| } catch (error) { | ||
| reject(error); | ||
| } | ||
| }); | ||
| }, | ||
|
|
||
| countAsync() { | ||
| return new Promise((resolve, reject) => { | ||
| try { | ||
| // $FlowExpectedError[object-this-reference] | ||
| resolve(this.count()); | ||
| } catch (error) { | ||
| reject(error); | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| module.exports = { | ||
| extendCollection, | ||
| extendCursor | ||
| }; | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
[minor] Can you add a line saying that
accounts-basepackage is required?Probably most meteor projects already have accounts installed and/or this requirement can be communicated dynamically with an error message but it would be great to know of this prerequisite in the readme (and the reason behind it since it might be a little bit strange to require a package that seems to be unrelated if someone doesn't know the details 😄 ).
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.
Do you think it would make sense to just ask the consumer to directly pass in the result of a
find()instead, so that we can inject & remove that dependency?The downside is that it exposes this "hack" to the consumer a little bit more. The upside is that it removes this dependency on
accounts-base, which you're right is a little weird (I was trying to pick a collection I expect to exist in every Meteor instance, but I guess you're right that it's still a dep)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.
Update: Okay, I went ahead and made this change to ask the consumer to pass in a cursor, rather than assuming that
Meteor.usersis a valid collection. Thanks for pointing out the implicit dependency!