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

Question: does waitOn actually wait for subscription to be ready? #265

Closed
nleush opened this issue Oct 31, 2013 · 29 comments
Closed

Question: does waitOn actually wait for subscription to be ready? #265

nleush opened this issue Oct 31, 2013 · 29 comments

Comments

@nleush
Copy link

nleush commented Oct 31, 2013

I'm expecting waitOn will wait for subscription.ready() === true and then start getting data and render template. But in this example:

https://github.com/nleush/iron-router-wait-test/blob/master/iron-router-wait-test.js#L22

Console output:

false
true

That means data and render runs first time when subscription not ready yet.

@copleykj
Copy link

@cmather can probably give a good firm answer on this, but for now if you wrap the code in your call backs in an if(this.ready()) it will fix this issue for now.

    Router.map(function () {
        this.route('hello', {
            path: '/',
            data: function() {
                if(this.ready()){
                    console.log(s.ready());
                }
            },
            waitOn: s
        });
    });

@yubozhao
Copy link

Hi @nleush @copleykj

Yes, waitOn will wait the subscription to be ready.

Right now, as 0.6 version, You could do something like this as you were with waitOn. I found this pattern a lot easier to reason about.

this.route('postShow', {
  path: '/posts/:slug',
  //XXX before could be in form of array
  before: function () {
     // Use this.subscribe to subscribe whatever subscription you want for this route.  If you want to wait for the subscription to be ready, then use .wait() method.
     this.subscribe('post', this.params.slug).wait();
     this.subscribe('all-posts');
  },
  data: function () {
    return Posts.findOne({
       slug: this.params.slug
    });
  }
}

@yubozhao
Copy link

before could be array of functions.

@bryanaka
Copy link

The above didn't work for me, it just stays loading forever.

router.js

this.route('showTravel', {
  path: '/travel/:_id',
  before: function() {
    this.subscribe('singleTravelRequest', this.params._id).wait();
  },
  data: function() {
    return TravelRequests.findOne({_id: this.params._id});
  }
});

publish.js

Meteor.publish('singleTravelRequest', function(id) {
  return TravelRequests.findOne(id);
});

I echoed this problem using controllers in #261, #38, and in this gist - https://gist.github.com/bryanaka/7253198

@cmather
Copy link
Contributor

cmather commented Oct 31, 2013

In your publish function return a cursor (find instead of findOne). This has the side effect of sending a ready ddp message which is what signals to the client that the subscription is ready.

When you call wait() on a subscription handle it just adds the handle to a list. When you ask if the controller instance is ready by calling if (this.ready()) {...} it loops through the handles and returns true if all of the handles' ready methods return true.

This works because the ready methods of subscription handles are reactive. If the status changes, the computation will be invalidated and rerun.

@cmather cmather closed this as completed Nov 1, 2013
@nleush
Copy link
Author

nleush commented Nov 1, 2013

@yubozhao if using internal router subscription:

this.subscribe('post', this.params.slug).wait();

then how to unsubscribe from it when data is no longer needed?

And also, if "waitOn will wait the subscription" why it calls data before subscription is ready?

@yubozhao
Copy link

yubozhao commented Nov 1, 2013

@nleush
Iron router just wrap around Meteor.subscribe
this.subscribe('post') equals Meteor.subscribe('post')

When you call the wait method, this.subscribe('post').wait() It acts as the router will wait on this subscription to be ready then go on its business.

As far as unsubscribe, Meteor is taking care of that. You dont really need to do anything. And if you set some sorta of Session variable that you want to clean out, you can always use the unload hook

this.route('post', {
  before: [/* some functions you want to run in before hook*/],
  data: function () {
    /* find data and return them here */
  },
  unload: function () {
    /* anything you want to do before it went on to different route. */
   // for example
   Session.set('postId', null);  // clean up Session variables
  }
});

It calls data before the wait() subscription is ready? Could you give me an example to look further?

@nleush
Copy link
Author

nleush commented Nov 1, 2013

It calls data before the wait() subscription is ready? Could you give me an example to look further?

Maybe I'm doing something wrong but here it is:
https://github.com/nleush/iron-router-wait-test/blob/master/iron-router-wait-test.js#L27

Router doesn't wait for .wait() nor waitOn.

@nleush
Copy link
Author

nleush commented Nov 1, 2013

@yubozhao how does Meteor actually know that subscription should be unsubscribed?

@th0r
Copy link

th0r commented Nov 1, 2013

Router doesn't wait for .wait() nor waitOn.

That's because you didn't set loadingTemplate. If you will, IronRouter will render it, wait for all the subscriptions to be ready and only after this will call data function.

@yubozhao
Copy link

yubozhao commented Nov 1, 2013

@nleush
too tired, going to sleep. Here is the quote from doc.meteor

If you call Meteor.subscribe within a reactive computation, for example using Deps.autorun, the subscription will automatically be cancelled when the computation is invalidated or stopped; it's 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.

You probably have to look into Meteor source to found now more about it. It would be in the packages/livedata/livedata_connection.js file, I think.

@yubozhao
Copy link

yubozhao commented Nov 1, 2013

@nleush
To find out more about how iron router run function works You can going into the source, lib/client/route_controller.js

@nleush
Copy link
Author

nleush commented Nov 1, 2013

@th0r it works! thanks :)

