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

Add support for async collection methods #299

Merged
merged 4 commits into from
Aug 31, 2023

Conversation

jdgjsag67251
Copy link
Contributor

@jdgjsag67251 jdgjsag67251 commented Aug 28, 2023

See #288.

Ok, so apparently Meteor just internally calls the non-async methods and wraps them in a Promise: https://github.com/meteor/meteor/blob/devel/packages/mongo/collection.js#L998.
This PR just adds tests to make sure that the hooks keep working when Meteor changes their implementation.

@jdgjsag67251 jdgjsag67251 changed the title Added support for async collection methods Add support for async collection methods Aug 28, 2023
@jdgjsag67251 jdgjsag67251 marked this pull request as draft August 28, 2023 09:27
@jdgjsag67251 jdgjsag67251 marked this pull request as ready for review August 28, 2023 09:57
collection-hooks.js Outdated Show resolved Hide resolved
Copy link
Member

@StorytellerCZ StorytellerCZ left a comment

Choose a reason for hiding this comment

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

Please use *Async methods when calling DB. Other than that, it looks fine. Much apprectiated!
Once we merge this we should first move to support async functions in hooks and then move the hooks internals to use the new async functions to be ready for Meteor 3.

tests/async.js Outdated Show resolved Hide resolved
@jdgjsag67251
Copy link
Contributor Author

jdgjsag67251 commented Aug 30, 2023

Cool, thnx. I've updated the PR.

@jankapunkt
Copy link
Member

jankapunkt commented Aug 31, 2023

That's great @jdgjsag67251 👍 Thanks a lot!
@StorytellerCZ lgtm, can be merged

@StorytellerCZ StorytellerCZ merged commit d91014e into Meteor-Community-Packages:master Aug 31, 2023
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