dominictarr/rc

badness ensues if the default configuration is frozen

Closed this issue · 11 comments

trivial example:

var DEFAULTS = Object.freeze({baz: 'quux'});
// var DEFAULTS = {baz: 'quux'};
process.argv.push('--foo', 'bar');
console.log(require('rc')('test', DEFAULTS));

expected output:

{ baz: 'quux', _: [], foo: 'bar' }

actual output:

{ baz: 'quux' }

(I'd suspect the fix here would be to unshift an empty object onto the configs array)

(but deep-extend might not work like that?)

why is the default object frozen?

the same reason you'd freeze any object, I suppose.

That is not an answer. To maintain a project like this I need to understand how people are using it.
Normally the defaults are just an object literal that is hard coded, so no reason to freeze it.
If you are freezing it, that tells me it's being passed in from somewhere strange, and that you are probably using the defaults object for something else too... what is that strange place, and what is that something else?

I understand your concern. My use case is trivial: the object should be immutable, because I don't expect it to change after it is frozen. Since the const keyword only applies to primitives, AFAIK Object.freeze is the only analog.

This suggests to me that rc, but maybe more likely deep-extend, is modifying the "defaults" object in an unexpected way. If the culprit is the latter, I'll file an issue there instead.

so are you using it again for something else? or is it just because you feel it should be immutable?

The latter. I'm not clear why this library or its dependencies prevent that.

Okay so no offence but i don't consider incompatibility with an opinion to be a bug.
you would fix this in deep-extend.

This isn't really a matter of opinion; it's a matter of intent.

This is not a bug in deep-extend, so I won't be filing an issue there--deep-extend's intended behavior is to modify the first parameter passed to it, which is how rc is using it.

Is it rc's intended behavior to modify the "defaults" object? If rc modifies no other sources of configuration (including process.env, process.argv, and files), why does rc modify the "defaults" object? Is it intended to work like deep-extend?

If this was the intent, please close this issue. But if it isn't, please consider my proposed alternative.

The fix is trivial; but to be on the safe side, it would necessitate a major release, because it breaks the API.

What is really bothering me about this issue that you took several round trips to explain why you needed this, and it's also not actually fixing a real bug. closing.