#57487 closed enhancement (fixed)
Add support for "development mode"
Reported by: | azaozz | Owned by: | flixos90 |
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | has-unit-tests has-dev-note needs-patch |
Focuses: | Cc: |
Description
The idea to have a dedicated "development mode" has been around for a while. Previously the consideration was that having WP_DEBUG
and SCRIPT_DEBUG
set to true
is enough. However it seems that's not the case any more.
For example lately the WordPress code is more and more aware of different methods and levels of caching. Having a development mode would make it easier/clearer when to use these caches and when to reset them.
Another nice use for a development mode would be to output more, and more verbose errors and notices specifically targeted to "make the developers life easier".
Change History (72)
#2
follow-up:
↓ 5
@
23 months ago
Seems the most straightforward way would be to add another constant similarly to WP_DEBUG
and SCRIPT_DEBUG
. Then, when setting the new constant perhaps the code can also look at WP_ENVIRONMENT_TYPE
or at environment vars. That would make it even moire convenient to set in some cases.
The new constant should probably set both WP_DEBUG
and SCRIPT_DEBUG
to true, and also WP_DEBUG_LOG
and WP_DEBUG_DISPLAY
. Of course it would still be possible to override any of the existing constants. That would make it possible to fine-tune the behaviour.
#3
in reply to:
↑ 1
;
follow-up:
↓ 8
@
23 months ago
Replying to afragen:
Yea, this is somewhat similar to setting WP_ENVIRONMENT_TYPE
to development
. However the "environment types" functionality doesn't seem to be particularly useful for setting development mode. It seems unclear and harder to use, and buggy in some cases. The setenv
and getenv
functions don't seem to be enabled or to work reliably in some cases.
Another very big disadvantage there is that setting WP_ENVIRONMENT_TYPE to 'local' means it cannot be set to 'development' and vice-versa. Unfortunately that makes the whole functionality not particularly useful.
#4
follow-up:
↓ 6
@
23 months ago
Also the WP_ENVIRONMENT_TYPE constant was deprecated
What do you mean by this?
#5
in reply to:
↑ 2
;
follow-up:
↓ 7
@
23 months ago
Replying to azaozz:
The new constant should probably set both
WP_DEBUG
andSCRIPT_DEBUG
to true, and alsoWP_DEBUG_LOG
andWP_DEBUG_DISPLAY
. Of course it would still be possible to override any of the existing constants. That would make it possible to fine-tune the behaviour.
It's worth noting that 'development' === wp_get_environment_type()
already sets WP_DEBUG to true as of [48372] / #33161, perhaps SCRIPT_DEBUG
should be added there too?
Just curious, is there a use case where setting WP_ENVIRONMENT_TYPE
to development
is not enough and a new constant is needed?
Also the
WP_ENVIRONMENT_TYPE
constant was deprecated, but is still needed/required in these cases which makes it... weird :)
No, it was the WP_ENVIRONMENT_TYPES
constant (note the plural name) that was deprecated in [48896] / #50992.
The WP_ENVIRONMENT_TYPE
constant is not deprecated and can be used exactly when getenv()
is not enabled or setting an environment variable is not preferable for some other reason.
#6
in reply to:
↑ 4
@
23 months ago
Replying to TimothyBlynJacobs:
Uh, sorry, edited the above comment to fix it.
#7
in reply to:
↑ 5
@
23 months ago
Replying to SergeyBiryukov:
Replying to azaozz:
The new constant should probably set both
WP_DEBUG
andSCRIPT_DEBUG
to true, and alsoWP_DEBUG_LOG
andWP_DEBUG_DISPLAY
. Of course it would still be possible to override any of the existing constants. That would make it possible to fine-tune the behaviour.
It's worth noting that
'development' === wp_get_environment_type()
already sets WP_DEBUG to true as of [48372] / #33161, perhapsSCRIPT_DEBUG
should be added there too?
Yes, think so.
Just curious, is there a use case where setting
WP_ENVIRONMENT_TYPE
todevelopment
is not enough and a new constant is needed?
Unfortunately WP_ENVIRONMENT_TYPE
can only be set to either 'local' or 'development'. What if both of these are true/needed? Same for 'staging' and 'development', or 'local' and 'production'. What if I want to test production on a local environment? This makes the whole functionality there pretty confusing and useless imho :(
#8
in reply to:
↑ 3
@
23 months ago
Replying to azaozz:
Another very big disadvantage there is that setting WP_ENVIRONMENT_TYPE to 'local' means it cannot be set to 'development' and vice-versa. That makes the whole functionality pretty useless.
I see, thanks! There was a discussion in #51064 on the differences between local
and development
, perhaps it should be revisited.
#9
@
23 months ago
My preference is to improve the environment type defaults if required. I thinking adding something new would be confusing.
Instead of checking for the development
environment WordPress could use in_array( wp_get_environment_type(), array( 'development', 'local' ), true )
.
For staging and production environments requiring features of the default development variables, I think it reasonable to require they continue to be set individually per the current requirements.
#10
@
23 months ago
There was a discussion in #51064 on the differences between local and development, perhaps it should be revisited.
Yes, but unsure if this problem can be resolved by trying to figure out the differences or if there should be some. Truth is that anybody can develop or test on both local and remote WP installs. Don't see why this setting should be limiting.
Instead of checking for the development environment WordPress could use in_array( wp_get_environment_type(), array( 'development', 'local' ), true ).
True, but then WP will end up with all sorts of 'development' AND 'local'
, 'development' OR 'local'
, 'development' AND 'staging'
, etc. Why the complexity?
The problem with wp_get_environment_type()
is that the types are not mutually-exclusive. Also, the intended use of the environment types seem more like "server settings", not "user intent". I.e. should a user be able to set local
when WP is on a remote server? Should that affect/interfere with development mode?
Those are probably the reasons why wp_get_environment_type() is used so little in core.
#11
follow-up:
↓ 15
@
23 months ago
This has become a mess, but can be kept without a lot of BC breaks this way:
For wp_get_environment_type()
add/allow a 'local ' prefix in WP_ENVIRONMENT_TYPE
:
[
'local', // deprecate later, return as 'local development'
'local development',
'local staging',
'local production'.
'development',
'staging',
'production' // default
]
Maybe also:
Introduce WP_LOCAL_DEV
, which forces the 'local ' prefix (in case not already set).
In case 'local ' is included and WP_LOCAL DEV
is undefined, define WP_LOCAL DEV
as true.
In case 'local ' is no included and WP_LOCAL DEV
is undefined, define WP_LOCAL DEV
as false.
Finally:
if ( str_ends_with( wp_get_environment_type(), 'development' ) ) {
//set other development related constants not set earlier
}
Recommendations for use:
- To check for local or remote, use constant
WP_LOCAL DEV
(preferred) orstr_starts_with
(possible, giving same result) - To check for legacy environment type only, also in case on local, use
str_ends_with
(edge case) - The common check
if ( 'production' !== wp_get_environment_type() )
on remote environments will then work as before. 'local production' is new and a very edge case, maybe useful on intranets.
#12
follow-ups:
↓ 13
↓ 24
@
23 months ago
Spotting this ticket with some delay, I have provided the more concrete use-case that at least partially led to this ticket in #57573, which is maybe a duplicate, maybe not (depending on approach to take).
Reading through the conversation here, it seems we are mixing up environment and development mode, which I think is diverging a bit from the original intent. Yes, there is some overlap: WP_ENVIRONMENT_TYPE
is for the environment, and to a degree it's possible to infer or guess certain things on the development mode. For example, if WP_ENVIRONMENT_TYPE
is "development" or "local", it is likely that this environment is being used for development. However just likely, not guaranteed, as @azaozz has pointed out with an example above. So I think we should decouple the two topics - development mode and environment type are not the same.
For example, when you develop, you may be developing a plugin, or a theme, or contribute to core. Or maybe even something a bit more obscure like develop a drop-in. [55138], [55148], and [55155] are great examples for something where we need a way to signify whether a theme is being developed or not, because only if a theme is being developed the code should behave differently and not cache. If a plugin is being developed for example, the code should behave the same it would on a regular production site and use the cache.
This leads me to think we need something closer to maybe a WP_DEVELOPMENT_MODE
constant, which for example could be set to something like core
, plugin
, theme
. By default it could be either not set, or an empty string, signifying that no particular development mode at all should be used.
Curious about your thoughts on this.
#13
in reply to:
↑ 12
@
22 months ago
Replying to flixos90:
Reading through the conversation here, it seems we are mixing up environment and development mode
Yes, exactly my thoughts! You explained it better :)
This leads me to think we need something closer to maybe a
WP_DEVELOPMENT_MODE
constant, which for example could be set to something likecore
,plugin
,theme
. By default it could be either not set, or an empty string, signifying that no particular development mode at all should be used.
Yep, also thinking a constant would be best. Would probably be good to have it always set, like WP_DEBUG
, as presumably WP_DEVELOPMENT_MODE
will make WP a lot "noisier", even probably throwing fatal errors with backtrace when needed, etc. It can work with core
, plugin
, theme
, false
so checking it could be:
if ( WP_DEVELOPMENT_MODE ) { ...
if ( 'theme' === WP_DEVELOPMENT_MODE ) { ...
if ( WP_DEVELOPMENT_MODE && 'core' !== WP_DEVELOPMENT_MODE ) { ...
etc.
#14
@
22 months ago
@azaozz That sounds great to me.
Though I would suggest we wrap this constant check in a function that also has a filter so that we can control it at runtime as well (critical for unit tests for example).
Basically like this:
function wp_development_mode() { $development_mode = defined( 'WP_DEVELOPMENT_MODE' ) ? WP_DEVELOPMENT_MODE : false; return apply_filters( 'wp_development_mode', $development_mode ); }
#15
in reply to:
↑ 11
;
follow-up:
↓ 16
@
22 months ago
Replying to knutsp:
For
wp_get_environment_type()
add/allow a 'local ' prefix inWP_ENVIRONMENT_TYPE
:
[ 'local', // deprecate later, return as 'local development' 'local development', 'local staging', 'local production'. 'development', 'staging', 'production' // default ]
Something like this was my initial thinking too, but it looks like a separate function might be more straightforward to use in the long run, and would also allow for differentiating between core, plugin, and theme development.
Replying to flixos90:
Basically like this:
function wp_development_mode() { $development_mode = defined( 'WP_DEVELOPMENT_MODE' ) ? WP_DEVELOPMENT_MODE : false; return apply_filters( 'wp_development_mode', $development_mode ); }
This looks good to me :) Should we also check for an environment variable if set, or would the constant and the filter be enough for now?
#16
in reply to:
↑ 15
@
22 months ago
- Keywords needs-patch needs-unit-tests added
Replying to SergeyBiryukov:
function wp_development_mode() { $development_mode = defined( 'WP_DEVELOPMENT_MODE' ) ? WP_DEVELOPMENT_MODE : false; return apply_filters( 'wp_development_mode', $development_mode ); }This looks good to me :) Should we also check for an environment variable if set, or would the constant and the filter be enough for now?
I think we could do it without an environment variable for now, since e.g. WP_DEBUG
doesn't have an environment variable either. WP_ENVIRONMENT_TYPE
supports an environment variable, but arguably that also makes more sense there than here, since the environment type is, even per the naming, directly tied to the environment, whereas the development mode is not.
Something else I'd just like to note is that we should probably validate in the eventual code that the value from the constant/filter is one of the allowed values; didn't include it in this mini code example, but later in the PR I think we should have it.
This ticket was mentioned in PR #4565 on WordPress/wordpress-develop by @flixos90.
18 months ago
#17
- Keywords has-patch added; needs-patch removed
This PR implements a proposed path forward for the below ticket and also applies it in a first usage, replacing the somewhat misused WP_DEBUG
lookups to modify theme.json caching behavior.
Trac ticket: https://core.trac.wordpress.org/ticket/57487
#18
follow-up:
↓ 19
@
18 months ago
- Milestone changed from Future Release to 6.3
- Owner set to flixos90
- Status changed from new to assigned
@azaozz @SergeyBiryukov I've implemented this in a first pass in https://github.com/WordPress/wordpress-develop/pull/4565. It would be great if you could take a look.
If we want to move forward with this solution, I can add test coverage. I think it would be great if we could get this into WordPress 6.3 to also get rid of the todo
comments around "incorrect" WP_DEBUG
usage (see related #57573).
#19
in reply to:
↑ 18
@
18 months ago
Replying to flixos90:
I've implemented this in a first pass in https://github.com/WordPress/wordpress-develop/pull/4565.
Thanks! The PR looks good imho perhaps with exception of the wp_development_mode
filter (that filters the constant). This type of filters don't seem to be working particularly well and thinking it would be better without them.
Also seems it would be good to set WP_DEBUG
, WP_DEBUG_DISPLAY
, (maybe WP_DEBUG_LOG
if the WP_DEBUG_DISPLAY
is off), SCRIPT_DEBUG
, etc. when development mode is on. I'll comment on the PR.
Any other suggestions and/or requests for "bells and wishes" for the development mode (things that many developers use and likely every developer would find useful)?
18 months ago
#20
@felixarntz Thanks for the PR!
Wondering if WP_DEBUG
, SCRIPT_DEBUG
, etc. constants should also be set when setting WP_DEVELOPMENT_MODE
? Seems they should be? Also, perhaps if WP_DEVELOPMENT_MODE
is set and WP_DEBUG_DISPLAY
is false
, can set WP_DEBUG_LOG
to true
?
Any other "shortcuts" that may make sense to be set for dev. mode?
@flixos90 commented on PR #4565:
18 months ago
#21
@azaozz
Wondering if
WP_DEBUG
,SCRIPT_DEBUG
, etc. constants should also be set when settingWP_DEVELOPMENT_MODE
? Seems they should be? Also, perhaps ifWP_DEVELOPMENT_MODE
is set andWP_DEBUG_DISPLAY
isfalse
, can setWP_DEBUG_LOG
totrue
?
I think for WP_DEBUG
that would be reasonable, since it is already conditionally set based on the environment type, and it's clear that any development mode should also result in WP_DEBUG
enabled by default, as it is recommended to develop with debugging on. I've updated that in https://github.com/WordPress/wordpress-develop/pull/4565/commits/093d74b535232d8dcc88778b8b90f4c02b27093a.
For the other suggestions, I would suggest to handle that separately since those would be changes that require more discussion. Any of those changes we could have also made e.g. during introduction of WP_ENVIRONMENT_TYPE
, but we didn't, so I think that should be decoupled.
@flixos90 commented on PR #4565:
18 months ago
#22
@azaozz
Wondering if
WP_DEBUG
,SCRIPT_DEBUG
, etc. constants should also be set when settingWP_DEVELOPMENT_MODE
? Seems they should be? Also, perhaps ifWP_DEVELOPMENT_MODE
is set andWP_DEBUG_DISPLAY
isfalse
, can setWP_DEBUG_LOG
totrue
?
I think for WP_DEBUG
that would be reasonable, since it is already conditionally set based on the environment type, and it's clear that any development mode should also result in WP_DEBUG
enabled by default, as it is recommended to develop with debugging on. I've updated that in https://github.com/WordPress/wordpress-develop/pull/4565/commits/093d74b535232d8dcc88778b8b90f4c02b27093a.
For the other suggestions, I would suggest to handle that separately since those would be changes that require more discussion. Any of those changes we could have also made e.g. during introduction of WP_ENVIRONMENT_TYPE
, but we didn't, so I think that should be decoupled.
#23
@
18 months ago
- Keywords has-unit-tests added; needs-unit-tests removed
@azaozz @SergeyBiryukov @desrosj @spacedmonkey It would be great if some of you could review https://github.com/WordPress/wordpress-develop/pull/4565. We're a few days away from Beta, and it would be great if we could get this in.
#24
in reply to:
↑ 12
;
follow-up:
↓ 26
@
18 months ago
Replying to flixos90:
This leads me to think we need something closer to maybe a
WP_DEVELOPMENT_MODE
constant, which for example could be set to something likecore
,plugin
,theme
. By default it could be either not set, or an empty string, signifying that no particular development mode at all should be used.
Sorry, I missed this at the time.
If different development modes are to be defined, then site
would be helpful for agencies developing sites using both custom themes and custom plugins.
However, I fear that would lead to the problem perceived to exist with the environment type variables, the need to check for multiple values.
If the goal is to avoid a constant that over complicates things then using true
or false
seems the least complicated way of doing it.
#25
@
18 months ago
Just reading through this, I think that the proposed solution of having a "development mode" is not a good approach and is so vague that it will most certainly lead to a lot of confusion and issues down the road.
What if I want to "develop", but I need caches to see the scaling behavior? What if I want to disable caching on a production server?
My suggestion would be to stick to the behavior that should be controlled, not to an overarching names that might make perfect sense with a full product, but quickly gets muddy with something that is used as a development framework by a lot of folks.
How about having a constant WP_USE_INTERNAL_CACHES
that defaults to ! WP_DEBUG
(NOT) instead?
You could even go as far as having it be full enable/disable with true
/false
or even allowing a comma-separated list of types of caches to use for more granular control (only if needed).
#26
in reply to:
↑ 24
;
follow-up:
↓ 45
@
18 months ago
Replying to peterwilsoncc:
If different development modes are to be defined, then
site
would be helpful for agencies developing sites using both custom themes and custom plugins.
Good point, there may be cases where both are being developed. I think core
, plugin
, and theme
are the natural first choices, but happy to explore adding more. I think this could happen either as part of an initial commit or as a follow-up.
If the goal is to avoid a constant that over complicates things then using
true
orfalse
seems the least complicated way of doing it.
That would not satisfy the requirements though, as developing a theme has different implications than developing core for example.
Replying to schlessera:
Just reading through this, I think that the proposed solution of having a "development mode" is not a good approach and is so vague that it will most certainly lead to a lot of confusion and issues down the road.
What if I want to "develop", but I need caches to see the scaling behavior? What if I want to disable caching on a production server?
That's why the development mode is not just a true/false consideration, it is more granular than that. If you are developing a theme, the scaling behavior of core caches is "irrelevant", as only core controls them. If you are developing core, the caches would be enabled - the granularity is why this concept came up in the first place. The example you outlined is precisely what this development mode concept would address.
The relevant caches are around file-based logic that is reasonable to cache in regular WP usage, but not when developing and possibly changing those files every minute or so. For example: theme.json
-based logic can easily be cached as long as you're not currently working on that theme, as it would only possibly change during a theme update. However, if you're currently working on that theme's code, the caches should be disabled to avoid messing up your workflow. So you would set WP_DEVELOPMENT_MODE
to "theme". When you're directly working on core though, that doesn't apply to you.
@flixos90 commented on PR #4565:
18 months ago
#28
Committed via https://core.trac.wordpress.org/changeset/56042 🎉
#29
@
18 months ago
There are potentially some follow-up enhancements to make here following the commit, let's keep discussing and open new tickets as needed.
With this being the solution for #57573 which wasn't clear from the beginning, I'm gonna close that ticket as a duplicate now.
@oandregal commented on PR #4565:
18 months ago
#33
@flixos90 commented on PR #4565:
18 months ago
#34
@oandregal Interesting. This change by itself is 100% not impacting performance. So question is why did the number change so much?
My first hunch is that we may be running the performance tests with WP_DEBUG enabled?? If yes, that would explain it, since before this change WP_DEBUG prevented caching on several block theme relevant functions, which was always something we wanted to change, and did here. So basically now the caches are enabled also if WP_DEBUG is enabled, so that may make things _seem_ faster in the metrics, but really there's no difference. Those are effectively the WP 6.2 performance wins only showing up now 😁
Would be good to validate that the above is in fact the case. Are we running performance tests with WP_DEBUG?
#35
@
18 months ago
What if I am working on a theme and a companion plugin for it (to add custom post types)? Would I set WP_DEVELOPMENT_MODE
to theme
or plugin
?
#36
@
18 months ago
This should be a string or an array of "modes", wp_development_mode()
should be wp_development_mode( $mode = null )
and return array|bool (array when argument $mode is null, bool otherwise).
Array constants can be set with const from PHP 5.6. From PHP 7 array constants can also be defined runtime. Let us set this as an array from the beginning, even if core will only define and use a string for 6.3.
When 7.0 is minimum, core may start setting and using it as an array.
@oandregal commented on PR #4565:
18 months ago
#37
Looking at the CI job, I can learn that ImageMagic supports 247 formats, though I am unable to find the values of WP_DEBUG
or SCRIPT_DEBUG
.
The performance workflow doesn't seem to set anything different from the defaults, hence, it sounds WP_DEBUG
and SCRIPTS_DEBUG
would be true.
I'd appreciate if someone with more experience in GitHub CI/wp-env than I have could double-check this.
@oandregal commented on PR #4565:
18 months ago
#40
Actually, scratch what I said above: I thought WordPress core used wp-env
but it uses a totally different mechanism. It seems to use this `.env` file that sets WP_DEBUG
and SCRIPTS_DEBUG
to true.
18 months ago
#41
but it uses a totally different mechanism. It seems to use this .env file that sets WP_DEBUG and SCRIPTS_DEBUG to true.
The values from that .env file are only used for the initial install (afaik). The actual wp-config.php
file is in your work/test dir, can tweak it as you want :)
@flixos90 commented on PR #4565:
18 months ago
#42
@oandregal Interesting, I just spotted that the numbers on https://www.codevitals.run/project/wordpress all went back up again a few commits later. Any idea why? Maybe that wasn't related to this commit at all? 🤔
@oandregal commented on PR #4565:
18 months ago
#43
@felixarntz note there were two drops and one uptick. The metrics haven't recovered from the first drop and it points to this PR:
I think it's related. Both core and Gutenberg had WP_DEBUG
to true
in the performance tests as far as I can see. I'm fixing Gutenberg in https://github.com/WordPress/gutenberg/pull/52016 but I don't know how to fix core's.
@flixos90 commented on PR #4565:
18 months ago
#44
@oandregal Ah my bad, thanks for clarifying, I somehow missed that there were 2 drops like that.
#45
in reply to:
↑ 26
;
follow-up:
↓ 46
@
17 months ago
The origin behind this ticket is that environment types defined in Core are not mutually exclusive and have lead to problems. I think that as implimented this code has the same problem.
Code will need to check multiple development modes (theme|plugin|core) which Ozz identified in comment #10 as problematic for environment types.
As this is going in to core, introducing the problem it is trying to solve is inadequate.
I like @knutsp's suggestion in comment #36 of passing a $mode
parameter and returning a boolean if it matches either exactly or by definition.
Replying to flixos90:
Good point, there may be cases where both are being developed. I think
core
,plugin
, andtheme
are the natural first choices, but happy to explore adding more. I think this could happen either as part of an initial commit or as a follow-up.
I think missing on site
(or whatever it ends up being called) is to miss the most common development mode for WordPress. In my experience at both small and large agencies, site wide development is the norm and exclusively developing either a theme or plugin is an exception.
#46
in reply to:
↑ 45
@
17 months ago
Replying to peterwilsoncc:
I like @knutsp's suggestion in comment #36 of passing a
$mode
parameter and returning a boolean if it matches either exactly or by definition.
That sounds like a good addition to me, rather than an alternative. Right now, we have wp_get_development_mode()
. How about another function wp_in_development_mode( $mode )
to make such usage more trivial/intuitive?
I think missing on
site
(or whatever it ends up being called) is to miss the most common development mode for WordPress. In my experience at both small and large agencies, site wide development is the norm and exclusively developing either a theme or plugin is an exception.
I agree with you, though I'm a bit hesistant on the name to give this. Something I thought about yesterday is: How about we instead introduce an "all" mode, which effectively is the same as "core and plugin and theme"? I find that a more intuitive name, and while e.g. an agency doesn't necessarily touch WordPress core, even that may happen sometimes. For someone that controls the entire site, maybe an "all" is the most suitable approach and feels like like a less opinionated name than "site" IMO.
Potentially "all" could even become sort of a default (e.g. is a site only sets WP_DEBUG
to true
but doesn't set WP_DEVELOPMENT_MODE
, default to WP_DEVELOPMENT_MODE=all
).
Worth noting that for someone setting the development mode to "all" this wouldn't provide any benefit over the previous workaround of using WP_DEBUG
(since at that point it's the same "all or nothing" switch), but even then the possibility of granularity (which is especially crucial for core, plugin, and theme developers that don't control the entire stack) is the benefit of this change.
#47
@
17 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
@flixos90 The additional function makes sense, so to summarize:
WP_DEVELOPMENT_MODE value | wp_in_development_mode === true |
---|---|
all | theme, plugin, core |
theme | theme |
plugin | plugin |
core | core |
If it's determined that a site
mode is required at a latter date, it would return true for theme, plugin
This ticket was mentioned in PR #4838 on WordPress/wordpress-develop by @flixos90.
17 months ago
#48
This PR implements the enhancements discussed on the ticket:
- New utility function
wp_in_development_mode()
that simplifies checking for a specific development mode. With this change,wp_get_development_mode()
becomes a more low-level function. - New possible value 'all' for
WP_DEVELOPMENT_MODE
constant. This value effectively means all 3 other development modes are enabled.
Trac ticket: https://core.trac.wordpress.org/ticket/57487
#49
@
17 months ago
Thanks @peterwilsoncc, that's what I meant. I implemented these enhancements in https://github.com/WordPress/wordpress-develop/pull/4838, ready for review.
#50
follow-up:
↓ 55
@
17 months ago
- Resolution set to fixed
- Status changed from reopened to closed
In 56223:
@peterwilsoncc commented on PR #4838:
17 months ago
#51
This ticket was mentioned in PR #4850 on WordPress/wordpress-develop by joemcgill.
17 months ago
#54
This is a follow up to [56223] which replaces the wp_in_
prefix with wp_is_
to be more consistent with other boolean functions in WordPress, like wp_is_maintenance_mode(), wp_is_recovery_mode(), etc.
See #57487.
Trac ticket: https://core.trac.wordpress.org/ticket/57487
#55
in reply to:
↑ 50
;
follow-ups:
↓ 58
↓ 62
@
17 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
Replying to peterwilsoncc:
Introduce the utility function
wp_in_development_mode()
allowing developers to detect the mode via a parameter.
Hmm, think that's not quite ready. Seems wp_in_development_mode()
lacks the ability to detect "any" dev. mode. I.e. a possible (desirable) future use of development mode would be to do strict checks, and throw more fatal errors. That should be enabled in "any" dev. mode.
Maybe add:
if ( 'any' === $mode ) { return true; }
just under the if empty()
check.
Edit: even better, add that in the existing conditional:
// Return true if the current mode encompasses all modes or the check is for any mode. if ( 'all' === $current_mode || 'any' === $mode ) { return true; }
#56
follow-up:
↓ 57
@
17 months ago
It may be too late to make this change, but I'd propose that we change the name of the function wp_in_development_mode()
to wp_is_development_mode()
to be more consistent with other similar function names. I've created PR #4850 with this proposed change.
If committed, the related dev-note for this feature will need to be updated.
#57
in reply to:
↑ 56
@
17 months ago
Replying to joemcgill:
I'd propose that we change the name of the function
wp_in_development_mode()
towp_is_development_mode()
to be more consistent with other similar function names. I've created PR #4850 with this proposed change.
Good catch, thanks! wp_is_development_mode()
would be my preference too.
See comment:6:ticket:49959 for renaming wp_in_maintenance_mode()
to wp_is_maintenance_mode()
previously.
PR approved.
#58
in reply to:
↑ 55
@
17 months ago
Replying to azaozz:
Hmm, think that's not quite ready. Seems
wp_in_development_mode()
lacks the ability to detect "any" dev. mode.
I was initially confused by this suggestion, but I agree that letting this function return true if any development mode is set would be nice. I'd propose that the default behavior for this function if no explicit mode is passed (i.e. $mode = null) would be to return true if wp_get_development_mode()
returns any valid value rather than adding an explicit "any" mode that needs to be passed, as I think that can easily get confused with "all" and lead to unintended bugs.
joemcgill commented on PR #4850:
17 months ago
#59
I've added a new commit to this PR to address @azaozz's comment in https://core.trac.wordpress.org/ticket/57487#comment:55 about the need to support checking for any valid development mode.
@oandregal commented on PR #4565:
17 months ago
#60
@felixarntz @azaozz any progress on why this PR affected TTFB? Also, making sure we use production environment values would be important. I'm happy to create a trac issue for this if that's preferred.
This ticket was mentioned in PR #4856 on WordPress/wordpress-develop by joemcgill.
17 months ago
#61
This allows wp_is_default_mode()
to be called with a default value of 'any' in order to return true if any valid default mode is set. This was requested in #55.
Note that this includes changes from e5fe64b0
which will be handled in https://github.com/WordPress/wordpress-develop/pull/4850.
Trac ticket: https://core.trac.wordpress.org/ticket/57487
#62
in reply to:
↑ 55
;
follow-up:
↓ 67
@
17 months ago
Replying to azaozz:
Seems
wp_in_development_mode()
lacks the ability to detect "any" dev. mode. I.e. a possible (desirable) future use of development mode would be to do strict checks, and throw more fatal errors. That should be enabled in "any" dev. mode.
Why is that needed now? The reason that this ticket was created was due to a need for specific development modes. The question "Is my site in (general) development mode?" could have just been as easily answered by WP_DEBUG
.
All use-cases that we currently have are for specific development modes. If there is a real use-case where it becomes useful to check for any
development mode, we can reconsider, but it's a long-standing core philosophy to not make API decisions without concrete use-cases, so this should be considered only if/when it's actually needed.
We can always add more supported development mode values, and we can always add a default and make the function parameter optional, without breaking backward compatibility. We can't do the reverse though. So decisions for adding something should be based on actual use-cases present.
@flixos90 commented on PR #4565:
17 months ago
#63
@oandregal I thought it was https://github.com/WordPress/wordpress-develop/pull/4565#issuecomment-1609670528? If the tests are run with WP_DEBUG=true
, it means that the results of several functions weren't cached prior to this change. But now we're no longer relying on WP_DEBUG
, but the new `WP_DEVELOPMENT_MODE`, which was introduced partially to address exactly those TODO
comments. Since the performance tests (I assume) don't set WP_DEVELOPMENT_MODE
, now the results are cached, and therefore faster.
joemcgill commented on PR #4850:
17 months ago
#64
I've committed the name change in https://core.trac.wordpress.org/changeset/56249. The conversation about adding a default value can continue on this PR.
joemcgill commented on PR #4565:
17 months ago
#65
@felixarntz and @oandregal, I'm just catching up on this conversation and confirmed that the Performance GH workflow in the core repo uses the docker environment bundled in Core, rather than the wp-env
package, which means it also is configured by the included .env
file in the core repo, which does set both WP_DEBUG
and SCRIPT_DEBUG
to true
, and sets WP_DEVELOPMENT_MODE
to core
. These are definitely oversights, in my opinion that should be easy to fix. I can make a new ticket to address these issues.
I think a bit of investigation into why each of those large shifts in performance numbers were recorded, would be useful. It seems like the initial drop was indeed caused by the addition of WP_DEVELOPMENT_MODE
, but am unsure why the second drop happened on this commit. The final large increase happened during this package update, which could be several factors, like merged PHP changes related to block library code, but it's hard to pinpoint.
joemcgill commented on PR #4565:
17 months ago
#66
Ticket created: https://core.trac.wordpress.org/ticket/58825
#67
in reply to:
↑ 62
;
follow-up:
↓ 68
@
17 months ago
Replying to flixos90:
We can always add more supported development mode values, and we can always add a default and make the function parameter optional, without breaking backward compatibility. We can't do the reverse though. So decisions for adding something should be based on actual use-cases present.
I share Felix's view that any
isn't necessarily needed at this point.
@azaozz how strongly do you feel about this? You're the 6.3 tech lead, so it's your call.
#68
in reply to:
↑ 67
;
follow-up:
↓ 69
@
17 months ago
Replying to peterwilsoncc:
how strongly do you feel about this? You're the 6.3 tech lead, so it's your call.
No strong feeling at all, just trying to brainstorm :)
We can always add more supported development mode values
I'll be a bit worried about changing it much though. As far as I see the last thing that can be added would be to return when a development mode is set, regardless of which particular flavor.
If there is a real use-case where it becomes useful to check for any development mode
Yes, there are few although I couldn't get the time to test and implement them. One is to change _doing_it_wrong()
behavior (or the new proposed function that would replace it) depending on whether dev. mode is set. Thinking it may even throw a fatal error in some cases on a particularly "wrong" things.
Another is to reexamine all uses of WP_DEBUG
, maybe make some of them conditional on dev. mode, and maybe make some throw more severe errors/warnings, perhaps with stack trace, etc.
I'd propose that the default behavior for this function if no explicit mode is passed (i.e. $mode = null) would be to return true if wp_get_development_mode() returns any valid value rather than adding an explicit "any"
Yep, I'm with @joemcgill here. Thinking that would be best. Thanks for the suggestion.
#69
in reply to:
↑ 68
;
follow-up:
↓ 71
@
17 months ago
- Resolution set to fixed
- Status changed from reopened to closed
Replying to azaozz:
We can always add more supported development mode values
I'll be a bit worried about changing it much though. As far as I see the last thing that can be added would be to return when a development mode is set, regardless of which particular flavor.
Again, I am unsure about a use-case for that. WP_DEBUG
could be used just as well for this.
If there is a real use-case where it becomes useful to check for any development mode
Yes, there are few although I couldn't get the time to test and implement them. One is to change
_doing_it_wrong()
behavior (or the new proposed function that would replace it) depending on whether dev. mode is set. Thinking it may even throw a fatal error in some cases on a particularly "wrong" things.
Why should debugging/error logging/error handling work differently between WP_DEBUG
vs WP_DEVELOPMENT_MODE
? The WP_DEBUG
constant is the current way to control those things, so making tweaks depending on WP_DEVELOPMENT_MODE
would intertwine things.
Either way, what is being discussed here does not fit in the scope of this ticket, for the following reasons:
- The reason
WP_DEVELOPMENT_MODE
was introduced was to allow certain core behaviors to change based on what kind of development is happening on the site (see e.g. #57573). That was a problem whichWP_DEBUG
would have been unable to address. - This follow-up conversation is about expanding the purpose of the
WP_DEVELOPMENT_MODE
to express a general development mode. This is different from the original purpose and even from the value "all". While I'm personally not a fan of that value, it still fits into the original paradigm and also addresses a real use-case, such as agencies working on all three things (core, plugins, themes) in a single environment. - The proposed change could be added at any point in the future without breaking backward-compatibility. It would be expanding the scope of the current concept, but not remove or change anything.
To clarify: The function renaming discussion was different, as that clearly belonged to this ticket's scope, since such a change would have been impossible to make in the future. That's not the case here though.
For those reasons, whether to expand WP_DEVELOPMENT_MODE
to be usable as a "yes/no" constant and whether to potentially replace some WP_DEBUG
usage with it should be discussed in a separate follow-up ticket. FWIW I'm not necessarily against the proposal, but it is an entirely separate purpose from the original scope of the WP_DEVELOPMENT_MODE
constant and therefore requires careful consideration. It shouldn't be rushed into 6.3, and it also doesn't need to be, since adding it later is perfectly possible without BC breaks.
Let's open a follow-up ticket to discuss the rationale and use-cases for such a potential change further.
#70
@
17 months ago
- Keywords has-dev-note added; needs-dev-note removed
Forgot to post it here earlier, but the dev note was posted on Friday (and since updated to reflect the renamed function, as discussed above): https://make.wordpress.org/core/2023/07/14/configuring-development-mode-in-6-3/
#71
in reply to:
↑ 69
@
17 months ago
- Keywords needs-patch added; has-patch removed
Replying to flixos90:
what is being discussed here does not fit in the scope of this ticket
Not so sure about that :)
The scope of this ticket is to add support for a development mode, not to decide how fragmented that support should be.
The reason WP_DEVELOPMENT_MODE was introduced was to allow certain core behaviors to change based on what kind of development is happening...
That's not exactly true too. The idea is/was to be able to set development mode in general, with some additional settings that could be used in particular cases. However the main use of WP_DEVELOPMENT_MODE
remains a boolean: true/false. That's the most useful part of it.
Why should debugging/error logging/error handling work differently between WP_DEBUG vs WP_DEVELOPMENT_MODE?
Think we discussed this already, can't find exactly where, sorry, maybe some PR. The idea is to expand what WP_DEBUG does based on whether dev. mode is enabled. Like perhaps throwing fatal errors with stack trace for cases that now throw only warnings, etc. Also keep in mind WP_DEBUG is auto-set whenever dev. mode is enabled. They should compliment one another not be mutually exclusive.
Anyway, unfortunately the new helper function wp_in_development_mode()
is now incapable to fulfill that requirement so all places that need to check whether development mode is enabled or not will have to use (bool) wp_get_development_mode()
. Inconsistent :(
Related #33161