Make WordPress Core

Opened 9 months ago

Last modified 3 months ago

#58855 new enhancement

Fix and enhance wp_is_development_mode() to be able to return whether a development mode is beiung used

Reported by: azaozz's profile azaozz Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch 2nd-opinion dev-feedback
Focuses: Cc:

Description (last modified by oglekler)

Follow up from: https://core.trac.wordpress.org/ticket/57487#comment:71.

The (new in 6.3) wp_is_development_mode() helper function cannot be used to check whether a development mode is enabled or not. It can only check for a particular development mode setting: either plugin, theme or core.

Being able to check whether a development mode is enabled or not will enhance the above function and improve consistency. Currently this can be achieved by using the returned value from wp_get_development_mode() as boolean.

Change History (18)

#2 follow-up: @joemcgill
9 months ago

  • Keywords has-patch added

My PR from the original ticket should still apply, though we need to decide what the default param should be null or ”any”.

#3 follow-up: @SergeyBiryukov
9 months ago

Just to clarify, what would be the difference between:

  • wp_is_development_mode() returning true (or wp_get_development_mode() returning a non-empty value)
  • wp_get_environment_type() returning 'development'
  • WP_DEBUG set to true

This already looks a bit confusing to me :)

#4 @knutsp
9 months ago

In case WP_DEVELOPMENT_MODE is defined as a legal value and WP_ENVIRONMENT_TYPE is undefined WP_ENVIRONMENT_TYPE should be defined to 'development'.

WP_DEBUG may explicitly be defined false, and still WP_ENVIRONMENT_TYPE could be 'development' and WP_DEVELOPMENT_MODE could be either undefined or set to false or an empty value. Debugging is not the same as developing, all the time, and vice versa.

What is the use case wp_i[n|s]_development_mode() to check for any mode? The environment and WP_DEBUG should be sufficient when it's about reporting, logging or checking that may slow down the site. The mode is primarily about caching and avoiding internal behviour that makes certain kinds of development harder, slower or unpredictable. But if one still insist some may need this, if ( wp_get_development_mode() ) {} will work without explicit casting to boolean.

#5 follow-up: @costdev
9 months ago

Some possible configurations that may help clarify the differences in possible usage:

Remote work on a staging site.

Development mode: 'theme'
Environment type: 'staging'
WP_DEBUG: false

An issue that's happening on production but not locally or on a staging site, causing confusion.

Development mode: ''
Environment type: 'production'
WP_DEBUG: true
WP_DEBUG_DISPLAY: false
WP_DEBUG_LOG: true

While development work should always be done locally first, and staging should always be 1:1 with production, we know that's not always the case, nor always available. Having the fine-grained control can be useful.


Regarding:

wp_is_development_mode() returning true (or wp_get_development_mode() returning a non-empty value)

I don't believe there would be a difference. If it's a valid value, wp_get_development_mode() should return that value.

However, we have a snag: '' is a valid value in wp_get_development_mode(). Casting it to boolean would mean wp_is_development_mode( '' ) would always return false, even though explicitly checking for '' should be true based on default-constants.php.

Note the description of wp_is_development_mode():

Checks whether the site is in the given development mode.

(my emphasis)

From what I can tell, these changes to wp_is_development_mode() should suffice:

  • Make $mode optional with a default value of null.
  • Change the first guard to:
    if ( empty( $current_mode ) ) {
        return '' === $mode;
    }
    

See 3v4l.

Last edited 9 months ago by costdev (previous) (diff)

#6 in reply to: ↑ 3 @azaozz
9 months ago

Replying to SergeyBiryukov:

Just to clarify, what would be the difference between:

  • wp_is_development_mode() returning true (or wp_get_development_mode() returning a non-empty value)
  • wp_get_environment_type() returning 'development'
  • WP_DEBUG set to true

This already looks a bit confusing to me :)

Yea, sorry about that. There will be no difference between wp_is_development_mode() (no param) and (bool) wp_get_development_mode() This ticket is just to make wp_is_development_mode() the best way to check whether dev. mode is enabled and a bit more consistent.

Afaik the environment_type refers more to the server settings, etc. not to how WP would handle errors and warnings. WP_DEBUG may be used (and is used) in different cases and different environments. Some sites (without staging?) may perhaps enable it temporarily for debugging/to log errors.

#7 in reply to: ↑ 2 @azaozz
9 months ago

Replying to joemcgill:

My PR from the original ticket should still apply

Yep, thanks, it looks good. Thinking null would be preferable perhaps but any would do.

#8 in reply to: ↑ 5 @azaozz
9 months ago

Replying to costdev:

Some possible configurations that may help clarify the differences

Yep, thanks.

From what I can tell, these changes to wp_is_development_mode() should suffice:

  • Make $mode optional with a default value of null.
  • Change the first guard to:
    if ( empty( $current_mode ) ) {
        return '' === $mode;
    }
    

Yep, or check for null like in the linked PR. Both ways would be fine imho.

#9 @peterwilsoncc
9 months ago

How about using true to check if any development mode is available. That will mean that WP doesn't return true when a falsey value is used. It will also allow for code such as:

<?php
if ( wp_is_development_mode( false ) ) {
  wp_cache_set( 'thing', 'stuff' );
}

With null being used to indicate any mode then the code to do the above seems a little less intuitive:

<?php
if ( ! wp_is_development_mode( null ) ) {
  wp_cache_set( 'thing', 'stuff' );
}

#10 @flixos90
9 months ago

It seems to me we're defining a solution here without defining a problem. Repeating my question from the initial ticket here: What would we use a general development mode for? How would a check for wp_is_development_mode() differ from a check for WP_DEBUG?

Until we can answer this question, I think this change is only for consistency sake with other functions that do different things, and would lead to confusion for which use-cases to rely on development mode vs debug mode.

If we make this change, we need to have a solid rationale what to use it for and most importantly how to differentiate it from WP_DEBUG.

Last edited 9 months ago by flixos90 (previous) (diff)

#11 @oglekler
8 months ago

  • Description modified (diff)
  • Keywords 2nd-opinion added
  • Summary changed from Fix and enhance wp_in_development_mode() to be able to return whether a development mode is beiung used to Fix and enhance wp_is_development_mode() to be able to return whether a development mode is beiung used

This ticket was mentioned in PR #5254 on WordPress/wordpress-develop by @oglekler.


7 months ago
#12

Make one function responsible for getting the development mode state.

Trac ticket: https://core.trac.wordpress.org/ticket/58855

#13 @oglekler
7 months ago

  • Keywords dev-feedback added

This ticket was mentioned in Slack in #core by oglekler. View the logs.


7 months ago

#15 @oglekler
7 months ago

  • Milestone changed from 6.4 to 6.5

This ticket was discussed during a bug scrub,

Because discussion about the solution and purpose is still open, I am moving it into the next milestone.

Add props to @hellofromtonya and @joemcgill

This ticket was mentioned in Slack in #core by chaion07. View the logs.


3 months ago

#17 @chaion07
3 months ago

Hello @azaozz and thanks for reporting this. We reviewed this ticket during a recent bug-scrub session. Based on the feedback received we request the following:

  1. Need a better approach to resolve the issue as suggested by @oglekler previously
  2. @joemcgill needs to address the feedback on the pull request

Cheers!

Props to @mukesh27, @oglekler & @joemcgill

#18 @joemcgill
3 months ago

  • Milestone changed from 6.5 to Future Release

We still need to have a real use case for this enhancement and agreement on the approach. Until then, there's no need to have this in a milestone, so I'm punting from 6.5. We can always bring it back if the need arises and an appropriate approach is determined.

Note: See TracTickets for help on using tickets.