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.