Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

nconf.file merges don't work correctly #28

Closed
andrewrk opened this Issue · 6 comments

4 participants

@andrewrk

2 problems:

  1. nconf.file defaults operate in the wrong order. You can see in this list below that the chain of overriding goes from top to bottom; the things listed higher up override the ones lower down. The nconf.file one operates in the reverse order, however. I had to swap those two lines to get this to work correctly in my app.

  2. If the first file listed is missing, it will delete all config, causing the defaults to be what gets selected. This is bad. Instead it should just not change any config.

nconf
  .argv()
  .env()
  .file({file: "#{process.env.HOME}/.groovebasinrc"})
  .file({file: "/etc/groovebasinrc"})
  .defaults
    log_level: 3
    http:
      port: 80
    mpd:
      host: 'localhost'
      port: 6600
      conf: "/etc/mpd.conf"
@jstewmon

I remember being burned by this as well. This scenario is not well documented, and perhaps an update should be made since I am not the only one who expected the library to work the way your sample is written.

Anyway, the way things currently stand...

You can only use each of the convenience (argv, env, file, defaults, overrides) methods once because each uses it's name as the key for the config store. If you want to use multiple files, you need to add them like this:

var conf = require('nconf');
conf.add('env-file', {type: 'file', file: path.join(path.resolve(__dirname, '../config'), (conf.get('NODE_ENV') || 'development') + '.json')});
conf.add('default-file', {type: 'file', file: path.resolve(__dirname, '../config/default.json')});

The important part is giving the config store a unique key, which is the first argument passed to add.

I'm surprised this issue sat for a month without a response. What's up, @flatiron ? @indexzero ? @mmalecki ? Is this lib being maintained?

@indexzero
Owner

@jstewmon Yes. It is. Unfortunately life can't be all open-source maintenance all the time =D

@indexzero
Owner

@jstewmon A pull-request to fix this issue would make you a hero in my book.

@andrewrk
@jstewmon jstewmon referenced this issue from a commit in jstewmon/nconf
@jstewmon jstewmon api and doc change for indexzero/nconf#28 81861d8
@jstewmon

@indexzero Watch out - that commit has that showHelp change in it too. Did you ever put that elsewhere? I'd like to align my master with flatiron, but I'm using that feature in a project.

@jfhbrook jfhbrook referenced this issue from a commit
@jstewmon jstewmon api and doc change for indexzero/nconf#28 (`.file` may now take a str…
…ing instead of an object)

Conflicts:

	lib/nconf/provider.js
6353d02
@pksunkara

Fixed

@pksunkara pksunkara closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.