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

Keep hierarchy when updating store with use() function #200

Closed

Conversation

ernoaapa
Copy link

Previously, when store were updated with Provider.use() function it were moved as last in configuration hierarchy. This caused that usually some other configuration were overriding updated configurations and the updates never get applied.
Updated use() function to not remove/add the store, instead just "re-set" the store so the store key keep the same position in the this.stores hash keys.
Closes #176

Example:

file1.json

{
  "candy": {
    "something": "file1"
  }
}

file2.json

{
  "candy": {
    "something": "file2"
  }
}
var nconf = nconf
  .use('file', { file: 'file1.json' })
  .defaults({candy: {something: 'should never get this because read from files'}});

nconf = nconf.use('file', { file: 'file2.json' });

nconf.get('candy:something') == 'file2'
=> false

nconf.get('candy:something')
=> "'should never get this because read from files"

Expect the value be file2 because the file store were updated with use()

Previously, when store were updated with `Provider.use()` function
it were moved as last in configuration hierarchy. This caused
that usually some other configuration were overriding updated
configurations and the updates never get applied.
Updated `use()` function to not remove/add the store, but just
re-define it so the store key keep the same position in the
`this.stores` variable.
@indexzero
Copy link
Owner

Thank you for your contribution! We ended up implementing this slightly differently, but you are correct that the lexigraphic sort ordering in this.stores was not being respected. See #265

@indexzero indexzero closed this Aug 9, 2017
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.

2 participants