Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dotenv] Add Dotenv::bootEnv() to check for .env.local.php before calling Dotenv::loadEnv() #35308

Open
wants to merge 1 commit into
base: master
from

Conversation

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jan 11, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? yes
Tickets -
License MIT
Doc PR -

The goal of this PR is to eventually get rid of the config/bootstrap.php file in Symfony 5.1 apps.
I think we've done enough iterations on that piece of bootstrapping logic to put it inside the Dotenv component.
This fully replaces https://github.com/symfony/recipes/blob/master/symfony/framework-bundle/4.2/config/bootstrap.php

It doesn't conflict with current apps so they'll be fine keeping the config/bootstrap.php file until they're upgraded.

The new bootstrapping logic will require adding this line in bin/console and public/index.php:

(new Dotenv())->bootEnv(dirname(__DIR__).'/.env');

Recipes updated at symfony/recipes#724

@nicolas-grekas nicolas-grekas added this to the next milestone Jan 11, 2020
@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:dotenv-boot branch 7 times, most recently from e44c3a7 to 19cd11c Jan 11, 2020
@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:dotenv-boot branch from 19cd11c to d66bcd9 Jan 12, 2020
@nicolas-grekas nicolas-grekas changed the title [Dotenv] load .env.local.php and define APP_DEBUG [Dotenv] Add Dotenv::bootEnv() to check for .env.local.php before calling Dotenv::loadEnv() Jan 12, 2020
@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:dotenv-boot branch 2 times, most recently from cd1b59d to 3c3f05a Jan 12, 2020
private $usePutenv;
private $envKey;
private $debugKey;
private $prodEnvs = ['prod'];

This comment has been minimized.

Copy link
@ro0NL

ro0NL Jan 12, 2020

Contributor

perhaps for consistency, define $testEnvs = ['test'] + $defaultEnv = 'dev'; as well? Then cleanup the arg list in loadEnv()

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Jan 12, 2020

Author Member

I considered that, but I think it's not worth the upgrade trouble.

This comment has been minimized.

Copy link
@ro0NL

ro0NL Jan 12, 2020

Contributor

it would avoid a half stateful / stateless API, and perhaps is worth it from the component's design POV.

This comment has been minimized.

Copy link
@ro0NL

ro0NL Jan 12, 2020

Contributor

do we prefer mutability? or what about new Dotenv('APP_ENV', 'APP_DEBUG', 'dev', ['prod'], ['test']) :/

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Jan 12, 2020

Author Member

We have no issues with mutability when its purpose is to configure the instance.
Using many constructor args is a pain when you only want to change the last one.
Setters are better here IMHO.

This comment has been minimized.

Copy link
@ro0NL

ro0NL Jan 12, 2020

Contributor

i undertand the technical aspect, im curious if a user should change the context between loading envs (vs using a dedicated instance per context). In practice the context is a fixed concern.

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Jan 12, 2020

Author Member

In practice, people will figure out what they need :) I considered alternatives before deciding for this design: it looks the best compromise to me.

This comment has been minimized.

Copy link
@ro0NL

ro0NL Jan 12, 2020

Contributor

IMHO it's component design first :) i strongly feel the right design is

$dotenv = new Dotenv('APP_ENV', 'APP_DEBUG', 'dev', ['prod'], ['test']);
$dotenv->load('.env', $putenv = true);
$dotenv->loadAll(['.env'], $putenv = true);

but im fine either way, my only use case is a generated one :)

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Jan 12, 2020

Author Member

I understand, that's what I meant by "compromise". We're not designing from the ground up, and in practice this is the most seamless.

…ling Dotenv::loadEnv()
@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:dotenv-boot branch from 3c3f05a to d66ecbe Jan 12, 2020
@lyrixx
lyrixx approved these changes Jan 14, 2020
@nicolas-grekas

This comment has been minimized.

Copy link
Member Author

nicolas-grekas commented Jan 20, 2020

public function __construct(bool $usePutenv = false)
public function __construct($envKey = 'APP_ENV', string $debugKey = 'APP_DEBUG')
{
if (\in_array($envKey = (string) $envKey, ['1', ''], true)) {

This comment has been minimized.

Copy link
@yceruto

yceruto Jan 20, 2020

Member

is_bool($envKey) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.