Contradictory behaviour when NODE_ENV is undefined and no validation rule set about envalid HOT 9 CLOSED

Fiiiiinally fixed this, devDefault values will no longer be used when NODE_ENV is not present in the environment.

Also:

This leads me to wanting to treat it like any other str env var and not do anything special with it. ❄️

Since there's no official spec about what its values should or shouldn't be (just convention)—or what will or won't happen if its not set (depends on your code and packages installed)—I think its probably safer, easier, and simpler to leave this definition in user-land.

This will be the case in v7, NODE_ENV is just like any other env var and you need to add a validator for it if you want it to be validated.

from envalid.

I agree defaultNodeEnv seems weird. it should instead add itself to the env if NODE_ENV is not set.

Does that make sense? Would love to se a PR for it!

from envalid.

Agreed, this is contradictory, thanks for pointing it out. The original intent was for NODE_ENV validation to be "baked in", and if you ever forgot to set the env var, production is the safest choice. devDefaults came later and I never noticed that they would also be applied in this case.

IMO the fix here is that in this case devDefaults should not be used. Any downside to just closing that loophole?

from envalid.

I think this is the key: "actual node env should then be used through out" combined with using a default

So something like:

function cleanEnv(inputEnv, specs, options = {}) {
    let output = {}
    const errors = {}
    // If validation for NODE_ENV isn't specified, use the default validation:
    specs = Object.assign(
        {
            NODE_ENV: validateVar({
                name: 'NODE_ENV',
                spec: str({ choices: ['development', 'test', 'production'] }),
                rawValue: env.NODE_ENV || 'production'
            })
        },
        specs
    )
    const env =
        options.dotEnvPath !== null ? extendWithDotEnv(inputEnv, options.dotEnvPath) : inputEnv
    const varKeys = Object.keys(specs)

    // the rest...

Then everything should work correctly, right?

from envalid.

Yes that looks like it should do the trick

from envalid.

Just ran into this confusion myself refactoring #81.

@SimenB 's code is on the right track, but won't work as-is: the keys in specs are iterated below in // the rest ... and passed back to validateVar again, which expects spec objects and not values. So you'll end up with this error.

I like the intent behind this feature, but honestly I'm wary of its auto-magical nature. There's nothing on NodeJS docs about NODE_ENV specifically (go there and search the page for NODE_ENV and find nothing).

According to a Nodejitsu engineer from back in the day:

[NODE_ENV is] not documented in node core because it's just a convention, made
popular by express. Node itself doesn't care what the environment vars
are really, it just lets you read and write them via
http://nodejs.org/docs/latest/api/process.html#process_process_env

This leads me to wanting to treat it like any other str env var and not do anything special with it. ❄️

Since there's no official spec about what its values should or shouldn't be (just convention)—or what will or won't happen if its not set (depends on your code and packages installed)—I think its probably safer, easier, and simpler to leave this definition in user-land.

My two cents.

from envalid.

@af I have a small question about this fix:

Shouldn't devDefault also work when I set NODE_ENV: str({default: 'development'}) in the cleanEnv ?
Thanks

from envalid.

@MikeYermolayev here's the current logic for when devDefault is used. So in the case above, if NODE_ENV is not set to production then I believe it should be used.

    // Use devDefault values only if NODE_ENV was explicitly set, and isn't 'production'
    const usingDevDefault =
      rawNodeEnv && rawNodeEnv !== 'production' && spec.hasOwnProperty('devDefault')

from envalid.

@af well looks like it's not true.

I assume rawNodeEnv is being read only from provided environment so usingDevDefault doesn't consider any defaults from provided spec

from envalid.