@yubozhao thanks!

@hellogerard
Copy link

So... there was a lot of discussion on this, but the original issue (which I also witness) wasn't addressed. If I use the latest version of iron-router today, data is called twice: once, right away, and again after the waitOn subscription is ready. Couple observations:

  • If you add a loadingTemplate, this problem goes away. So adding a loadingTemplate is a solution.
  • This wasn't always like this. Earlier versions of iron-router only ran data after the waitOn was ready, regardless of whether or not loadingTemplate was set.

Is this the intended logic? Did something change?

@cmather
Copy link
Contributor

cmather commented Nov 1, 2013

For the previous question about how subscriptions get stopped automatically check out this video:
https://www.eventedmind.com/feed/GWNPanQChaM7LKKEt.

@hellogerard, This behavior was changed a bit. In a previous version (on dev) the entire route would not run until the controller was in a ready state. This was changed to allow individual functions in the route (before hooks, action function, after functions) to decide what to do based on the ready state. I just checked and realized the setDataHook does not check for the ready state and it probably should. I'm also open to other suggestions for the right flow here.

@nleush
Copy link
Author

nleush commented Nov 2, 2013

@cmather thanks for video! Very clear explanation.

So as I understand, iron-router calls hooks and run method inside Deps.atutorun which computation is stopped after route unload. So subscription from those method will be stopped too. Thats awsome! :)

@lencinhaus
Copy link

@cmather i'm having this problem too, and i think that running data hooks without waiting for subscriptions to be ready kinda defeats the whole purpose of the wait/waitOn mechanism. So I strongly vote for checking the ready state in setDataHooks, if it means that data and action hooks won't be called until the subscriptionsdare ready, even if loadingTemplate is not set (just leave the currently renderd templates until the new ones are ready to be rendered). Otherwise, this loadingTemplate dependency should be at least documented in the readme.

@hellogerard
Copy link

@cmather Thanks for the clarification. I'm open to any flow, as long as it's documented.

@petrometro
Copy link

+1 for checking the ready state, or at least adding this to the available options on RouteController with default set to false.

@hellogerard
Copy link

So if anyone reads this, I spent some time understanding the current logic around this issue. I think I have a handle on them.

Note that where ever I say "block" below, I mean the route is stopped, but since it's reactive, it gets re-run once the data is ready.

Pre 0.6.x, iron router would block the entire render function until the data in waitOn was ready. After 0.6.x, as mentioned above, that logic was taken away in favor of making routes more flexible.

This behavior was changed a bit. In a previous version (on dev) the entire route would not run until the controller was in a ready state. This was changed to allow individual functions in the route (before hooks, action function, after functions) to decide what to do based on the ready state.

So in the current iron router, if you have a waitOn function and a data function, it's entirely probable that your template will render before the data is ready, and you will get undefined or null errors. You have to work in logic around this.ready() in your data function or before hook yourself (see Quick Start example in the README).

