Skip to content
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

JSON files with the same name in the config directory #3846

Closed
fenichelar opened this issue Sep 30, 2016 · 5 comments
Closed

JSON files with the same name in the config directory #3846

fenichelar opened this issue Sep 30, 2016 · 5 comments
Labels

Comments

@fenichelar
Copy link

fenichelar commented Sep 30, 2016

Sails version: 0.12.6
Node version: 0.12.15
NPM version: 2.15.1
Operating system: OS X, CentOS, Debian (appears on all tested OSs)


I upgraded Sails from 0.12.3 to 0.12.6. I am no longer able to have JSON files with the same name in the config directory. Is this intentional or a mistake? Can these be changed to allow JSON files with the same name in the config directory?

To Reproduce:

sails generate new testApp
cd testApp
mkdir config/test1
mkdir config/test2
echo "{}" > config/test1/test.json
echo "{}" > config/test2/test.json
sails lift

Output:

/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/include-all/lib/help-include-all-sync.js:217
              if (_modules[grandchildKey]) { throw new Error('Attempting to fl
                                                   ^
Error: Attempting to flatten modules but duplicate key detected (`test`).  Enable `keepDirectoryPath: true` to enable namepspacing based on hierarchy.
    at /usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/include-all/lib/help-include-all-sync.js:217:52
    at /usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/lodash/index.js:3073:15
    at baseForOwn (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/lodash/index.js:2046:14)
    at /usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/lodash/index.js:3043:18
    at Function.<anonymous> (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/lodash/index.js:3346:13)
    at /usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/include-all/lib/help-include-all-sync.js:211:13
    at Array.forEach (native)
    at _recursivelyIncludeAll (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/include-all/lib/help-include-all-sync.js:174:11)
    at includeAll (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/include-all/lib/help-include-all-sync.js:306:5)
    at helpBuildDictionary (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/include-all/lib/help-build-dictionary.js:42:15)
    at Function.module.exports.aggregate (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/include-all/index.js:101:10)
    at Array.loadOtherConfigFiles (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/lib/hooks/moduleloader/index.js:246:22)
    at /usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/async/lib/async.js:591:38
    at _arrayEach (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/async/lib/async.js:85:13)
    at Object.async.auto (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/async/lib/async.js:552:9)
    at Hook.loadUserConfig (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/lib/hooks/moduleloader/index.js:244:13)
    at Hook.wrapper [as loadUserConfig] (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/lodash/index.js:3095:19)
    at Hook.loadModules (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/lib/hooks/userconfig/index.js:41:18)
    at Hook.wrapper [as loadModules] (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/lodash/index.js:3095:19)
    at Array.async.auto.modules (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/lib/hooks/index.js:79:27)
    at /usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/async/lib/async.js:591:38
    at _arrayEach (/usr/local/Cellar/nvm/0.32.0/versions/node/v0.12.15/lib/node_modules/sails/node_modules/async/lib/async.js:85:13)
@sailsbot
Copy link

Hi @fenichelar! It looks like you missed a step or two when you created your issue. Please edit your comment (use the pencil icon at the top-right corner of the comment box) and fix the following:

  • Verify "I have tried all the following (if relevant) and my issue remains:"

As soon as those items are rectified, post a new comment (e.g. “Ok, fixed!”) below and we'll take a look. Thanks!

If you feel this message is in error, or you want to debate the merits of my existence (sniffle), please contact inquiries@treeline.io.

@fenichelar
Copy link
Author

Ok, fixed!

@fenichelar fenichelar changed the title include-all is attempting to include json files in config directory JSON files with the same name in the config directory Sep 30, 2016
@mikermcneil
Copy link
Member

mikermcneil commented Oct 5, 2016

@fenichelar Thanks for following up! This should be allowed, since everything's just getting merged together anyway irrespective of filename. I believe this was due to the include-all version bump (and removal of sails-build-dictionary). Looking into it ASAP.

Note: there are a few (probably obvious) exceptions: It'd be a bad idea to create local.json, or config/env/production.json, for example.

@mikermcneil mikermcneil added the bug label Oct 5, 2016
@sgress454
Copy link
Member

@fenichelar @mikermcneil So the deal here is, the new version of include-all is exposing a previously unknown issue: namely that config files with the same name were silently overriding each other. So in the above example, if config/test1/test.json had {"dog": "woof"}, and config/test2/test.json had ("cat":"meow"}, then your lifted Sails app would have sails.config.cat but no sails.config.dog. As @mikermcneil said, all of these files are supposed to merge together, and file paths are not supposed to have any impact on the final sails.config object (with the understanding that env/** files are merged in after other config files, and local.js is merged on top of everything).

Good news is, this is a pretty easy fix. Just keep in mind that after we fix it and publish 0.12.7, any .json files you have under config/** will be merged together and added to sails.config (so you'll get both the 🐶 and the 🐱).

sgress454 added a commit that referenced this issue Oct 5, 2016
@sgress454
Copy link
Member

Fixed in 57b7775, published in v0.12.7. Thanks all!

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

No branches or pull requests

4 participants