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

fix: issue with collection hooks #404

Closed
wants to merge 24 commits into from

Conversation

jdgjsag67251
Copy link

This package performs multiple find operations. This can cause issues when the Meteor collection hooks package is installed; since this can add restrictions on the selectors which may prevent updates from being sent.
This PR just adds a check if the direct key is available, if so it will use that for all internal find operations.

@@ -164,6 +164,9 @@ class RedisSubscriptionManager {
debug(
`[RedisSubscriptionManager] Exception while processing event: ${e.toString()}`
);
Meteor._debug(
Copy link
Author

Choose a reason for hiding this comment

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

Previously these were just swallowed when debug was not turned on

@jdgjsag67251 jdgjsag67251 changed the title Fix issue with collection hooks fix: issue with collection hooks Apr 10, 2024
@StorytellerCZ
Copy link
Collaborator

Any chance of fixing the tests?

@jdgjsag67251
Copy link
Author

BTW, when I tried running the tests locally I ran into a syntax error in Chai because it uses some JS features that are not available in Node v14. I installed chai@legacy and it started working.

@StorytellerCZ
Copy link
Collaborator

Once the tests pass I will release it as a patch.

@StorytellerCZ
Copy link
Collaborator

@jdgjsag67251 some issues still remain.

@jdgjsag67251
Copy link
Author

@jdgjsag67251 some issues still remain.

Yeah looks like the issue I had with chai...

@jdgjsag67251 jdgjsag67251 closed this by deleting the head repository Sep 30, 2024
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