One thing tripping me up: autoRenderLoadingTemplateHook function DOES call this.stop() until data is ready IF you set a loadingTemplate (https://github.com/EventedMind/iron-router/blob/master/lib/client/route_controller.js#L66). This is why things that didn't work suddenly started working just by adding a loadingTemplate. autoRenderLoadingTemplateHook blocked until this.ready() was true.

I was having issues around undefined errors, and having just our header/footer showing up followed by a delay before the main content suddenly showed up. This stuff helped me to resolve these issues.

Hopefully this helps someone. You'll need to set up iron router differently depending on if you want a loading indicator vs holding up the whole screen waiting for data vs etc.

@cmather
Copy link
Contributor

cmather commented Nov 19, 2013

Hey @hellogerard, That's a pretty good summary of what's happening. I think what's needed in the short term is a check on the default action and on the setData hook to make sure everything is ready. I think those checks are currently missing. But the loadingTemplateHook checks the ready state to decide whether to show the loading template. Pull requests definitely welcome to update the hook behavior (lib/client/route_controller.js).

@hellogerard
Copy link

Thanks @cmather. I'd love to take a crack at a PR, but, like you, I'm not sure what the best design here is. Need a balance between easy, out-of-the-box behavior, and flexibility to really customize route behavior. I'll think on it. Thanks for all your work.

@ariemer
Copy link
Contributor

ariemer commented Nov 23, 2013

With help from @th0r and @hellogerard, solved with the following approach. See my comment on #287 for background.

Use only before, data, and action. yieldTemplates and waitOn replaced with custom logic in before, data, and action. This probably isn't essential.

Pass to before an array of functions; the first contains all subscriptions with .wait(). Any further functions within before are wrapped in if(this.ready()) {} . These functions contain desired actions for before hooks, like so:

before: [
        function() {
            this.subscribe('post', this.params._id).wait();
            this.subscribe('rooms').wait();
        },
        function () {
            if ( this.ready() ) {
                room = Posts.findOne({_id: this.params._id}).room;
                Session.set('currentRoom', room);
            };
        }
    ], // end of before

Wrap entire content of action and data in if (this.ready()) {} . I think this basically undoes the change to 0.6.x described by @cmather and @hellogerard.

    data: function () {
        if ( this.ready() ) {
            var result = Posts.findOne({_id: this.params._id});
            return(result);
        };
    },
    action: function () {
        if( this.ready() ) {
            this.render();
            this.render('singlePost', {to: 'postsYield'});
        }
        else {
            this.render('loading', {to: 'layout'})
        };
    },

I had some final bugs caused by urls to assets required by these templates. Check to be sure the urls still make sense within the path you've given the router for a particular view.

@DenisGorbachev
Copy link

The "must have" requirement for loadingTemplate doesn't make any sense, it should just stop() regardless of loadingTemplate presence.

@mrmowgli
Copy link

If the pattern is to wrap every data block in a this.ready() check, why not just have IronRouter do the check first? It seems silly to call the data hook before the route is actually ready.

@yubozhao
Copy link

Hi @mrmowgli,

I don't think wait on data is necessary for every route. For some page, I am not waiting for the data to be ready to load up the page. So, I think this design give me a lot of flexibility of how should pages behave.

@mrmowgli
Copy link

@yubozhao , I understand, but in that case why not use an before hook? Or if its so important for the hook to be called before the collections from waitOn are actually ready, why couldn't Iron Router have a final_data hook or something similar instead?

@th0r
Copy link

th0r commented Feb 17, 2014

IMHO if user calls wait() on subscription, he means, that he really wants to wait for it before generating route's data and invoking action method. If he doesn't call wait, then he don't want us to wait for it.
What do you think about it?

@mhilland
Copy link

After encountering this issue firsthand, I was able to get it working by experimenting with some of the recommended solutions above, but moreso by reading up on the new amendments to the Iron Router documentation section called Waiting on Subscriptions, found here: https://github.com/EventedMind/iron-router/blob/dev/DOCS.md#waiting-on-subscriptions-waiton

Firstly, it is important to note that the before function mentioned in the above comments has been deprecated in place of onBeforeAction. While it is possible to attach the wait() function to any subscription calls in the onBeforeAction function, after running some checks in my console I noticed there's a slight change in the flow of logic between onBeforeAction and waitOn. Whereas the onBeforeAction subscriptions call the data function twice (the first time passing no data), using the waitOn function only causes the data function to be called once, data included. This means that your render() function called inside of your action function is all that needs to be wrapped with an if (this.ready()), and will in turn pass any given data to your template. Thus, if you're relying on any conditional logic with regards to your data, I recommend using the waitOn function. Also note that you need to return the subscription with waitOn rather than directly subscribe to it.

  waitOn: function () {
    return Meteor.subscribe('campaigns_edit', this.params.uuid);
  },

  data: function () {
    return Campaigns.findOne({uuid: this.params.uuid});
  },

  action: function () {
    if (this.ready()) {
      this.render();
    }
  }

Thanks for all your help!

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

No branches or pull requests