Skip to content

Conversation

hiagodotme
Copy link

Dynamically record application routes

Dynamically record application routes
@hiagodotme hiagodotme changed the title Update app.js Dynamically record application routes Nov 3, 2016
@hiagodotme
Copy link
Author

Hello I do not think it's cool I have to keep logging the routes, so I'm assuming that everything in the routes folder is a route. So I made an automatic importer.

Dynamically record application routes
Dynamically record application routes
Dynamically record application routes
@hiagodotme
Copy link
Author

Honestly, I do not know why you're not testing.

@gokaygurcan
Copy link

gokaygurcan commented Nov 4, 2016

I think everyone has their own way of registering routes. One may use one big routes.js while another uses a bunch of small or logically separated files.
At this point, generator doesn't try to direct you. Like Express' itself, I like to have a generator which is un-opinionated.

This changes forces me to have a ./routes folder, have an index.js file under that folder and have all the other routing files under routes folder again. I don't like this kind of approaches.

var index = require('./routes/index');
app.use('/', index);

This is simple, I can just change the path in require and voilà! In this new usage with dynamic routes (!), I need to change the whole logic. And finally, there're these performance problems too, having two synchronous method calls and a loop.

Of course, this is just my idea, I'd like to hear what others think.

@dougwilson dougwilson self-assigned this Jan 25, 2017
@dougwilson
Copy link
Contributor

So the reason the tests are failing is because the changes will no longer properly work when the process cwd is not the directory with the app, which is a common situation in CIs (like Travis CI) and app SaaS environments. I haven't heard from any other Express members on this, but my opinion on this is twofold: (1) this is just too much logic being added to the generator's code directly that needs to be maintained by the express-generator project (instead of pulling in a dependency) and (2) I think it is too opinionated, especially with the app.use path being determined by the name of a file on the file system, which means that you're bring in the character limits and case sensitivity limits of the underlying file system into the routing and limits you only to strings for the paths.

@dougwilson dougwilson closed this Jan 25, 2017
@dougwilson dougwilson added the pr label Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants