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

New triggers implementation with redirect support #172

Merged
merged 11 commits into from Jun 21, 2015

Conversation

Projects
None yet
4 participants
@arunoda
Contributor

arunoda commented Jun 19, 2015

This is a re-implementation with our triggers API with redirect support with a clean codebase.

This is the implementation of the redirect API.

FlowRouter.route('/', {
  triggersEnter: [function(context, redirect) {
    redirect('/some-other-path');
  }],
  action: function(_params) {
    throw new Error("this should not get called");
  }
});

When someone route to / then he'll be redirected to /some-other-path. In this case, action won't get called or any exit trigger if exists.

This can be done for exit triggers as well.

We can also do this prevent route changes like this.

FlowRouter.route('/', {
  triggersExit: [function(context, redirect) {
   // may be in exit handlers, we should accept to redirect without an URL.
   // If so, it will simply stop. But, this should be not be in the enter triggers
    redirect(context.path);
  }],
});

In the above case, it is impossible to leave from route /.

Here are some properties of redirect

  • redirect must have an URL
  • redirect must be called within the same eventloop cycle (no async or inside a Tracker)
  • redirect cannot be called multiple times

All these have been added to make sure you are not doing any reactive code inside the Router.

@arunoda arunoda added the doing label Jun 19, 2015

arunoda added some commits Jun 19, 2015

Use _.bind for backward compatibility
We don't support older browsers
But at least, this allow us to run tests inside Phantom
@arunoda

This comment has been minimized.

Contributor

arunoda commented Jun 19, 2015

@delgermurun check this. Give it a try and help me to figure out any issues?

}
// we need to trigger the above invalidations immediately
// otherwise, we need to face some issues with route context swapping
Tracker.flush();

This comment has been minimized.

@arunoda

arunoda Jun 20, 2015

Contributor

Check how flush behave if a user change the route from a template helper.

That's weird. But possible.

@delgermurun

This comment has been minimized.

Contributor

delgermurun commented Jun 21, 2015

@arunoda Nice refactoring! Working good. What's the issues :)?

@arunoda

This comment has been minimized.

Contributor

arunoda commented Jun 21, 2015

@delgermurun I mean if there are any issues :)

@arunoda

This comment has been minimized.

Contributor

arunoda commented Jun 21, 2015

Everything looks good. I think we can do a release.

@delgermurun

This comment has been minimized.

Contributor

delgermurun commented Jun 21, 2015

Yeah. Do it.

@arunoda arunoda merged commit ced2b63 into master Jun 21, 2015

1 check passed

continuous-integration/travis-ci/push The Travis CI build passed
Details

@arunoda arunoda removed the doing label Jun 21, 2015

@cstrat

This comment has been minimized.

cstrat commented Jul 4, 2015

This is perfect, thanks Aronuda 👍

@derwaldgeist

This comment has been minimized.

derwaldgeist commented Jul 28, 2015

I'm trying to redirect to a sub-route of the current route, for instance from /foo to /foo/bar. This works (shows the right template for the router), but the URL ifself is not updated (still shows /foo). The same applies to my navigation bar which is testing for the route to highlight the right navigation elements. If I'm using FlowRouter.go() instead of redirect(), the same happens. How can I update the URL and my navigation template once a redirect happens?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment