Bug: WP_ENVIRONMENT_TYPE set doesn't check if constant is already defined

[ethanclevenger91]

Terms

• [X] I have read the guidelines for Contributing to Roots Projects
• [X] This request is not a duplicate of an existing issue
• [X] I have read the docs and followed them (if applicable)
• [X] I have seached the Roots Discourse for answers and followed them (if applicable)
• [X] This is not a personal support request that should be posted on the Roots Discourse community

Description

What's wrong?

Some environments (like Local) set the WP_ENVIRONMENT_TYPE constant. Bedrock's implementation should probably check for that to avoid a fatal error.

What have you tried?

Local's prepended PHP file can be updated, but a more bulletproof implementation here is likely preferred.

Possible solutions

if (!defined('WP_ENVIRONMENT_TYPE') && !env('WP_ENVIRONMENT_TYPE') && in_array(WP_ENV, ['production', 'staging', 'development'])) {
    Config::define('WP_ENVIRONMENT_TYPE', WP_ENV);
}

I'm also not sure what the !env('WP_ENVIRONMENT_TYPE') check does in that line, since there's no code to convert that exact env value to the constant value, unless WordPress is doing it under the hood (it might be).

Temporary workarounds

Update Local's config

Steps To Reproduce

1. Install a Bedrock site in Local
2. See fatal error

Expected Behavior

No fatal error

Actual Behavior

Fatal error

Relevant Log Output

No response

Versions

Since PR #668

[swalkinshaw]

What is the actual error?

[ethanclevenger91]

Fatal error: Uncaught Roots\WPConfig\Exceptions\ConstantAlreadyDefinedException: Aborted trying to redefine constant 'WP_ENVIRONMENT_TYPE'. define('WP_ENVIRONMENT_TYPE', ...) has already been occurred elsewhere. in /path-to-local-site/app/public/vendor/roots/wp-config/src/Config.php:106 Stack trace: #0 /path-to-local-site/app/public/vendor/roots/wp-config/src/Config.php(26): Roots\WPConfig\Config::defined('WP_ENVIRONMENT_...') #1 /path-to-local-site/app/public/config/application.php(57): Roots\WPConfig\Config::define('WP_ENVIRONMENT_...', 'development') #2 /path-to-local-site/app/public/web/wp-config.php(8): require_once('/Users/ethancle...') #3 /path-to-local-site/app/public/web/wp/wp-load.php(55): require_once('/Users/ethancle...') #4 /path-to-local-site/app/public/web/wp/wp-admin/admin.php(34): require_once('/Users/ethancle...') #5 /path-to-local-site/app/public/web/wp/wp-admin/index.php(10): require_once('/Users/ethancle...') #6 {main} thrown in /path-to-local-site/app/public/vendor/roots/wp-config/src/Config.php on line 106

Local has this file auto-prepended via PHP to all sites:

<?php
/**
 * This is a PHP script that is auto-added to Local's PHP Lightning Service php.ini's
 * via auto_prepend_script to add relevant constants.
 *
 * @copyright Copyright (c) 2020, WP Engine
 */
define( 'WP_ENVIRONMENT_TYPE', 'local' );

function localwp_auto_login() {
	/**
	 * Do not auto-login if X-Original-Host header is present.
	 *
	 * This prevents auto login from being used over Live Links Pro.
	 */
	if ( !empty( $_SERVER['HTTP_X_ORIGINAL_HOST'] ) ) {
		return;
	}

	if ( empty( $_GET['localwp_auto_login'] ) ) {
		return;
	}

	if ( ! function_exists( 'wp_set_auth_cookie' ) ) {
		return;
	}

	$admin_id = $_GET['localwp_auto_login'];

	$user     = get_user_by( 'id', $admin_id );

	if ( ! is_wp_error( $user ) ) {
		wp_clear_auth_cookie();
		wp_set_current_user( $user->ID );
		wp_set_auth_cookie( $user->ID, false, is_ssl() );

		do_action( 'wp_login', $user->user_login, $user );
		$redirect_to = user_admin_url();
		wp_safe_redirect( $redirect_to );
		exit();
	}
}

$GLOBALS['wp_filter'] = array(
	'init' => array(
		10 => array(
			'localwp_auto_login' => array(
				'function'      => 'localwp_auto_login',
				'accepted_args' => 1,
			),
		),
	),
);

[LetterAfterZ]

We had this issue too, but since this covers the variable in all our use cases, I've just gone and removed our manually setting of this config variable.

However I'm also interested in understanding the thinking behind "!env('WP_ENVIRONMENT_TYPE')"

[yangm97]

It seems like WP_ENV working two jobs isn't cutting anymore. For instance, #668 got merged but that and OP's suggestion are missing the valid local environment type, not to say wordpress already does its own validation and falls back to production automatically.

May I suggest the deprecation of WP_ENV in favor of WP_ENVIRONMENT_TYPE (native) and WP_ENVIRONMENT_NAME (new)? Migration path could be as simple as follows:

define('WP_ENVIRONMENT_TYPE', env('WP_ENVIRONMENT_TYPE') ?: 'production');
define('WP_ENVIRONMENT_NAME', env('WP_ENVIRONMENT_NAME') ?: WP_ENVIRONMENT_TYPE);
define('WP_ENV', env('WP_ENV') ?: WP_ENVIRONMENT_NAME);

[swalkinshaw]

1. local support was fixed
2. I agree, just checking !env('WP_ENVIRONMENT_TYPE') seems incomplete

The intent was for Bedrock to only set WP_ENVIRONMENT_TYPE when it hasn't been manually defined. To put it another way: map WP_ENV to WP_ENVIRONMENT_TYPE with the equivalent value.

I'd prefer to just fix this issue before re-evaluating WP_ENV at least. Is anyone interested in submitting a PR?

I think the solution should only check !defined('WP_ENVIRONMENT_TYPE') and even just ignore the env check (since the result is defining a constant).

[huubl]

Maybe relevant to know:

Wordpress 6.3 will introduce a new WP_DEVELOPMENT_MODE constant.

https://make.wordpress.org/core/2023/07/14/configuring-development-mode-in-6-3/