Discuss: env var replacement in any config file

[leoj3n]

This is a continuation of discussion that originated in #535.

Currently, env var replacement is only allowed in custom-environment-variables.*.

Sectioning off env var replacement to this file seems obtuse, and it's not clear what the benefits are.

I think most people using config for the first time would expect to be able to use env vars in any config file, like support was added for by the following downstream project:

https://docs.feathersjs.com/api/configuration.html

So, can anyone think of or explain the benefits of limiting env var replacement to custom-environment-variables.*? If not, I think it would more intuitive to replace env vars in any config.

Of course, there is always the "nuclear" option of using *.js, but if that's the recommendation the benefit of custom-environment-variables.* becomes unclear.

[lorenwest]

custom-environment-variables has usefulness in mapping environment variables to configs, and is a different concern from the issue of those mappings being static or dynamic.

Can anyone explain why we should add templating to .json files when the functionality has been there in .js since day one?

-Loren

blah,blah: I think it opens unnecessary complexity. Once you add it to custom-environment-variables, someone is 100% sure to want it in other .json files. Then the discussion of your favorite templating format, with opinions from the Ruby and Java worlds. Maybe the format should be configurable? How about nested templating? Hey - let's add templating methods - looping, math, regex's. Sheesh - just use .js already.

[leoj3n]

@lorenwest In that case what about completely removing env var support?

[lorenwest]

Good question. The practical answer is that people are using it. Like anything - once you add it, it's very hard to remove.

[jsardev]

@lorenwest I don't get your arguments. Just choose one standard of templating and stick to it. Current custom-environment-variables solution is actually the thing that adds unnecessary complexity. It's completely unclear for a newcomer that a production.json config file doesn't use any env variables and that they live in a separate file.

IMHO custom-environment-variables should be removed and just leave a hint that you can use .js files with process.env. I guess that another solution would be to provide a feature to interpolate env variables into all of the config file formats.

According to:

The practical answer is that people are using it. Like anything - once you add it, it's very hard to remove.

Just remove it and release it as a next major version which is not backwards-compatible. That's what semantic versioning is for.

[lorenwest]

Agreed. The custom-environment-variables mechanism could go away with a common template format for all file types. If we did this I'd vote for ${env_var_name} format because it's NodeJS literal format and shell-esque.

I'm in favor of this change, and would recommend rolling it out in two major releases. The first release announces any usage as deprecated, and removing it in the following release. I would want at least a year (maybe two) of deprecation warnings before removing the functionality because it requires work to change from one mechanism to the other.

[jsardev]

@lorenwest That sounds reasonable! 😃

[markstos]

I'm not in favor of change. I think custom-environment-variables.json usefully isolates environment variable mentions to a single place. I like knowing that I can go to a single file to find as a reference all the environment variables affecting my configuration.

Also, removing custom-environment-variables.json requires users of .js config files to use an adhoc solution, since templating those files is not a good fit.

[jsardev]

@markstos But environment variables isolation is the main problem here. Why would you look for a file particular for environment variables? You rather look for a file with a specific environment configuration. Having default config is already making it cumbersome to get to know the configuration of a particular environment. But that's just my opinion.

Actually seeing how many overrides there are in node-config I'm becoming convinced that current behavior is suitable for this library, as it encourages to split up the config in many files. And I don't like that so maybe I'm just looking for something else 😄

[lorenwest]

The value of the default config is to have a single place to see the full list of configurations, and their default value. All the other files contain only the changes from defaults, so they tend to be very concise, making complex deployments manageable.

I agree with @sarneeh that there are too many built-in file path overrides. I'd be in favor of reducing the number of built-in overrides, replacing it with the more powerful but not often used feature of using multiple values for NODE_ENV.

As to the custom_environment_variables issue, I'm also with @sarneeh that we already have too many files and ${env_var} type templating is more natural than the complex custom_environment_variables file format. I'm ok with it being different for .js because ${process.env.VAR} works in that file. Maybe our templating logic could use ${process.env.VAR} everywhere to make it consistent?