Skip to content

Conversation

@orthagonal
Copy link
Collaborator

No description provided.


// 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) => {
Copy link
Member

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) => {
Copy link
Member

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

// for each filename, get a list of configured routes defined by it
configureAllRoutes(files, done) {
const routeConfigs = {};
files.forEach((fileName) => {
Copy link
Member

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

processedRouteConfig.path = getCompletePath(options, fileName, originalRouteConfig.path);
fileRouteList.push(processedRouteConfig);
});
routeConfigs[fileName] = fileRouteList;
Copy link
Member

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?

done();
}
}, (err2) => {
if (err2) {
Copy link
Member

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

glob('**/*.js', {
cwd: settings.path
}, (globErr, files) => {
if (globErr) {
Copy link
Member

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?

async.autoInject({
// confirm that settings.path exists and is a directory:
confirmDirectoryExists(done) {
fs.exists(settings.path, (exists) => {
Copy link
Member

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

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 !== '/') {
Copy link
Member

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

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

Successfully merging this pull request may close these issues.

3 participants