Remove phpdotenv or move it to require-dev

[q0rban]

For performance reasons, it's not a good idea to use phpdotenv in production. And especially if the idea of not using it in production is "just don't have a .env, but use phpdotenv to search for one anyway." This choice seems like a choice that an individual project should make, not one that drupal-project should make for everyone using it as a starterkit. And if you do want to use it, it should IMO be a require-dev package, not something that ever makes it to production.

[weitzman]

The point of a starter kit IMO is that you show people good practices and you expect them to customize it. drupal-project is not forcing any choices on anyone. This is a recurring misconception or disagreement.

[q0rban]

The way it's added, it's quite tricky to remove phpdotenv from this project. It's not as simple as composer remove vlucas/phpdotenv.

[weitzman]

It can be done by commenting out https://github.com/drupal-composer/drupal-project/blob/8.x/composer.json#L44. I agree that we could document this better.

[q0rban]

It's actually not as simple as just that either. For the record, in case anyone is reading this that wants to remove it, here are the steps:

1. composer remove vlucas/phpdotenv
2. rm load.environment.php .env.example
3. Remove "files": ["load.environment.php"] from composer.json
4. Run composer dump-autoload to regenerate the autoloader.
5. Run composer update --lock to update the lock hash, since composer.json was manually edited.

[webflo]

@q0rban Thanks for the pull-request. I will think about it, especially because of the long discussions file_exists we had in Drupal core.

There is a shorter version for removing it.

1. rm load.environment.php .env.example
2. Remove "files": ["load.environment.php"] from composer.json
3. composer remove vlucas/phpdotenv (Removes the packages, updates autoloader and lock hash)

[q0rban]

Nice, that's much better @webflo! I do think that is unlikely to be people's first thought when removing it, unless they are very familiar with how Composer works.

[fidelix]

@webflo your 3 steps don't work. I get a fatal error on step 3:

 PHP Warning:  require(/zzz/vendor/composer/../../load.environment.php): failed to open stream: No such file or directory in /zzz/vendor/co  
  mposer/autoload_real.php on line 66 

By the way, I support removing this package from the project, or at least moving it to require-dev. The way it is now is just too opinionated.

[kevinquillen]

Agree. If its not necessary to running Drupal OOTB, it should be listed as require-dev at the least.

[Chi-teck]

Per this comment require-dev is a proper location for this package.

Production environments rarely use .env files.

https://github.com/drupal-composer/drupal-project/blob/8.x/load.environment.php#L19

Worth noting that it is not a big deal to load environment variables without vlucas/phpdotenv package.

<?php

/**
 * This file is included very early. See autoload.files in composer.json and
 * https://getcomposer.org/doc/04-schema.md#files
 */

putenv('DRUSH_OPTIONS_URI=http://localhost');

/**
 * Load local environment, if available.
 */
if (file_exists(__DIR__. '/local.environment.php')) {
  require __DIR__. '/local.environment.php';
}

[AlexSkrypnyk]

Moving it to require-dev is not a good option from the workflow POV: you would have a different way how the app consumes env variables based on where the app is deployed.

I think we should decide on whether we leave it as-is or fully remove it.

[deviantintegral]

Since this was filed, I think there’s a new common case for using this package in require: running functional tests in a development environment like ddev, but where you aren’t installing any development dependencies or disabling theme caching. We do this with an environment variable like DDEV_PRODUCTION_MODE that’s pulled in via the .env file.

[leymannx]

I think it should stay but we could make it easier for people removing it.

[AlexSkrypnyk]

Let's only document how to remove it. If we get to building a customiser for this project - we can add this removal there.

[leymannx]

We could put a class exist in the load.environment.php right? Then it's just composer remove.

[AlexSkrypnyk]

I would like to understand how/if people use this with Drupal (workflow-wise). We do not provide any documentation for this, so others may have the same question.

When using a container-based environment (locally, CI, hosting), the reading of variables from .env and then the injection of the variables into PHP process is usually done by the container configurations. This means that this functionality is duplicated (and actually more confusing, since there are now two method of injecting variables).

On the environments where reading of .env is not supported, the variables are set per-environment explicitly on a platform level.

In addition, there are also performance concerns. Other frameworks solving them by caching environment variables loaded by dotenv within own code. Do we have the same caching in Drupal?