Update user logging sequence and make collections deal with reactivity better#111
Update user logging sequence and make collections deal with reactivity better#111jankapunkt merged 18 commits intometeorrn:devfrom
Conversation
Merge dev into master
jankapunkt
left a comment
There was a problem hiding this comment.
Thanks a lot! I have mostly requests for documentation for now, this is mostly due to the circumstance that future devs may require to know why this code has been introduced at these places (which you already summarized in the PR description).
In the meantime I run this with our MVP project and will give feedback if there are any issues.
| }); | ||
| } | ||
| }; | ||
| if (observersByComp[collection]) { |
There was a problem hiding this comment.
please add one or more lines of comments:
// what this block does
// why it's needed| typeof selector == 'string' ? { _id: selector } : selector | ||
| ); | ||
|
|
||
| if (Tracker.active && Tracker.currentComputation) { |
There was a problem hiding this comment.
// what this block does
// why it's needed| }); | ||
| }); | ||
| } | ||
| let observers = getObservers('added', this._collection.name, item); |
There was a problem hiding this comment.
// what this block does
// why it's needed| }); | ||
| }); | ||
| } | ||
| let newItem = this._collection.findOne({ _id: id }); |
There was a problem hiding this comment.
// what this block does
// why it's needed| }); | ||
| } | ||
|
|
||
| let observers = getObservers('removed', this._collection.name, element); |
There was a problem hiding this comment.
// what this block does
// why it's needed| cb.callback(); | ||
| } | ||
| }); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
// what this block does
// why it's needed
src/Meteor.js
Outdated
| } | ||
| ); | ||
|
|
||
| NetInfo.fetch().then( |
There was a problem hiding this comment.
// what this block does
// why it's needed
jankapunkt
left a comment
There was a problem hiding this comment.
Took me a while since we released yesterday our app but I worked through the code and on my end this did not introduce any issues. From what I see in the code this has potential to be a good starting point for even further optimization.
I will prepare 2.3.0 for publish.
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