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

[Discussion] Implement 'pass' function to skip to next matching (middleware) route #854

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

hellogerard
Copy link

New feature: a route can tell iron-router to continue to the next matching route using the pass() method. When this method is invoked, iron-router will immediately stop processing the current matching route and invoke the next matching route. If no subsequent matching route is found, notFound handlers are called as normal.

Example:

Router.route('/:whatever', function() {
  this.pass();
  this.render('thisWillNeverRender');
});

Router.route('/hello', function() {
  this.render('hello');
});

Requesting /hello will render the hello template. Requesting anything else will result in a notFound. pass() works on the server as well.

This was actually pretty easy once I understood the basic mechanics of the new middleware-based iron-router (nice work BTW!). But there is a lot of complexity there, so while I think this is right, please review I've done this correctly.

The idea for this feature was brought about because of a meteor-blog issue, where we wanted the blog to handle any incoming slug to see if it was a blog post, and then pass the request on if it wasn't.

WDYT?

@hellogerard hellogerard changed the title Implement 'pass' function to skip to next matching route Implement 'pass' function to skip to next matching route ('next' branch) Sep 24, 2014
@hellogerard hellogerard changed the title Implement 'pass' function to skip to next matching route ('next' branch) Implement 'pass' function to skip to next matching (middleware) route Sep 24, 2014
@cmather cmather added this to the Brainstorming and Ideas milestone Oct 26, 2014
@cmather
Copy link
Contributor

cmather commented Oct 26, 2014

This is interesting. I guess in the current implementation you can call this.next() but it doesn't really get you quickly out of the entire route and onto another route. I'll have to take a closer look at the implementation. But the idea seems good. What do others think?

@hellogerard
Copy link
Author

I forget exactly at the moment why, but there was a difference between
calling this.next() and throwing the exception (maybe it was after
hooks?). On the server, though, it's identical.

On Saturday, October 25, 2014, Chris Mather notifications@github.com
wrote:

This is interesting. I guess in the current implementation you can call
this.next() but it doesn't really get you quickly out of the entire route
and onto another route. I'll have to take a closer look at the
implementation. But the idea seems good. What do others think?


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

Sent from my mobile device.

@cmather
Copy link
Contributor

cmather commented Oct 27, 2014

cool i'll need to come back to this to make sure i understand what it's doing.

@cmather cmather changed the title Implement 'pass' function to skip to next matching (middleware) route [Discussion] Implement 'pass' function to skip to next matching (middleware) route Oct 27, 2014
@hellogerard
Copy link
Author

Trying to help out on this one.. I re-ran this PR with my little test app, which at the time was built on top of the next branch using an older version of meteor (0.9.3, I believe). Meteor said something like:

In order to resolve constraints, we had to use the following
experimental package versions:

And then I got this error:

TypeError: Object [object Object] has no method 'setController'

I upgraded everything, and tried to run the apply the PR changes to the latest devel, but now my test app is saying that router has no method 'use'. Don't want to spend too much time on getting this PR back up & running.....

@hellogerard
Copy link
Author

So I did get this up & running on the latest devel branch as of this comment. It looks like it's working as intended. If I just use next() instead of throwing the exception in the client, it doesn't work.

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

Successfully merging this pull request may close these issues.

2 participants