Path matching semantics update

[blakeembrey]

I would like to make a breaking change update to the way paths match, but I'm interested in everyones opinions before I implement any changes. My biggest gripe right now is that the special characters outside of a custom matching group restricts the characters that can be used inside of a matching group. I want to change this behaviour so only regex characters can be used inside a matching group and characters outside will be escaped.

Examples:

• /test? - currently valid, makes the t character optional which really looks like a bug
• /:test(.*) - currently invalid, periods are being escaped and asterisks are being turned into matching any character groups
• /(\\d+) - currently won't be indexed properly since we don't have any regex group logic
• /\\* - currently invalid, can't escape the meaning of special characters and will still become a match all route

[jonathanong]

1. /test? should match both / and /test. non-strict mode, /asdf/test? should match /asdf as well.
2. /:test should essentially be /:test(.*) anyways, so (.*) would be unnecessary.
3. is that a regex expression without a name? without a name they're useless anyways, so i don't mind

[blakeembrey]

@jonathanong Sorry, these were incredibly trivial examples - let me think of something more complex and realistic.

1. I don't think the question mark should actually have any matching ability outside of a regex group, but that behaviour would be tricky to get right.
2. Any regex using * and . is currently impossible to create because of the replacement rules. E.g. /:test(.+), /:test(\\d*), etc.
3. Is there a value in keeping non-named matching groups around? I can imagine it being useful in a number of cases, but if not we could transform them into non-matching.

[jonathanong]

Is there a value in keeping non-named matching groups around? I can imagine it being useful in a number of cases, but if not we could transform them into non-matching.

i don't see any value and wouldn't mine deprecating them or something

i'm okay with the other two gotcha's as long as they're documented.

[chirag04]

app.get('/about', info.about);

app.get('/:country', home.city_selection);

//generic renderer 
app.use(function(req, res){
        var somedata = {};
    res.render('website.ejs', somedata);
});

citiy_selection and about will just set the site_state and call next().

Control should then goto the generic renderer to display the site.

However when i visit about page citiy_selection route is also executed. What is it that i am doing wrong here?

[blakeembrey]

@chirag04 Is info.about calling next()? Since both /about and /:country would match /about, they run sequentially.

[chirag04]

@blakeembrey Yes, both info.about and home.city_selection call next so that control moves to the generic renderer.

Is there a way to not execute multiple matching routes?

How should i go about solving this apart from not calling next in info.about?

[blakeembrey]

@chirag04 There isn't really a way to do this without something hacky, but in reality you don't want to write routes that do fall through like that. Can you pull the function out use it in both places?

[chirag04]

I want to make it generic. I dont want to pull that function out and duplicate in both places.

However i am using this approach for now:

Instead of calling next, each about and city_selection will call render_site function.

Eg:

var render_site = require('../helpers/render_site');

about: function(req, res, next) {
        req.data.site_state = 'about full-page';
        req.data.page_title = 'About Us';
        render_site(req, res);
    }

Render site moved out of app.use wrapper into an independent module

module.exports = function(req, res) {
    res.render('website.ejs', req.data);
}

This seems to work for now.