wecodemore / wpstarter

Easily bootstrap whole site Composer packages for WordPress.
https://wecodemore.github.io/wpstarter/
MIT License
244 stars 34 forks source link

Env loading/overriding/naming #131

Closed nlemoine closed 1 year ago

nlemoine commented 1 year ago

Hi @gmazzap,

First, thanks a lot for this project. I’ve bookmarked since quite a long time and never really paid attention. I thought it was just another Bedrock like stack and came back to it recently. And it actually reaches some stratospheric levels of awesomeness I totally missed. I must say I’m really blown by your work in various WordPress fields and how you manage to create order from chaos 👏👏👏

So I started to play around with WP Starter and as a user of symfony/dotenv library, I think env loading could be managed a bit differently. Since v3 is still in beta, I’m taking my chance to change this while BC breaks are still allowed ;) Here’s a detailed explanation.

A related request has been made by @lkraav in #122.

I’ll try to advocate against local term as valid environment type for multiple reasons.

Semantically ambiguous

local is an ambiguous word. It can mean developing locally (as opposed to something that is « remote »). The issue is that it can also be understood as the « current » environment, e.g. where the project is currenlty running, no matter the environment is (production, staging, etc.).

Standard

Although I don’t have strong opinion on which meaning is the right one, it seems like the second one is more widely used and established than the first, and tends to be more « standard/predictable/familiar ».

Symfony is using env overriding via local: https://symfony.com/doc/current/configuration.html#overriding-environment-values-via-env-local NextJS is also using the same kind of overriding via ’local: https://nextjs.org/docs/pages/building-your-application/configuring/environment-variables#default-environment-variables

This kind of env loading order feels also more fluent, you can commit a default .env file that can be overridden by a .env.local, but that’s a personal point of view.

Leverage symfony/dotenv features

symfony/dotenv already provides some features that are implemented in WordPressEnvBridge.

/**
 * .env                            contains default values for the environment variables needed by the app
 * .env.local                      uncommitted file with local overrides
 * .env.$WP_ENVIRONMENT_TYPE       committed environment-specific defaults
 * .env.$WP_ENVIRONMENT_TYPE.local uncommitted environment-specific overrides
 */
(new Dotenv('WP_ENVIRONMENT_TYPE', 'WP_DEBUG'))
    ->setProdEnvs(['production', 'prod', 'live', 'public'])
    ->usePutenv()
    ->bootEnv('path/to/.env', 'development');

Will take care of loading overrides, filling global vars ($_SERVER, etc.) and cached PHP file.

It would simplify code and maintenance and rely a solid standard and library.


Unfortunately, this would introduce a breaking change and would require to:

I've read in another issue that it was your intention but I'm still giving it a shot 🙂

gmazzap commented 1 year ago

Hi @nlemoine, and thank you very much for yout feedback.

I’ll try to advocate against local term as valid environment type for multiple reasons.

This is a term that WordPress uses. See https://github.com/WordPress/WordPress/blob/a3398d1cc4fa55ab460299f67949b16eaee7ba0d/wp-includes/load.php#L223

Yes, it has ambiguity, but in the WordPress space is now used with the meaning of the "local machine". Using a different name would make it more confusing for WordPress developers, who ultimately are the ones who will consume this package. Even because the WP Starter environment is mapped to WordPress WP_ENVIRONMENT_TYPE and I think it is not a good idea to map "something else" to WordPress' "local" (which is hardcoded in WP).

Symfony/dotenv already provides some features that are implemented in WordPressEnvBridge.

TBH, I prefer to keep control over it. First of all, I already changed once the library used for parsing env files, and I can not exclude it again in the future, so I would like to avoid being too coupled to Symfony's Dotenv. Even because Symfony components tend to have a lot of breaking changes across major versions, and I already had sites breaking because of a BC change in Symfony. Moreover, the approach of WP Starter is to promote the usage of real environment variables and not dot-env files, which should be used when developing locally. Finally, even if v3 is in beta, there are already a lot of sites using it, even in production. So a breaking change for v3 is not really possible.

nlemoine commented 1 year ago

Thanks for your feedback @gmazzap. I totally understand your motivation over this. Just a couple of notes/questions before closing.

This is a term that WordPress uses

Indeed. However, WordPress does not make any assumption on the behavior inferred from choosing local. Should development also not triggering env file caching? I find a bit weird that when set to development, env vars are cached (I'm fully aware this is configurable with a filter :))

Moreover, I am wondering why development ≠≠≠ local? The WP_LOCAL_DEV isn't defined anywhere in WordPress.

So a breaking change for v3 is not really possible.

Yes, this makes sense 👍

gmazzap commented 1 year ago

I find a bit weird that when set to development, env vars are cached (I'm fully aware this is configurable with a filter

In my experience a "development" environment is an environment used to test behavior, before it reaches the "staging" status. But in such environment you probably want to see a production-like behavior. So if env vars caching is causing issues, you probably want to spot those issues in a development environment.

This is, again, based on my experience, and as you said, there are several way of preventing cache. You could do that via the cache-env configuration, so no need to write code for that.

That said, WP recently introduced WP_DEVELOPMENT_MODE and one of the explicit reasons is to avoid caching wen that is not empty, plus specific behavior depending on the type of development. I've just added support for that constant and when that constant is set to any non-empty value there's no env vars cache.

Moreover, I am wondering why development

Well, first of all WordPress defines "local" and "development" environments names. In my experience, "local" is a developer's local machine, a "development" environment is an online environment (on the hosting servers) where developers test their code before moving to a "staging" environment, where usually non-developers (clients, testers, etc) can do their testing. This is for example how wp.com VIP hosting defines them.

But again, I set the default based on my experience, all of that is very deeply configurable, it has never been an issue.

nlemoine commented 1 year ago

Thanks for your detailed feedback.

The whole thing relies on the ambiguity of the local keyword. In my practice, the development stage is your local stage whereas local defines the current env (whether it is online staging/prod, dev computer, etc.).

all of that is very deeply configurable

WPStarter is indeed flexible enough to handle both env naming convention / types of env hierarchy loading.