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

Tracker.Computation.onStop #3915

Closed
ccorcos opened this issue Mar 11, 2015 · 10 comments
Closed

Tracker.Computation.onStop #3915

ccorcos opened this issue Mar 11, 2015 · 10 comments
Labels

Comments

@ccorcos
Copy link

@ccorcos ccorcos commented Mar 11, 2015

I have a feature request for Tracker.Computation.onStop. I couldn't find in the codebase where subscriptions are defined -- I was curious how subscriptions are stopped when Meteor.subscribe is called inside a reactive computation. I could be wrong, but I figured it listened to Tracker.currentComputation.onInvalidate to see if the computation is stopped. I came up with this, and its pretty convenient. I'm creating a package that allows you to cache subscriptions so it is very useful.

Tracker.Computation.prototype.onStop = (func) ->
  checkNextInvalidate = =>
    this.onInvalidate (comp) ->
      if comp.stopped
        func()
      else
        Tracker.afterFlush(checkNextInvalidate)

  checkNextInvalidate()

This is basically how I'm using it.

c = Tracker.currentComputation
sub = Tracker.nonreactive -> Meteor.subscribe('posts')
c.onStop -> Meteor.setTimeout( (-> sub.stop, 1000*60*10) )

This will unsubscribe 10 minutes after the current reactive computation is stopped.

@glasser
Copy link
Member

@glasser glasser commented Mar 17, 2015

Thanks for the feature request! We welcome discussions about how to make Meteor better. If you haven't yet, check out our wiki page about feature requests on GitHub.

I'm still not sure that I understand your use case, though. When Meteor.subscribe is called in a reactive context, the subscription is stopped on any invalidation, not just on stop. While I can't see anything wrong with adding yet another event callback, I am not sure why onInvalidate doesn't help with your use case. It seems weird to me that you would want to keep subscriptions going for the next run of the computation even if they don't get renewed.

@mitar
Copy link
Collaborator

@mitar mitar commented Mar 17, 2015

@ccorcos
Copy link
Author

@ccorcos ccorcos commented Mar 24, 2015

It seems weird to me that you would want to keep subscriptions going for the next run of the computation even if they don't get renewed.

If the computation is stopped, we may not want to throw away the subscription immediately -- what if the user resubscribes to the same thing a minute later? That would be a waste to send the same data again, so it may make sense to cache these subscriptions.

I think I understand why you think its weird though. It actually makes more sense to use onInvalidate here... I would just need to keep track of which subscription to invalidate each time.

@mitar, I give a good explanation of why I'm making my own subscription caching package.

@ccorcos
Copy link
Author

@ccorcos ccorcos commented Mar 24, 2015

So I'm actually using onInvalidate instead of onStop now (version 0.0.4). It just makes much more sense.

I can't actually think of a specific situation when onStop would be more useful than onInvalidate so I'm going to close this request. Maybe someone will find it here and find a use for it.

@ccorcos ccorcos closed this Mar 24, 2015
@mitar
Copy link
Collaborator

@mitar mitar commented Mar 24, 2015

I would just need to keep track of which subscription to invalidate each time.

I think that with new Meteor version you have subscription ID with each handle.

@mitar
Copy link
Collaborator

@mitar mitar commented Mar 24, 2015

@mitar, I give a good explanation of why I'm making my own subscription caching package.

Have you tried sending a pull request? All things listed there look to me as a good candidate for a pull request. Instead of having one more package to choose from.

@ccorcos
Copy link
Author

@ccorcos ccorcos commented Mar 24, 2015

I tried fixing subs-manager for these things, but it was too hard. Some of these problems are fundamental to the way the package was built. It would require a complete overhaul. So I figured I was better of just writing a new package from scratch...

@stubailo stubailo reopened this Apr 22, 2015
@glasser glasser closed this in 91ce156 Apr 29, 2015
@glasser
Copy link
Member

@glasser glasser commented Apr 29, 2015

Implemented.

@ccorcos
Copy link
Author

@ccorcos ccorcos commented Apr 29, 2015

yay!

@stubailo
Copy link
Contributor

@stubailo stubailo commented Apr 29, 2015

Wow, that's awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants