-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Description
Q | A |
---|---|
Bug report? | no |
Feature request? | yes |
BC Break report? | no |
RFC? | yes |
Symfony version | 3.x, 4.x |
We can access env vars in several ways, and the Dotenv
class is already capable of handling env vars in all possible ways:
symfony/src/Symfony/Component/Dotenv/Dotenv.php
Lines 72 to 77 in f95ac4f
foreach ($values as $name => $value) { | |
$notHttpName = 0 !== strpos($name, 'HTTP_'); | |
// don't check existence with getenv() because of thread safety issues | |
if (!isset($loadedVars[$name]) && (isset($_ENV[$name]) || (isset($_SERVER[$name]) && $notHttpName))) { | |
continue; | |
} |
There are also some cases where the current Flex recipes are using $_SERVER
and it seems it breaks and should use $_ENV
.
A nice example is the symfony/recipes#331 issue: the APP_ENV
env var is created before the entrypoint is executed, and PHP doesn't set it in $_SERVER
but sets it in $_ENV
.
It's possible that it's a PHP issue, but I think we can handle this right in DotEnv
, and possibly the same way it handles checks in the populate()
method.
Proposal
I propose to add Dotenv::get($name)
and Dotenv::exists($name)
methods that would retrieve env vars based on their existence in these cases and this order:
- First in
$_ENV
- Then in
$_SERVER
- And fallback to
getenv()
even if it's not thread-safe
This way, we could use this in our front-controllers, either console or web, and we would never expect any edge case from this.
This would mean that Dotenv would be required in dev & prod, and not only in dev, but then the APP_ENV check could be reduced to this:
if (!Dotenv::exists('APP_ENV')) {
(new Dotenv())->load(__DIR__.'/../.env');
}
WDYT?