Closed dnaber-de closed 4 years ago
Let me first describe the reasons this is a breaking change has been introduced.
v2 declared a constant for each env var, v3 declares a constant only for the env variables whose name match WordPress constant name.
The first reason behind the change is that, to be precise v2 declared a constant for each env var that was declared in an env file.
In v3 developers are encouraged don't use env files outside of development ennvironment, but use real environment in production and production-like envs. Which means that there's no way to determine in advance "all" the variables to declare constants for.
That said, assuming we could find a way env variables are always strings, meaning that if WP Starter declares constants for each of them, there could be issue with types.
Let's take the example of a plugin tht expect a config like const MY_FOO = true
and then pass the value of this constant to a function that type-declare bool
. By setting an env variable MY_FOO=true
(MY_FOO=1
) this would be turned into a string causing a type error, and WP Starter would have no way to know that MY_FOO
as to be declared in a constant as boolean.
Assuming we found a way to determine all env variables, how do we exclude those that are not supposed to be used in the web application?
Let's take the example of a system that has an env variable with API keys, and that is converted to a constant. A simple get_defined_constants()
could leak that info.
Yes that is true also in case of WP constants like DB contants, but if I'm a plugin developer and I'm going to output the content of get_defined_constants()
somewhere I know that I have to exclude constants holding DB configuration (and probably also nonces), but I can't know that there are other dangerous constants.
Env variables might be present in the system from several sources, even something that has nothing to do with WordPress. Let's take the example of assets builder that requires an env var DEFAULT_USER
. Then let's assume that a webiste project decides to use a constant named in the exact same way. Normally constants should be prefixed with something unique, but how do we warranty that?
So I don't hink that going back to v2 behavior is doable. If not anything else, the requirement of .env
files instead of real environment was one of the major reasons v3 was written at all.
I would avoid to use composer.json
to define the env var to load, it would be impossible to define constants per environment and honestly not that ideal to maintain.
I am thinking about having a list of "white-listed" env vars in an env variable like:
WP_STARTER_ENV_TO_CONST="MY_PLUGIN_CONSTANT,MY_OTHER_CONSTANT"
and then in wp-config
, after WordPress constants are setup (so after this line: https://github.com/wecodemore/wpstarter/blob/dev/templates/wp-config.php#L36), there could be a code like:
$toDefine = explode(',', $envLoader->read('WP_STARTER_ENV_TO_CONST') ?: '');
foreach ($toDefine as $envVar) {
$envLoader->defineCustomConstant($envVar);
}
where defineCustomConstant()
would be very similar to the current WordPressEnvBridge::defineWpConstant()
.
This workflow would be very flexible, quite safe, and compatible with current workflow and caching system.
The only thing still impossible to fix would be the possibility to store types other than string in constants, something that we can do for WordPress because the expected type of WordPress constants is known (https://github.com/wecodemore/wpstarter/blob/dev/src/Env/WordPressEnvBridge.php#L18-L177).
Maybe we could imagine an optional "type" info to be passed in the env var, e.g.:
WP_STARTER_ENV_TO_CONST="ONE_CONFIG:INT,ANOTHER,YET_ANOTHER,LAST:BOOL"
How this solution look?
Thanks for the detailed elaboration. Your suggestion of a WP_STARTER_ENV_TO_CONST
variable is a good idea. Do you accept a PR or want to implement it on your own?
@dnaber-de I can't work on it next few days, if you have time and will to do it, feel free to send a PR. And thanks!
@dnaber-de I realized this needs more work than originally thought. I will take care of it because I need to be compliant with other changes I'm working on.
@dnaber-de this is implemented in dev
branch. Tests are very welcome.
It still needs documentation, but it is implemented as described above: you need a WP_STARTER_ENV_TO_CONST
env var that contains a comma-separated list of env vars you want to turn into constants.
Tested (without cache) and it works. However, it would be more useful if the constant doesn't get defined in case the env variable is not defined. That way you can fall back to defaults by just comment out a env variable in .env
. Right now I have to comment the variable out and edit a (probably very long) string value of WP_STARTER_ENV_TO_CONST
.
Do you mean if onw of the env variables defined in WP_STARTER_ENV_TO_CONST
is not defined?
In that case no constant is defined.
As you can see here: https://github.com/wecodemore/wpstarter/blob/dev/src/Env/WordPressEnvBridge.php#L613-L619
If reading the env returs null
and that's the case if no env var is set, then define
never happen.
Or I missed something?
My bad. There were still the early hooks file in place that interfere with the constant declaring. Tested with cache and without and it works as expected.
Merged and released in https://github.com/wecodemore/wpstarter/releases/tag/3.0.0-beta.3
Is your feature request related to a problem? Please describe. In WordPress land it is still very common to configure plugins using PHP constants. Version 2 of WPStarter turned every environment variable that extists in .env into a PHP constant. This was quite of convenient IMO. Now I have to write individual code for each and every project just to define some configuration values.
Describe the solution you'd like It would be nice to either go back to the behavior of v2 or provide a way to explicitly name environment variables that should be turned into PHP constants:
Describe alternatives you've considered Right now I would use the early-hooks-file to do it but it requires an additional commit each time I want to add a configuration value for WordPress plugins.