Different loading of user configuration in subdirectories

[kumpelblase2]

I have a small issue with the way user configurations are loaded into sails.config when it comes to configuration files in subdirectories.
Lets suppose we have this directory tree:

config/
- env/
  ...
- locals/
  ...
- fruits/
  - apple.js
  - banana.js
...

When I now load my app, wouldn't the most intuitive way of accessing the values of banana be sails.config.fruits.banana ? Because right now, all config files of subdirectories get merged into the main config value, without sub-directives which depict the directory they're in, thus resulting in sails.config.banana. Since locals and env have their own loading process, this shouldn't be much of an issue right?

This would not only tidy up the config variable (because you can put similar configuration files into a subdirectory instead of polluting the main namespace), but also allows you to separate sails specific configuration and user made configuration. For example, when I want to have a config for some policies of my internal logic, I could then put them under config/app/policies.js and it would not add to the policies for sails.

TL:DR: A configuration file under config/sub/file.js should be accessible via sails.config.sub.file and not via sails.config.file.

[kumpelblase2]

I would have a working but dirty solution ready:
sails:

diff --git a/lib/hooks/moduleloader/index.js b/lib/hooks/moduleloader/index.js
index e0ed0c6..9edc846 100644
--- a/lib/hooks/moduleloader/index.js
+++ b/lib/hooks/moduleloader/index.js
@@ -166,7 +166,8 @@ module.exports = function(sails) {
             exclude   : ['locales', 'local.js', 'local.json', 'local.coffee', 'local.litcoffee'],
             excludeDirs: /(locales|env)$/,
             filter    : /(.+)\.(js|json|coffee|litcoffee)$/,
-            identity  : false
+            identity  : false,
+            markDirectories: true
           }, cb);
         },

sails-build-dictionary:

diff --git a/index.js b/index.js
index 97f66fb..1e96b9c 100644
--- a/index.js
+++ b/index.js
@@ -83,10 +83,14 @@ function buildDictionary (options, cb) {
                                return cb(new Error('Invalid module:' + module));
                        }

-                       // Merge module into dictionary
-                       _.merge(dictionary, module);
+                       if(!module.isDirectory) {
+                               // Merge module into dictionary
+                               _.merge(dictionary, module);

-                       return;
+                               return;
+                       } else {
+                               module = _.omit(module, 'isDirectory');
+                       }
                }

Like I said, it's dirty and I feel like the changes made to build-dictionary are too case specific.

[sgress454]

With the exception of the env folders, Sails configuration is intentionally ignorant of file location; it just loads all the files and merges them together. You'll notice that the default files, like config/cors.js, start with module.exports.cors = {. If you want config/fruits/apple.js to work the way you mentioned, just structure the file like:

module.exports.fruits = {
   apple: {
   }
};

The extra few keystrokes are a lot better than the headache you'd have if we automatically determined the config keypath based on the file structure, and then you moved one of those files.

[kumpelblase2]

As far as I have tested, using the content of apple.js you provided (sitting under config/fruits/apple.js) I do not end up with sails.config.fruits.apple but instead I get sails.config.apple.fruits.apple.

So as far as I can see, I have to put everything into one file or use a place that is not getting auto-loaded and load it manually?

[sgress454]

Did you revert your changes to moduleLoader before you tried?

[kumpelblase2]

Yes, I did a clean npm install

[sgress454]

My apologies--I should have actually tried this myself before I suggested it 😳.

It looks to me like the flattenDirectories option should be set here, which would make things work the way they're intended. I'm a bit hesitant to do it now in case there are folks who are somehow relying on the current behavior; we'd at least to provide a way to revert to the previous behavior (probably an option you could put in .sailsrc...)

[kumpelblase2]

Should I change the description of the main issue to depict the new context?

[sgress454]

Added a fix and tests for this, and added a note to the migration guide. Thanks for pointing this out!

[kumpelblase2]

Alright, thank you!