search

studiointeract / tracker-component

Easy reactive React Components with Meteor and Tracker
MIT License
37 stars 4 forks source link

Update index.jsx #13

Open Elviron opened 8 years ago

Elviron commented 8 years ago

Save the subscriptions based on only the name of the subscription. So you don't save multiple subscriptions of the same name.

maxnowack commented 8 years ago

With this, it isn't possible to have multiple subscriptions to the same publication in the same component. I think it's better to wrap the subscribe call in a autorun, since subscription inside autoruns will automatically stopped if the autorun will rerun.

timbrandin commented 8 years ago

Great idea @maxnowack, that would remove the need to track the subscriptions on each component, and reduce the load on GC too. So the implementation would rather be:

this.autorun(() => this.__subscribe.apply(this, [name, ...options]))

Rather than the current lookup and removal. What do you think @Elviron ?

maxnowack commented 8 years ago

@timbrandin I don't think that it is a good idea to wrap every subscribe in an autorun. If you take a look at the implementation in blaze, you'll see that the subscription handle will be saved as a reference and the subscription will be stopped if the view gets destroyed (Tracker.Component does something similar with tracker computations). In my opinion, this is the most flexible way. I could provide a PR for this.

PS: I don't get why you stringify the arguments of the subscriptions to store the reference. Is there a reason for this?