search

iron-meteor / iron-router

A client and server side router designed specifically for Meteor.
MIT License
1.98k stars 412 forks source link

Blaze redirects #469

Closed davidworkman9 closed 10 years ago

davidworkman9 commented 10 years ago

Is this.redirect('<route>'); supposed to work with blaze-rc0? I'm getting a warning of:

You called this.stop() inside a hook or your action function but you should use pause() now instead.

And my redirects don't work. Seems I'm not the only one running into this

cmather commented 10 years ago

Oops I may have not changed the redirect method yet. Just use Router.go as it will work fine now. I will change over this.redirect sometime next week or a PR is welcome. Controllers are now automatically stopped if you change routes in a hook. You can pause the current run by calling the pause method which is passed as a parameter to your hooks and action functions.

On Mar 1, 2014, at 6:54 PM, Dave Workman notifications@github.com wrote:

Is this.redirect(''); supposed to work with blaze-rc0? I'm getting a warning of:

You called this.stop() inside a hook or your action function but you should use pause() now instead.

And my redirects don't work. Seems I'm not the only one running into this

— Reply to this email directly or view it on GitHub.

darkship commented 10 years ago

It didn't work with Router.go() when I tried in Router.configure.before or in other hooks. It just worked when you use it else where like in a click event.

And when I tried to refresh a page that redirect to itself with parameters, it breaks with an error

davidworkman9 commented 10 years ago

I don't know if I have enough knowledge to fix it and submit a PR. I'll give it a whirl though. @darkship, if you put the Router.go call into the next turn of the event loop it works, (i.e. setTimeout(function () { Router.go('<route>'); }, 0);).

cmather commented 10 years ago

This seems like it might be a bug. I'll be looking into it today and will report back.

darkship commented 10 years ago

I found where my error by refreshing comes from and it's not from Iron-router. A subscribe handler http://docs.meteor.com/#meteor_subscribe raised an error in Deps since I switched my project to Blaze.

cmather commented 10 years ago

Oh okay. Interesting. We should tidy up iron-router behavior on errors thrown by the application.

davidworkman9 commented 10 years ago

I don't think this should be closed, the original issue was about redirects which isn't fixed yet.

cmather commented 10 years ago

Oh sorry that was an accident. I'm looking into the redirect issue today.

davidworkman9 commented 10 years ago

Hope you didn't get too far into it. I think this PR solves the issue. Looks like this.stop was being called internally when you call this.redirect. I assumed the proper fix is to not, and just invoke run immediately.

cmather commented 10 years ago

@davidworkman9, Thanks for that PR! It gave me the right place to start so thanks for that! The thing it was missing is that we actually do need to stop the previous controller from running indefinitely. Otherwise, its computations will never stop running (functions will just keep rerunning any time a dependent data source changes). Try my latest commit and let me know if you guys agree that this fixes the issue.

davidworkman9 commented 10 years ago

Should probably have posted this here:

The redirect occurs now, at least according to the browser address bar. The template doesn't change. Here is a repo I setup just for testing this problem

darkship commented 10 years ago

@cmather : it works, thanks

davidworkman9 commented 10 years ago

Thanks a ton Chris!