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

BUG: Stop template-bound computations on route change #96

Closed
boustanihani opened this issue Apr 27, 2015 · 6 comments
Closed

BUG: Stop template-bound computations on route change #96

boustanihani opened this issue Apr 27, 2015 · 6 comments
Assignees

Comments

@boustanihani
Copy link

In my app, when a user has 2 open browser-tabs, and he logs out in one of them, he will automatically get logged-out (and redirected to "/") in the other. I have a Tracker.autorun() function that checks when Meteor.user() becomes falsy and redirects using FlowRouter.go("/") if the user happens to be on a "requireLogged" page.

In my profile-template I have a helper that returns Meteor.user().emails.pop().address. Now when Meteor.logout() succeeds my Tracker.autorun() function is rerun first and calls FlowRouter.go("/") which finally causes FlowLayout.render(...). Everything is fine, but my only problem is that the current template helpers are also being called (one last time) before route change happens resulting in a "NullPointerException".

I know I can avoid this by doing simple checks but IMO, FlowRouter.go("/") (or maybe `FlowLayout.render()``) should prevent all currently rendered templates from updating until route change completes. I think this is "natural behavior" because there is no need to update any templates that will most probably get destroyed when a route change is just about to happen. What do you think?

@arunoda
Copy link
Contributor

arunoda commented Apr 27, 2015

I this is not something we can track. Meteor.user() updated by meteor. So,
it will invalidate all the places it tracks.

That includes both of your places. So, we can't sure which will run first.

I think #96 will have something do with this, if router.go() called before
the helper.

Anyway, you need to do checks.
On 2015 අප්‍රේල් 27, සඳුදා at ප.ව. 7.13 boustanihani <
notifications@github.com> wrote:

In my app, when a user has 2 open browser-tabs, and he logs out in one of
them, he will automatically get logged-out (and redirected to "/") in the
other. I have a Tracker.autorun() function that checks when Meteor.user()
becomes falsy and redirects using FlowRouter.go("/") if the user happens
to be on a "requireLogged" page.

In my profile-template I have a helper that returns
Meteor.user().emails.pop().address. Now when Meteor.logout() succeeds my
Tracker.autorun() function is rerun first and calls FlowRouter.go("/")
which finally causes FlowLayout.render(...). Everything is fine, but my
only problem is that the current template helpers are also being called (one
last time
) before route change happens resulting in a
"NullPointerException".

I know I can avoid this by doing simple checks but IMO, FlowRouter.go("/")
(or maybe FlowLayout.render()`) should prevent all currently rendered
templates from updating until route change completes. I think this is
"natural behavior" because there is no need to update any templates that
will most probably get destroyed when a route change is just about to
happen
. What do you think?


Reply to this email directly or view it on GitHub
#96.

@boustanihani
Copy link
Author

@arunoda I suppose that when we have several autoruns and something changes that affects all of them then they will get rerun in the same order they where started. I agree that in my issue I have to make sure that FlowRouter.go() gets run before the helper and this is also the case (I checked this), but don't you think that the helpers should stop executing until route change completes? I mean why update a template which is about to get destroyed (after route-change) ?

@boustanihani
Copy link
Author

Please allow me to explain my issue in further details.

When writing a template that displays user information, we assume that a user is already logged and that he won't be able to navigate to this route unless he was logged. So actually, there shouldn't be a need to check if Meteor.user() exists inside each helper function.

Now the other way around, suppose we are already on a route displaying user information and the user was logged out from another browser-tab (inside the same session) or maybe from the server, then IMO there should be a way to attach a run-first hook to the routing middleware that checks if a route-change is needed while stopping all computations that would naturally get stopped when a template is destroyed. I mean stopping all this.autorun() computations started inside onCreated(), onRendered() and also preventing all helper functions from updating the dom.

So, if in our "run-first" hook function, we decide that a route-change should happen (destroying some currently rendered templates), then this should happen immediately. (Again: Why update templates that are about to get destroyed?)

IMO routing-middleware should also control when templates stop updating because they are no longer needed and are about to get destroyed, so allowing a so-called "run-first" autorun hook would allow cleaner coding and also improve performance.

@boustanihani boustanihani changed the title Stop template-helper computations on route change Stop template-bound computations on route change Apr 27, 2015
@arunoda
Copy link
Contributor

arunoda commented May 18, 2015

Yes. This is an issue.
I'll work on this.

@arunoda arunoda added the bug label May 18, 2015
@arunoda
Copy link
Contributor

arunoda commented May 18, 2015

related to #103 as well.

@arunoda arunoda added doing and removed bug labels May 24, 2015
@arunoda arunoda changed the title Stop template-bound computations on route change BUG: Stop template-bound computations on route change May 24, 2015
@arunoda arunoda self-assigned this May 24, 2015
@arunoda
Copy link
Contributor

arunoda commented May 24, 2015

Released version 1.8.0 with fixes for this bug.

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

2 participants