4.0.1 regression

[oliviertassinari]

It seems that the default key is no longer respected as soon as we use an env key and this value is missing. I haven't looked to much into the issue. Maybe someone else will. I'm reverting to 4.0.0.

[madarche]

@oliviertassinari can you provide an example please?

[oliviertassinari]

@madarche For instance, the convict.get('session.secret') isn't present with v4.0.1 but is with v4.0.0. I'm relying on the SESSION_SECRET env variable.

session: {
  secret: {
    default: 'bubu',
    env: 'SESSION_SECRET',
    format: String,
  },
},

[kevinoneill]

I think this is to do with #217 which changes the behaviour of empty string from triggering the default to supplying the empty string.

In my case, the 4.0.0 -> 4.0.1 update was a generated as part of a yarn install that updated convict as a side effect.

A change like this (even if it represents a bug fix) should have been flagged as breaking.

[oliviertassinari]

I think this is to do with #217 which changes the behaviour of empty string from triggering the default to supplying the empty string.

@kevinoneill I think that you are right. I'm running node-convict in a docker environment. I have the following warning: The SESSION_SECRET variable is not set. Defaulting to a blank string.. With the new behavior of the project. I do no longer see the default value set, instead, a blank string.

[salper]

Well, considering that there is no null or undefined value in bash environment variables, it probably was a bold move.

[madarche]

A PR with the corresponding test is welcome.

[salper]

Reverting this pull request should be enough.

[mojavelinux]

If the environment variable is present and the value of the variable is empty string, the config value should be set to empty string. If the environment variable is missing, the default value should be used. Is that the correct assumption?

[kevinoneill]

Seems right to me.

…

On 4 Jul 2018, at 12:00 pm, Dan Allen ***@***.***> wrote: If the environment variable is present and the value of the variable is empty string, the config value should be set to empty string. If the environment variable is missing, the default value should be used. Is that the correct assumption? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#224 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAP1cTTVhQyOpTywGYEPHNC21m4ZXyyks5uDCHGgaJpZM4Pow1j>.

[salper]

Well... That is the regression. Previously, a FOO= would use the default value. Since the patch, it overrides the default value. Some services which rely on declaring statically needed environment variables like docker-compose are impacted by this kind of behavior.

[mojavelinux]

I agree this change warranted a major version. At this point, though, there are probably users relying on both behaviors, so that's water over the damn. What we need to decide is how to move forward.

If the decision is made to rollback, there still needs to be a way to unset a config value using an environment variable as the environment variable is the last resort to override a value. One option is to recognize the special value "undefined" as an intent to unset the value. Another option is to use a companion environment variable whose sole purpose is to unset the value (e.g., NO_SESSION_SECRET=true). (This is similar to a negated CLI option like --no-session-secret). If anyone has other ideas, please propose them.

[A-312]

If anyone has other ideas, please propose them.

With customGetter #313 devs can rewrite env and arg getters like they want.

[A-312]

With custom getter (next update, 6.0.0) :

convict.addGetter('env', function(value, schema, stopPropagation) {
  if (value in this.getEnv()) {
    stopPropagation();
    return schema._cvtCoerce(this.getEnv()[value]);
  }
}, false, true);