FlowRouter.go() should stop unneeded computations

[boustanihani]

I am reopenning issue #96 because it seems to be still existing...

To reproduce, click on SignIn, then swith to the User page and finally SignOut, you should get this on your console:

Redirecting "/user" to "/"

Exception in template helper: TypeError: Cannot read property 'username' of null
    at Object.Template.user.helpers.username (reactive-flowing.js:28:33)

After calling FlowRouter.go() the templates that are about to get destroyed should no longer update. (review discussion in #96 for further details)

Repo: https://github.com/boustanihani/reactive-flowing
Deployment: http://reactive-flowing.meteor.com

And here is my code:

<template name="layout1">
    <div style="text-align:center;padding:1em;">
        <div>
            <a href="/">Home</a>&nbsp;
            <a href="/user">User</a>&nbsp;
            {{#if currentUser}}
                <a href="#" id="signOut">SignOut [{{currentUser.username}}]</a>
            {{else}}
                <a href="#" id="signIn">SignIn</a>
            {{/if}}
        </div>
        <hr>
        <div>
            {{> Template.dynamic template=main}}
        </div>
    </div>
</template>

<template name="home">
    <h1>Home</h1>
</template>

<template name="user">
    <h1>User: {{username}}</h1>
</template>

if (Meteor.isServer) {

    var testUser = Meteor.users.findOne({
        username: 'test'
    });

    console.log('testUser:', testUser);

    if (!testUser) Accounts.createUser({
        username: 'test',
        password: '123456'
    });
}

if (Meteor.isClient) {

    Template.layout1.events({
        'click #signIn': function() {
            Meteor.loginWithPassword('test', '123456');
        },
        'click #signOut': function() {
            Meteor.logout();
        },
    });

    Template.user.helpers({
        username: function() {
            return Meteor.user().username;
        }
    });

    var requireLogged = [
        '/user',
    ];

    Tracker.autorun(function() {

        FlowRouter.watchPathChange();

        var route = FlowRouter.current();

        if (!route.path) return;

        var section = '/' + route.path.split('/')[1];

        if (requireLogged.indexOf(section) != -1) {
            if (!Meteor.user()) var redirect = '/';
        }

        if (redirect) {
            console.log('Redirecting "' + section + '" to "' + redirect + '"');
            FlowRouter.go(redirect);
        }
    });

    FlowRouter.route('/', {
        action: function(params, queryParams) {
            BlazeLayout.render('layout1', {
                main: "home"
            });
        },
    });

    FlowRouter.route('/user', {
        action: function(params, queryParams) {
            BlazeLayout.render('layout1', {
                main: "user"
            });
        },
    });
}

[arunoda]

This is not something we can track. In this case, logout happens whithin a layout. So, router has no control over that.

Here you are watching the user() via also a tracker and do a redirect. So, template helper could fire before the redirect tracker. That's not we can control.

[boustanihani]

But shouldn't the user template get destroyed immediately? I mean FlowRouter.go(redirect) is getting called before the template (to be destroyed) tries to update...

[arunoda]

Currently, even if we it gets calls before that, there is a global tracker inside where FlowRouter.go() pass an redirect action. It waits until all other computations end.
(We have reasons to do that, specially with Router level subs registration)

It's best to do something like this:

{{#if currentUser}}
  <div>
    {{> Template.dynamic template=main}}
  </div>
{{/if}}

We'll can do immediate route change when we remove subs logic from the router. I'm having some ideas. Let's see.

I mark this as a bug, until we found a built-in solution.

[boustanihani]

Thanks, I just hope FlowRouter/BlazeLayout would offer some way to interrupt/hook into template specific computations (helpers, subscriptions, autoruns etc...) and allow stopping them immediatly if needed...

This would then eliminate the need for traditional "if-checks" like in the workaround you mentioned and result in cleaner code

[boustanihani]

There is another small issue I noticed, if you open the page http://reactive-flowing.meteor.com/ and you are still not signed in, try clicking directly on "User", the user page will not show and you will be redirected to "Home" but the browser path will not be updated accordingly...

I tried calling FlowRouter.go(redirect) inside `Tracker.afterFlush()``but this also die not help solve the problem...

[AnthonyAstige]

I've hacked together a full white screen w/spinner between routes where I can add one line to all of my slow to render templates.

Template code

{{{showBetween}}}

Router code

var between = new ReactiveVar(false);
// Control entirely from templates
UI.registerHelper('showBetween', function() {
       // After rendered we're not between anymore
       Template.instance().view._callbacks.rendered.push(function() {
               between.set(false);
       });

       // Don't show between if not between
       if (!between.get()) { return ''; }

       // Let user know between pages
       return '<div style="' +
                               'z-index: 9999;' +
                               'background: white;' +
                               'position:fixed;' +
                               'top: 0;' +
                               'left: 0;' +
                               'height: 100%;' +
                               'width:100%' +
                       '">' +
                               '<img src="/img/ajax-loader.gif">' +
                       '</div>';
});

...

AppRouter.authed = FlowRouter.group({
        name: 'authed',
        triggersEnter: [function(context, redirect) {
               between.set(true);

               ...

        }

...

});

Other thoughts & caveats

This could easily be extended to prevent rendering at all between routes, but the only way I saw to do that was with an {{#if ...}}....{{if}} wrapper in every one of my templates, and I preferred cleaner template code.

My code is using _callbacks which I don't like. I tried using Template[name] by grabbing the name from the instance but that didn't work.

This code potentially creates multiple onRendered callbacks, but it would mostly be a minor optimization to prevent that.