Skip to content
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

feat: add an internal tryNext method #2638

Merged
merged 1 commit into from
Nov 30, 2020
Merged

feat: add an internal tryNext method #2638

merged 1 commit into from
Nov 30, 2020

Conversation

mbroadst
Copy link
Member

The default cursor iteration behavior is to block until either a cursor is dead (has a cursor id of 0), or a non-empty batch is returned from the server for a getMore command. tryNext allows users to iterate a cursor optionally returning null if the getMore returns an empty batch (likely on a tailable + awaitData cursor).

NODE-2917

The default cursor iteration behavior is to block until either a
cursor is dead (has a cursor id of 0), or a non-empty batch is
returned from the server for a `getMore` command. `tryNext` allows
users to iterate a cursor optionally returning `null` if the
`getMore` returns an empty batch (likely on a tailable + awaitData
cursor).

NODE-2917
@@ -604,7 +624,11 @@ function next(cursor: AbstractCursor, callback: Callback<Document | null>): void
return cleanupCursor(cursor, () => callback(err, nextDocument(cursor)));
}

next(cursor, callback);
if (cursor[kDocuments].length === 0 && blocking === false) {
return callback(undefined, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a specific reason to use null over undefined? I suppose it makes more sense to communicate a non-existent object, so I'm not taking issue with it just wondering if there was some thinking behind it

Although maybe one reason to use undefined is so the result doesn't respond true to: typeof result === 'object'.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used null here for symmetry with our semantics around a "dead" cursor. When a cursor returns a zero cursor id, we return an explicit null to the user. I think this might have been left over from days when a Cursor inherited Readable since ending a stream is accomplished by readable.push(null), but either way the convention exists so I stuck with it

@mbroadst mbroadst merged commit 43c94b6 into master Nov 30, 2020
@mbroadst mbroadst deleted the NODE-2917/try-next branch November 30, 2020 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants