arillo/meteor-flow-router-helpers

subsReady should be updated or clarified

Closed this issue · 8 comments

Your subsReady I believe is checking if the subscription in FlowRouter.myRoute.subscriptions are ready? This usage is not recommended so I self.subscribe() in an autorun in my template onCreated. Shouldn't your subsReady check these? I've just used Template.subscriptionsReady for now, which works, but might save some folks a few minutes if you clarified that?

@lynchem You are completely right with the template level subscription approach. I have been trying out some stuff and could offer following solution for the subsReady helper:

subsReady = (subs...) ->
  return FlowRouter.subsReady() and Template.instance().subscriptionsReady() if subs.length is 1
  subs = subs.slice(0, subs.length - 1)
  _.reduce subs, (memo, sub) ->
    memo and FlowRouter.subsReady(sub)
  , true

Since template level subscription are not named in difference to the flow-router ones, it will only check for them to be ready if subsReady is used without parameters. I will update the Readme like this:

Usage subsReady

If you call subsReady without parameters it will check for all flow-router subscriptions to be ready and also for all template level subscriptions to be ready.

If you pass parameters it will only check for the specific flow-router subscriptions to be ready. The parameters are the subscription names you used when registering them on FlowRouter, like:

FlowRouter.route('/posts', {
    subscriptions: function(params, queryParams) {
        this.register('posts', Meteor.subscribe('posts'));
        this.register('items', Meteor.subscribe('items'));
    }
});
{{#if subsReady 'items' 'posts'}}
  <ul>
  {{#each items}}
    <li>{{title}}</li>
  {{/each}}
  </ul>
  <ul>
  {{#each posts}}
    <li>{{title}}</li>
  {{/each}}
  </ul>
{{/if}}

What do you think? Can you think of any problems this could bring on current implementations?

Hey banglashi, that looks great. My only worry would be that maybe some people using it are depending on it only checking router subs but I think this would definitely be more in line with what you would expect. Thanks.

zimme commented

FWIW, I feel like template level subscription helper belongs in it's own package, because this one is specifically for flow-router helpers.

Unless there are flow-router template level subscriptions?

zimme commented

Template.subscriptionsReady is already a helper for template level subscriptions too.
http://docs.meteor.com/#/full/Blaze-TemplateInstance-subscribe

Yeah @zimme, that's what I'm using. My point was mainly that it wasn't clear in the documentation which subs were ready (at least to me). I just looked to use the subsReady in this package first as I was using the pathFor part of it. Possibly the easiest would be to clarify the documentation and leave it at that.

zimme commented

Making sure the documentation is obvious is the first choice, then depending on the need of the users of this package, we can move on from there. That's at least what I think =)

I did an update on the docs to reflect that template level subscriptions are not considered. Published version 0.4.7.

zimme commented

Just a note, you can do meteor publish --update if you only wanna update the meta data/documentation on atmospherejs.com =)