Skip to content

Make Collection updates more seperate and don't auto logout client with bad internet #93

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 25 commits into from

Conversation

ToyboxZach
Copy link
Contributor

No description provided.

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2022

This pull request introduces 3 alerts when merging 4a2f131 into 0f00542 - view on LGTM.com

new alerts:

  • 3 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2022

This pull request introduces 3 alerts when merging d5dc78a into 0f00542 - view on LGTM.com

new alerts:

  • 3 for Unused variable, import, function or class

@ToyboxZach
Copy link
Contributor Author

This PR looks bigger than it is because I used prettier, like I expected master to have been using.

@ToyboxZach
Copy link
Contributor Author

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

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2022

This pull request introduces 3 alerts when merging 7d1acde into 0f00542 - view on LGTM.com

new alerts:

  • 3 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2022

This pull request introduces 1 alert when merging 598c2cf into 0f00542 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@TheRealNate
Copy link
Collaborator

Hey @ToyboxZach, thinking due to the size of the change maybe we merge this onto dev branch? From there I can publish a beta version for people to test.

@ToyboxZach
Copy link
Contributor Author

Ok moved to dev

@ToyboxZach ToyboxZach closed this Jun 23, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants