Add findOne() functionality #233
Conversation
adamchel
left a comment
adamchel
left a comment
There was a problem hiding this comment.
Looks good to me, though I'm of the opinion we should be using undefined everywhere, since mixing undefined and null can get very confusing for users. Let's have a slack discussion about why the server returns null, and make a decision based on that. If we do end up going with null, we need to make sure we update the Promise types returned by the findOne methods.
packages/browser/services/mongodb-remote/src/RemoteMongoCollection.ts
Outdated
Show resolved
Hide resolved
packages/browser/services/mongodb-remote/src/internal/RemoteMongoCollectionImpl.ts
Outdated
Show resolved
Hide resolved
packages/core/services/mongodb-remote/src/internal/CoreRemoteMongoCollection.ts
Outdated
Show resolved
Hide resolved
tkaye407
left a comment
tkaye407
left a comment
There was a problem hiding this comment.
Now findOne() will return undefined
| return undefined; | ||
| } | ||
|
|
||
| const obj = EJSON.parse(response.body!, { strict: false }); |
There was a problem hiding this comment.
This is what I added, let me know if this is too sketchy
There was a problem hiding this comment.
This is very sketchy, and I don't think it's a check we should be making for every request.
Would it be better to do the following
if (obj === null) {
return undefined;
}
after the EJSON.parse call?
3 participants
Not sure if this is a big deal/wrong, but the findOne() method returns null as opposed to undefined when there are no matching documents. We can discuss if this is not the right behaviour.