-
Notifications
You must be signed in to change notification settings - Fork 140
Description
Hello, first of all thank you for this awesome tool.
I would like to report which seems to me like a bug.
In our setup we use default values, config.set, config.loadFile and environment variables.
During the issue debugging I have discovered that in the scenario when config.set and environment variables are used without config.loadFile we get unexpected results. Consider examples below, which are excerpt from your documentation and the simplified version of our setup:
example-1
process.env.PORT = 8080; // environment variable is set
const config = convict({
port: {
default: 3000,
env: 'PORT'
}
});
config.load({ port: 9000 });
console.log(config.get('port')); // 8080 from env variable
example-2
process.env.PORT = 8080; // environment variable is set
const config = convict({
port: {
default: 3000,
env: 'PORT',
},
});
config.set('port', 9000); // config.set without config.loadFile or config.load
console.log(config.get('port')); // 9000 from config.set
example-3
process.env.PORT = 8080; // environment variable is set
const config = convict({
port: {
default: 3000,
env: 'PORT',
},
});
config.set('port', 9000); // config.set
config.load({ port: 10000 }); // config.load after config.set
console.log(config.get('port')); // 8080 from env
✅ In example-1 everything works as it should and as per documentation.
❌ In example-2 you can see that using config.set will override environment variable which must have higher precedence as per docs.
✅ In example-3 if we add config.load right after config.set it actually fixes the issue from example-2. It feels like config.load re-instantiates environment variables precedence which config.set doesn't do.
If it is intentional behaviour of the config.set method then it should be clearly stated in the documentation, ideally with the provided example. If not, then I believe this is a bug that should be fixed.