remove ./routes from generator... #1323

Closed
tj opened this Issue Sep 7, 2012 · 12 comments

Projects

None yet

6 participants

@tj
Owner
tj commented Sep 7, 2012

too confusing for people

rump commented Nov 4, 2012

Where do you envision (and encourage) them that is less confusing? Do people outgrow putting them in app.js quickly?

Owner
tj commented Nov 4, 2012

yeah if you put the callback definitions in app.js that's no good, but considering this is where people start with most of the time they often get confused about require()ing in other modules in ./routes since they haven't learned node's module system yet

juzerali commented Nov 4, 2012

I think ./routes is sweet. It enforces good structure early on.

Owner
tj commented Nov 4, 2012

maybe just some more examples in ./routes would work, the index.js part confuses people new to node, most seem to think that if you create more files in there they'll be available to "routes" var in routes = require('./routes')

juzerali commented Nov 4, 2012

May be give a command line switch that keeps the code in 'routes' folder. Without the switch the new app will be minimal with all the code in app.js.

rump commented Nov 4, 2012

I discovered node this week so forgive me if this is crazytalk but what if app.js scanned and included everything in routes/? Then people could simply add a file in there and combine the VERB definitions with the callbacks.

rump commented Nov 4, 2012

Okay, so looking over how other 'sinatra inspired' frameworks are doing this, it seems they come in two flavors:

  1. Everything in app.js.
  2. Utilizing the seemingly standard subdirectory names: routes/views/tests/public/models/helpers/vendor/etc.

I don't think sinatra itself has a generator and it seems that people really appreciate Express for being clean and simple. So, maybe by default the express generator does #1 and then optionally juzerali's idea of a #2 command line switch?

That sounds better to me than splitting conventions and having #1 and #2 mixed.

The ones that include the subdirectories do automate the requiring of the files with app.rb/php/js logic but using different methods, for example ruby ones seem to require views/init.rb which then manually specifies each file to include.

On another note, I stumbled on a post where TJ mentioned a drupal module inspired app structure that maybe is worth exploring?

Owner
tj commented Nov 4, 2012

yeah neither are best-practice IMO, but we do need something to get people started. I used to have it all in app.js but then people kept asking how to use other files, then when I added other files people get confused as well haha. I also dont want to imply that ./routes is "the" way to go. I'm leaning towards app.js again because then at least people have to look into node's module system to discover how that works first

juzerali commented Nov 4, 2012

@visionmedia But please don't forget to include a command line switch that creates ./routes.

I am also against creating a fictive rule for using express routes in a
certain way. The example Code covers the various possibilties pretty good.
Nov need for a Standard
Am 05.11.2012 00:11 schrieb "Juzer Ali" notifications@github.com:

@visionmedia https://github.com/visionmedia But please don't forget to
include a command line switch that creates ./routes.


Reply to this email directly or view it on GitHubhttps://github.com/visionmedia/express/issues/1323#issuecomment-10056976.

I think you hit it when you said the index.js part confuses people. A lot of people won't know that require('/dir') automatically loads dir/index.js, so its confusing right away. Plus, index.js exports an 'index' function, so now there are two things called 'index' that you have to keep straight in your head. I'd suggest calling index.js something else (routes/home.js??), doing require('/routes/home') so it's explicit, and leave users to discover the dir/index.js alias trick on their own.

Contributor

The latest thinking by @jonathanong is to just remove the generator altogether and stick to examples (which I am +1 for). Going to close this issue since it has seen no love and we have better documentation on the site and examples.

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