-
Notifications
You must be signed in to change notification settings - Fork 1
split into files #23
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
base: master
Are you sure you want to change the base?
split into files #23
Conversation
lib/getCompletePath.js
Outdated
|
|
||
| // the 'base' of the path for all routes defined in a given file. | ||
| // eg "/api/folder1/myfile.js" will return "/api/folder/myfile/", | ||
| const getRoutePathBase = (options, fileName) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move this into a separate file and add tests just for this function?
| }; | ||
|
|
||
| // get the full route path: | ||
| module.exports = (options, fileName, originalPath) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets make sure we test this function by itself
lib/routeLoader.js
Outdated
| // for each filename, get a list of configured routes defined by it | ||
| configureAllRoutes(files, done) { | ||
| const routeConfigs = {}; | ||
| files.forEach((fileName) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets create a separate function for getting routes from a file and another getting a route from an export and add tests for those functions individually
lib/routeLoader.js
Outdated
| processedRouteConfig.path = getCompletePath(options, fileName, originalRouteConfig.path); | ||
| fileRouteList.push(processedRouteConfig); | ||
| }); | ||
| routeConfigs[fileName] = fileRouteList; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need them by filename? Can't we just turn into an array here and pass that through to server.routes in the next method?
lib/routeLoader.js
Outdated
| done(); | ||
| } | ||
| }, (err2) => { | ||
| if (err2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't think we need this if here, I think it'll bubble up the error
lib/routeLoader.js
Outdated
| glob('**/*.js', { | ||
| cwd: settings.path | ||
| }, (globErr, files) => { | ||
| if (globErr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this here, can we just pass in done as the callback?
lib/routeLoader.js
Outdated
| async.autoInject({ | ||
| // confirm that settings.path exists and is a directory: | ||
| confirmDirectoryExists(done) { | ||
| fs.exists(settings.path, (exists) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets create another lib file for validateRoutesDirectory
lib/getCompletePath.js
Outdated
| let returnPath = path.join(basePath, originalPath || '').replace(new RegExp('(\\\\)', 'g'), '/'); | ||
| returnPath = `${prefix}${returnPath}`; | ||
| // if there's a trailing slash, make sure it should be there: | ||
| if (_.endsWith(returnPath, '/') && (!_.endsWith(basePath, '/') || basePath === '/') && returnPath !== '/') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe endsWith and startsWith are now built in. We might not need lodash anymore
No description provided.