Dotenv::required should check for empty values

[MethodGrab]

Dotenv::required should check for empty values

[motin]

+1

[vlucas]

I agree.

[MethodGrab]

Great, thanks 👍 !

[sebastiaandegeus]

I disagree. Especially to include such a breaking change in a minor release. This could break projects which depend on an empty value being set.

See also:
#38

[motin]

Relevant: #35

[motin]

I agree with both sides :) There are pros and cons.

The generally better way to check env vars is to check for it's existance (getenv() returns false when it doesn't exist). Only env vars that have not been set, or specifially unset before execution of a script will be caught as "not set". Thus, a prerequisite for that type of check is to support unset within .env in case a env var is already defined for some reason (also see OP's example). This way, configuration directives can be set to "0", "" and other "empty" values as defined by php, without issues.

However, many will want the basic validation check to indicate that it has a non-empty, so changing it to the default behavior is not a bad idea imo. At the same time, it is not a good idea to break the api between minor versions. Maybe have two different methods? One requiredExists that requires it to be set only, and one required that requires them to be non-empty?

Btw, as soon as #35 is merged, one can manually specify "" as an allowed value for those specific config directives, so both methods could be used.