-
Notifications
You must be signed in to change notification settings - Fork 8
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
Live query support #10
Live query support #10
Conversation
@DominikGuzei and @rhyslbw, when you have time please take a look here. |
subscriptionHandle = trackerMobxAutorun(); | ||
} else { | ||
computation.invalidate(); | ||
subscriptionHandle && subscriptionHandle.stop(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Subscriptions are stopped on each autorun invalidation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that even needed? See meteor docs
If you call Meteor.subscribe within a reactive computation, for example using Tracker.autorun, the subscription will automatically be cancelled when the computation is invalidated or stopped; it is not necessary to call stop on subscriptions made from inside autorun. However, if the next iteration of your run function subscribes to the same record set (same name and parameters), Meteor is smart enough to skip a wasteful unsubscribe/resubscribe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, this is not needed and it will cause wasteful re-subscriptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This allows also for multiple subscriptions per autorun
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably introduce array with subscription handle references to support stopping multiple subscriptions in stop()
method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darko-mijic what @RSchwan means (and the docs say) is that all subscriptions (any number) are automatically stopped when the autorun is invalidated 😉 so we do not need todo anything here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you are right. We don't even need to stop subscription in scope of autorun.stop(), ok ;)
Thanks @darko-mijic this looks great 👍 |
Nice work @darko-mijic and @RSchwan. Looks good to me |
}), | ||
}); | ||
} else { | ||
action(`${actionPrefix}: initialized`, () => { | ||
storeAttribute.replace([]); | ||
observableArray.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Maybe there is some advantage by using this new utility for observing subscriptions? |
@fabiodr Have you done any investigation into this? |
Autoruns are not very performant with large number of fetched documents (1000+) since whole observable array needs to be replaced by mobx. Cursor needs to be observed instead and only added, chaged, moved and removed documents updated in the observable array.
Here are findings from @RSchwan:
darko-mijic/mantra-sample-blog-app#1 (comment)
@RSchwan also proposed a solution here:
darko-mijic/mantra-sample-blog-app#1 (comment)
There is new
observe
export from the package which enables simplification of autoruns like in the example here:https://github.com/darko-mijic/mantra-sample-blog-app/blob/28c236bb754467713eb4ffa3af067784cb0a012e/client/autoruns/comments.js
Thanks @RSchwan!