Envalid behaviour for str() validator changed from v2.4.1 to v2.4.2 about envalid HOT 6 CLOSED

Hi @bram-rongen thanks for raising the issue. IIRC that change was intended as a bugfix, but I kind of see both sides with this one:

• node does distinguish between '' and undefined for env vars, so allowing an empty string without raising an error does make sense in a way, but...
• I can't think of a case for having an empty string in an env var as a semantically useful value

Thoughts or feedback @SimenB @ibratoev ?

from envalid.

The intended functionality is to throw on missing values (undefined), not falsey values ('', null, false), so I think the current behavior is the correct one, and it was correct to have it in a patch release.

Whether the str validator should throw or not on empty (or only whitespace) value, I have no opinion. Maybe?

from envalid.

After thinking about it some more, I think @SimenB is correct and the current behaviour is what we want. If you want to reject empty strings, it's pretty simple to create a custom string validator that works that way.

@bram-rongen I'd welcome that test if you'd still like to write it, otherwise I'll try and get around to it in the next few days.

from envalid.

Thanks for you response guys, for me the current behaviour works well enough! We've changed our test suite and all is well :)

from envalid.

There actually already was a test for this case :) I added a note in the readme, so hopefully that resolves the ambiguity moving forward.

from envalid.

Thinking about this, we might want to check for empty string... What I wanted to accomplish was to not throw MissingEnv for false passed programatically. I can see the error when something like this is done:

$ SOMETHING= node -pe 'process.env.SOMETHING.length'
0

But meh 😄

from envalid.