drupal-project icon indicating copy to clipboard operation
drupal-project copied to clipboard

Remove phpdotenv or move it to require-dev

Open q0rban opened this issue 7 years ago • 16 comments

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.

q0rban avatar Apr 05 '18 13:04 q0rban

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.

weitzman avatar Apr 05 '18 14:04 weitzman

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.

q0rban avatar Apr 05 '18 14:04 q0rban

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.

weitzman avatar Apr 05 '18 14:04 weitzman

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.

q0rban avatar Apr 05 '18 14:04 q0rban

@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)

webflo avatar Apr 05 '18 17:04 webflo

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.

q0rban avatar Apr 05 '18 17:04 q0rban

@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.

fidelix avatar May 17 '18 09:05 fidelix

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

kevinquillen avatar Feb 20 '19 14:02 kevinquillen

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';
}

Chi-teck avatar Apr 03 '19 08:04 Chi-teck