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

Implemented Fibers-less MongoDB count methods. #12295

Merged
merged 6 commits into from
Nov 21, 2022

Conversation

radekmie
Copy link
Collaborator

@radekmie radekmie commented Nov 4, 2022

Just as announced at my Meteor Impact 2022 presentation, I wanted to bring the Meteor's Collections API on par with the native MongoDB driver. It's rather natural and doesn't cause any problems, as the method names mostly don't overlap anymore (e.g., insertOne instead of insert).

In this pull request, I add both countDocuments and estimatedDocumentCount. (Yes, both collection.count() and cursor.count() calls are deprecated.) On the server side, it uses the MongoDB driver directly, without any Meteor magic, and no Fibers whatsoever. On the client side, it's as simple as a call to countAsync.

NOTE: It's not ready to merge just yet, as it requires some documentation, but the code is there.

Copy link
Contributor

@Grubba27 Grubba27 left a comment

Choose a reason for hiding this comment

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

Does look good

@Grubba27
Copy link
Contributor

Grubba27 commented Nov 7, 2022

Do want to squeeze this into 2.8.1?

@denihs
Copy link
Contributor

denihs commented Nov 7, 2022

It looks more of a minor change. I think it should go on 2.9.

@radekmie
Copy link
Collaborator Author

radekmie commented Nov 7, 2022

Yeah, I'd also go with 2.9. There's also no documentation for it yet, and I probably won't find time for that in the next few days.

@Grubba27 Grubba27 added this to the Release 2.9 milestone Nov 8, 2022
@radekmie
Copy link
Collaborator Author

I added some documentation, but I'm not sure how to tackle it in general. Meteor never really listed all options in the docs, and now there's just way too many to do that. There's also a few client-specific ones too. For now I added only some short block and would like to hear what you think about it.

@denihs
Copy link
Contributor

denihs commented Nov 14, 2022

I have nothing against your approach here.

I think we just need to add those here with a little description of the methods, and we're good to go.

Maybe even add a "(deprecated)" to the count method?

@radekmie
Copy link
Collaborator Author

@denihs I added a short note about Cursor#count being deprecated and used the @deprecated JSDoc. If you think I should add more, let me know.

@Grubba27
Copy link
Contributor

Hey @radekmie can I merge this to 2.9?

@denihs
Copy link
Contributor

denihs commented Nov 18, 2022

I think it's good to go now @Grubba27

@radekmie
Copy link
Collaborator Author

Yeah, it's good to go.

@Grubba27 Grubba27 changed the base branch from devel to release-2.9 November 21, 2022 14:04
@Grubba27 Grubba27 merged commit 854fb7c into release-2.9 Nov 21, 2022
@Grubba27 Grubba27 mentioned this pull request Nov 30, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants