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

Dynamic segments can't contain dashes #83

Open
pushred opened this issue Feb 13, 2014 · 6 comments
Open

Dynamic segments can't contain dashes #83

pushred opened this issue Feb 13, 2014 · 6 comments

Comments

@pushred
Copy link
Member

pushred commented Feb 13, 2014

I think this issue is more about answering the question of what characters are not allowed in view filenames, including dynamic segments. Currently given a filename like {album-id.hbs} the router throws an exception. At the very least there should be some feedback in the log that a character threw it off.

Dashes aren't reserved in common filesystems, but they don't work in JavaScript's dot notation, so maybe it's good to not support dashes. The filenames of route segments are used as keys in the parameters object. It does seem that Handlebars supports bracket notation, so parameters[two-words] should be valid however.

@vic3685

@Fauntleroy
Copy link
Contributor

Probably has something to do with this: https://github.com/SparkartGroupInc/solidus/blob/master/lib/page.js#L60

@Fauntleroy
Copy link
Contributor

Alright it looks like the issue has to do with the fact that I'm making express route strings, and I'm not sure if it picks up dashes and underscores in variable names. This might take some doing.

@Fauntleroy
Copy link
Contributor

It doesn't look like I can use a custom regular expression with keywords! There MIGHT still be some way for me to do it manually, but it could take some time. For now we might just want to avoid... using dashes in there. You can always use an underscore!

@Fauntleroy
Copy link
Contributor

Note for @joanniclaborde

This is one of those issues that makes me think that using express for Solidus might not be the right approach. Express includes a great deal of functionality, and we're using it in a somewhat creative way. We might want to use something lower level, and possible multiple modules (http server, routing, middleware, etc) instead of one.

@pushred
Copy link
Member Author

pushred commented Apr 10, 2014

Express 4 came out recently, with some big changes to routing. It's also broken away from Connect. Have you seen koa? Is Connect starting to fall out of favor?

The big challenge to switching away from Express would probably be finding a suitably mature replacement for the Handlebars view engine. At least it's being maintained and works in Express 4.

@Fauntleroy
Copy link
Contributor

It doesn't really seem like Connect has fallen out of favor per-se, just that they're not basing Express on it anymore. The middleware seems like it should still work, it's just not auto bundled. I think they're just working on making Express more modular, and splitting out dependencies into their own independent modules.

I've seen koa, but I've only glanced over it. It uses some node.js features that are still experimental and is not ready for production use.

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

No branches or pull requests

2 participants