Make WordPress Core

Opened 21 months ago

Closed 15 months ago

Last modified 15 months ago

#57487 closed enhancement (fixed)

Add support for "development mode"

Reported by: azaozz's profile azaozz Owned by: flixos90's profile 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)

#1 follow-up: @afragen
21 months ago

Related #33161

#2 follow-up: @azaozz
21 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: @azaozz
21 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.

Last edited 21 months ago by azaozz (previous) (diff)

#4 follow-up: @TimothyBlynJacobs
21 months ago

Also the WP_ENVIRONMENT_TYPE constant was deprecated

What do you mean by this?

#5 in reply to: ↑ 2 ; follow-up: @SergeyBiryukov
21 months ago

Replying to azaozz:

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.

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 @azaozz
21 months ago

Replying to TimothyBlynJacobs:
Uh, sorry, edited the above comment to fix it.

#7 in reply to: ↑ 5 @azaozz
21 months ago

Replying to SergeyBiryukov:

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?

Yes, think so. Also would be a good shortcut to set WP_DEBUG_LOG and WP_DEBUG_DISPLAY if not set separately.

Just curious, is there a use case where setting WP_ENVIRONMENT_TYPE to development 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 in a local environment? This makes the whole functionality there pretty confusing and useless imho :(

Last edited 21 months ago by azaozz (previous) (diff)

#8 in reply to: ↑ 3 @SergeyBiryukov
21 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 @peterwilsoncc
21 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 @azaozz
21 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.

Last edited 21 months ago by azaozz (previous) (diff)

#11 follow-up: @knutsp
21 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) or str_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.
Last edited 21 months ago by knutsp (previous) (diff)

#12 follow-ups: @flixos90
21 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.

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

#13 in reply to: ↑ 12 @azaozz
20 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 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.

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.

Last edited 20 months ago by azaozz (previous) (diff)

#14 @flixos90
20 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: @SergeyBiryukov
20 months ago

Replying to knutsp:

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
]

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 @flixos90
20 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.


16 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: @flixos90
16 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 @azaozz
16 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)?

Last edited 16 months ago by azaozz (previous) (diff)

@azaozz commented on PR #4565:


16 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:


16 months ago
#21

@azaozz

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?

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:


16 months ago
#22

@azaozz

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?

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 @flixos90
16 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: @peterwilsoncc
16 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 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.

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 @schlessera
16 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: @flixos90
16 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 or false 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.

#27 @flixos90
16 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 56042:

General: Introduce WP_DEVELOPMENT_MODE constant to signify context-specific development mode.

In recent releases, WordPress core added several instances of cache usage around specific files. While those caches are safe to use in a production context, in development certain nuances apply for whether or not those caches make sense to use. Initially, WP_DEBUG was used as a temporary workaround, but it was clear that a more granular method to signify a specific development mode was required: For example, caches around theme.json should be disabled when working on a theme as otherwise it would disrupt the theme developer's workflow, but when working on a plugin or WordPress core, this consideration does not apply.

This changeset introduces a WP_DEVELOPMENT_MODE constant which, for now, can be set to either "core", "plugin", "theme", or an empty string, the latter of which means no development mode, which is also the default. A new function wp_get_development_mode() is the recommended way to retrieve that configuration value.

With the new function available, this changeset replaces all existing instances of the aforementioned WP_DEBUG workaround to use wp_get_development_mode() with a more specific check.

Props azaozz, sergeybiryukov, peterwilsoncc, spacedmonkey.
Fixes #57487.

#29 @flixos90
16 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.

#30 @flixos90
16 months ago

#57573 was marked as a duplicate.

#31 @flixos90
16 months ago

#57573 was marked as a duplicate.

#32 @flixos90
16 months ago

  • Keywords needs-dev-note added

@oandregal commented on PR #4565:


16 months ago
#33

@felixarntz @azaozz I've noticed the metric "server time (total) for block themes" has improved dramatically after this change:

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/583546/a4b300c4-2f59-4815-a863-8e7a67d6a277

Is this expected? Could you elaborate why this happens? I'd be super excited if this is a real improvement :)

@flixos90 commented on PR #4565:


16 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 @w33zy
16 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 @knutsp
15 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.

Last edited 15 months ago by knutsp (previous) (diff)

@oandregal commented on PR #4565:


15 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.

#38 @kebbet
15 months ago

I've created #58646 as a follow up ticket.

#39 @flixos90
15 months ago

In 56079:

Site Health: Include new WP_DEVELOPMENT_MODE in the list of constants.

Follow-up to [56042].

Props kebbet.
Fixes #58646.
See #57487.

@oandregal commented on PR #4565:


15 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.

@azaozz commented on PR #4565:


15 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:


15 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:


15 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:

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/583546/ff2a9f1b-5515-44b7-b649-d415f4adfc70

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:


15 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: @peterwilsoncc
15 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, 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.

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 @flixos90
15 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 @peterwilsoncc
15 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.


15 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 @flixos90
15 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: @peterwilsoncc
15 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 56223:

General: Introduce all development mode.

Introduce the development mode all as a a cover-all mode for the existing theme, plugin and core development modes. Developers can use the all mode if they are developing both themes and plugins, for example.

Introduce the utility function wp_in_development_mode() allowing developers to detect the mode via a parameter. If the development mode is set to all this function will always return true. If the development mode is specific then only the chosen mode will return true.

Follow up to [56079,56042].

Props flixos90.
Fixes #57487.

#52 @SergeyBiryukov
15 months ago

In 56231:

Docs: Clarify where the wp_get_development_mode() value is retrieved from.

Includes:

  • Adding a mention of the WP_DEVELOPMENT_MODE constant.
  • Minor DocBlock formatting adjustments.

Follow-up to [56042], [56223].

See #57487, #57840.

#53 @SergeyBiryukov
15 months ago

In 56232:

Docs: Use consistent wording for development mode.

Follow-up to [56042], [56223], [56231].

See #57487, #57840.

This ticket was mentioned in PR #4850 on WordPress/wordpress-develop by joemcgill.


15 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: @azaozz
15 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;
}
Last edited 15 months ago by azaozz (previous) (diff)

#56 follow-up: @joemcgill
15 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 @SergeyBiryukov
15 months ago

Replying to joemcgill:

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.

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 @joemcgill
15 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:


15 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:


15 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.


15 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: @flixos90
15 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:


15 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:


15 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:


15 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.

#67 in reply to: ↑ 62 ; follow-up: @peterwilsoncc
15 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: @azaozz
15 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.

Last edited 15 months ago by azaozz (previous) (diff)

#69 in reply to: ↑ 68 ; follow-up: @flixos90
15 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 which WP_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 @flixos90
15 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 @azaozz
15 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 :(

Last edited 15 months ago by azaozz (previous) (diff)

#72 @azaozz
15 months ago

Follow-up: #58855.

Note: See TracTickets for help on using tickets.