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

Flowrouter.go() vs. {...}.redirect() #100

Closed
Streemo opened this issue May 1, 2015 · 8 comments
Closed

Flowrouter.go() vs. {...}.redirect() #100

Streemo opened this issue May 1, 2015 · 8 comments

Comments

@Streemo
Copy link

Streemo commented May 1, 2015

I noticed something when trying the former solution.

FlowRouter.go(pathA) will not run pathA's middleware if I'm already at pathA. However, if I use Flowrouter.redirect(pathA), the route will be run fully with middleware (like expected) regardless of what path I'm currently at.

Is this intended behavior?

Since FR is non-reactive by design, a common pattern is to rerun a route in light of new information. This seems to go against the design of FR (because that's pretty much what Iron Router does) so I am wondering whether or not its a better idea to render a different template instead of rerouting.

USE CASE: I have loginMiddleware which is very basic and renders a loginTemplate if the user isn't logged in. Once the user signs in from this view, I want the route to be re-run in light of the fact that she is now signed in. In the logging in callback function, I FlowRouter.redirect(currentPath) and all is well. Instead I'm thinking it might be a better idea to FlowLayout.render('layout',ifLoggedInTemplates) in the callback. Thoughts?

Another solution is to implement a separate sign-in route which is hit in the loginMiddleware if not signed in, and keep n=1 route history, and then just redirect back to the previous page after the user signs in.

@arunoda
Copy link
Contributor

arunoda commented May 18, 2015

We are doing some checks on FlowRouter.go to stop running route if called multiple times. Which means routes are idempotent. I like that idea. We need to do it for .redirect as well.

But, I'm still quite not sure this is the right way or not. May be I need to trust my gut feeling.

Handling login related stuff is pretty messy now. We need to find a way for that. I'll work on that, once I fixed major bugs.

@arunoda
Copy link
Contributor

arunoda commented May 18, 2015

I think I'll keep both .redirect() and .go() idempotent. Then we can have .reload() api.
reference: #83

@Streemo
Copy link
Author

Streemo commented May 18, 2015

@arunoda I think the idempotency is fine, but if you make both .go() and
.redirect() this way then I agree that there should be a .reload()
functionality which is not idempotent.

+1 for that.

Question, what is the difference between redirect and go (assuming both are
idempotent)? It seems I can use them interchangeably (minus the fact that
one of them currently is not idempotent)
On May 18, 2015 1:10 AM, "Arunoda Susiripala" notifications@github.com
wrote:

I think I'll keep both .redirect() and .go() idempotent. Then we can have
.reload() api.


Reply to this email directly or view it on GitHub
#100 (comment)
.

@arunoda
Copy link
Contributor

arunoda commented May 18, 2015

FlowRouter.redirect() can be used inside middlewares to redirect the route. Then it gives us clean history. see: https://visionmedia.github.io/page.js/

But, we need to complete this API a bit and add more docs.

@Streemo
Copy link
Author

Streemo commented May 18, 2015

I see, that clears things up. Thanks arunoda.
On May 18, 2015 1:24 AM, "Arunoda Susiripala" notifications@github.com
wrote:

FlowRouter.redirect() can be used inside middlewares to redirect the
route. Then it gives us clean history. see:
https://visionmedia.github.io/page.js/

But, we need to complete this API a bit and add more docs.


Reply to this email directly or view it on GitHub
#100 (comment)
.

@Streemo
Copy link
Author

Streemo commented May 18, 2015

@arunoda currently I (and I'm sure others) are relying on at least one of
these operations being repeatable (non-idempotent). If you decide to turn
both redirect and go into idempotent operations, it would be much
appreciated if first the reload() api is determined.

Thanks!
On May 18, 2015 11:21 AM, "Jay P" japatel99@gmail.com wrote:

I see, that clears things up. Thanks arunoda.
On May 18, 2015 1:24 AM, "Arunoda Susiripala" notifications@github.com
wrote:

FlowRouter.redirect() can be used inside middlewares to redirect the
route. Then it gives us clean history. see:
https://visionmedia.github.io/page.js/

But, we need to complete this API a bit and add more docs.


Reply to this email directly or view it on GitHub
#100 (comment)
.

@arunoda
Copy link
Contributor

arunoda commented May 23, 2015

I'm closing this favour of the reload API.

@arunoda arunoda closed this as completed May 23, 2015
@arunoda arunoda removed the question label May 23, 2015
@niklasp
Copy link

niklasp commented Jul 21, 2015

can you share your loginMiddleware

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

3 participants