Merged
Conversation
Merge pull request #99 from meteorrn/update-dependencies
…y better Merge pull request #111 from ToyboxZach/Merge Restructure how we handle the Tracker, instead of a global listen to any change make the listens happen on specific queries and collections. Also fixes some bugs where we would generate way too many instances of subscriptions. This also fixes the log in flow so the series of loggingIn -> LoggedIN is consistent and doesn't allow the package to ever auto log you out as that should be handled by the owning app just like in base meteor. Every time you register a callback it gets added to a generic "changed" callback, that means that every single callback needs to be called on every single change to a collection which causes a huge hold up on the UI thread. To make it even worst the subscriptions don't get cleaned up when you resubscribe so every time a "withTracker" or "useTracker" was called you would get stacked subscriptions. This then would lead to 100 plus subscriptions for a single component after some time which leads to bigger and bigger slow downs. My PR separates that out as much as possible and makes sure that you only have as many subscriptions as you actually need. It is a pretty hefty change and I don't personally use and local collections in my app so those are going to be pretty poorly tested, but if other people want to take my change as a jumping off point I think its a pretty good point for server only subscriptions. This also fixes up weirdness with the logging in and order of events where you would have an invalid user state or it would force you to logout because of bad internet Likely fixes these erorrs: #75 #58 and maybe #79
Member
Author
|
@TheRealNate with this release we will have 0 vulnerabilities, a pretty good coverage and even a few bugs fixed due to #111 Coverage report: |
not-ai-yet
approved these changes
Mar 22, 2023
Collaborator
|
Seems like it will be a big help! Approved, LGTM. |
Member
Author
|
It's merged but I still can't publish to NPM registry, can you do that or add me to the ord on NPM (I have 2FA active on NPM) |
Collaborator
|
@jankapunkt sure can do, same username? |
Member
Author
|
@TheRealNate thanks I just got the invitation and will publish now |
Member
Author
|
Hm seems to be revoked, anyway my account is https://www.npmjs.com/~jkuester |
Collaborator
|
Added! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This merges the current dev state into
master, making 2.4.0 the new main HEAD.