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

refine route lookup mechanisms #887

Closed
tj opened this issue Nov 10, 2011 · 6 comments
Closed

refine route lookup mechanisms #887

tj opened this issue Nov 10, 2011 · 6 comments
Labels
Milestone

Comments

@tj
Copy link
Member

tj commented Nov 10, 2011

the current app.lookup.get() etc stuff is kinda more "cute" than anything else, perhaps just a simple app.routes.filter(fn) and that's it

@quangv
Copy link

quangv commented Nov 14, 2011

yeh, is it suppose to return an array? cause I think it's returning an object right now...

the documentation has me believe it should return an array.

# Documentation  
app.lookup.get('/user/:id');
  // => [Function]

same goes for match()

app.match.all('/user/20');
  // => [Function, Function, Function]

But actual object being returned, not an array. (array-like object?)

# For successful lookup:

{ '0': 
   { path: '/.:format?',
     method: 'get',
     callbacks: [ [Function] ],
     keys: [ [Object] ],
     regexp: /^(?:\/\.([^\/\.]+?))?\/?$/i },
  router: 
   { app: 
      { stack: [Object],
        connections: 0,
        allowHalfOpen: true,
        _handle: [Object],
        _events: [Object],
        httpAllowHalfOpen: false,
        cache: {},
        settings: [Object],
        redirects: {},
        isCallbacks: {},
        _locals: [Object],
        dynamicViewHelpers: {},
        errorHandlers: [],
        route: '/',
        routes: [Circular],
        router: [Getter],
        __usedRouter: true,
        resources: [Object] },
     routes: { get: [Object] },
     params: {},
     _params: [],
     middleware: [Function] },
  length: 1 }


# For failed lookup:

{ router: 
   { app: 
      { stack: [Object],
        connections: 0,
        allowHalfOpen: true,
        _handle: [Object],
        _events: [Object],
        httpAllowHalfOpen: false,
        cache: {},
        settings: [Object],
        redirects: {},
        isCallbacks: {},
        _locals: [Object],
        dynamicViewHelpers: {},
        errorHandlers: [],
        route: '/',
        routes: [Circular],
        router: [Getter],
        __usedRouter: true,
        resources: [Object] },
     routes: { get: [Object] },
     params: {},
     _params: [],
     middleware: [Function] } }

@tj
Copy link
Member Author

tj commented Nov 14, 2011

yeah it's an array-like object right now but I definitely want to tweak this portion quite a bit

@tj
Copy link
Member Author

tj commented Nov 14, 2011

plus I think the docs haven't been updated, it's an array of Route objects

@bminer
Copy link

bminer commented Apr 25, 2012

What about just exposing app._router.map as app.routes? Then, one can just iterate through the map of routes? So, the 'GET' routes are exposed as an array of route objects at app.routes.get.

Or... what about something as simple as: app.router.match(method, url) and app.router.lookup(method, path) ???

@tj
Copy link
Member Author

tj commented Apr 25, 2012

lookup / match are too opinionated. they're stored as a map of { VERB: [ROUTES] } internally so if we want to make them removable people will have to go about it that way, though a single array would be nicer for some things. I guess first task is to determine what use-cases there are for exposing them at all

@bminer
Copy link

bminer commented Apr 25, 2012

Single array would be nice for some things, but usually, you know the VERB with which the route is associated, and you're only interested in those routes.

Use case for me is: I have a app.get('*', ...); route setup prior to a few other routes. The purpose here is to capture the request "no matter what." The route middleware essentially checks to see if the user is logged in. If so, it calls next(); otherwise, it will redirect to the login page. There is a special case here, though. If there are no other matching routes for that URL, I want to emit 404, rather than redirecting to the login page. That is, I just want to see if a given {method, url} pair matches ANY other route. Otherwise, I know that I should emit 404.

@tj tj closed this as completed in 18cdb3d May 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants