Prevent automatic expansion of environment variables

[claustres]

This is usually fine to be able to use environment variables in config files without the need of creating an additional file as with node-config : https://github.com/lorenwest/node-config/wiki/Environment-Variables#custom-environment-variables.

However it has a side effect that if you want to have a string value corresponding to the name of an environment variable you can't because you have no mean to prevent expansion. I found the specific case of a string value required to be TMP but this corresponds to the TMP environment variable, so expanded to my temporary directory path !

I would recommend to prefix environment variables with eg $ so that we have control over the expansion.

[claustres]

I have as well another unexpected side-effect of the config processing made by Feathers, any string value starting with a . get it expanded as the config file path : https://github.com/feathersjs/feathers-configuration/blob/master/src/index.js#L28. I suggest to avoid doing this which leads to unexpected side effects.

One of the interest of tools like node-config is that config files can be plain JS files so that they are interpreted. The user can thus manage any processing issue by its own like this:

module.exports = {
  forecastPath: path.join(__dirname, '..', 'forecast-data')
}

By the way you could manage environment variable as well like this:

module.exports = {
  forecastPath: process.env.FORECAST_PATH || path.join(__dirname, '..', 'forecast-data')
}

Let me know what you think about it.

[daffl]

You can use the escape character from #4

{
  "env": "\\NODE_ENV",
  "path": "\\./home"
}

Sets env to the actual string NODE_ENV and path to ./home

[claustres]

Missed this, thanks very much !

Maybe a single line in the README should be sufficient to let the world know about it.

[daffl]

I'm actually not sure why it got never added. A pull request would be much appreciated although I'm still debating of just doing it the node-config way. Maybe @ekryski and @slajax have some thoughts on this as well.

[kc-dot-io]

IIRC, It was my understanding when I implemented that node-config that it required you to add the custom-environment-variables.json file which acted as the proxy to any expansion. I had understood, that if the env var didn't exist in this config it was not expanded. Is this not the case?

Perhaps something changed up stream then?

[kc-dot-io]

As it's coming back to me, I probably have to revisit the code because I have the feeling that the expansion logic we used previously where (example) NODE_PATH in production.json would expand based on env vars conflicts with node-configs methodology. I think there were some possible edge cases when I implemented node-config.

I'll check the code here right now and see if I can remember, it was a while ago.

[kc-dot-io]

@daffl @ekryski I can't remember exactly, but as I recall, some of this logic didn't exactly jive with node-configs environment stuff. I didn't want to kill it, so I think I jerry-rigged it and it was working but it could have some un-intended edge cases. It's like a double env thing if you try to use both methods for expanding.

[claustres]

I think this logic is right almost 99% of the time and for specific use cases like mine the escape character is just fine so I propose to simply add a comment about it in the README/docs.

[kc-dot-io]

@claustres 10-4, do you mind kicking over a PR for us? We'll merge right away.

[claustres]

Will do that tomorrow, simply waiting for your approval

[claustres]

Actually I found it somewhere in the doc https://docs.feathersjs.com/guides/advanced/configuration.html#app-configuration, maybe I will simply add a comment in the README as well

[kc-dot-io]

@claustres what needs approval? You have my approval ;)

[daffl]

It was actually in a previous version of the documentation that got lost. I just added it back in #43