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

New triggers implementation with redirect support #172

Merged
merged 11 commits into from
Jun 21, 2015
Merged

Conversation

arunoda
Copy link
Contributor

@arunoda 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
We don't support older browsers
But at least, this allow us to run tests inside Phantom
@arunoda
Copy link
Contributor Author

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();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

That's weird. But possible.

@delgermurun
Copy link
Contributor

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

@arunoda
Copy link
Contributor Author

arunoda commented Jun 21, 2015

@delgermurun I mean if there are any issues :)

@arunoda
Copy link
Contributor Author

arunoda commented Jun 21, 2015

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

@delgermurun
Copy link
Contributor

Yeah. Do it.

@arunoda arunoda merged commit ced2b63 into master Jun 21, 2015
@arunoda arunoda removed the doing label Jun 21, 2015
@cstrat
Copy link

cstrat commented Jul 4, 2015

This is perfect, thanks Aronuda 👍

@derwaldgeist
Copy link

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants