wp-includes files should not contain hooks

[wonderboymusic]

It is general good practice to have 2 types of PHP files: those that declare symbols, and those that cause side effects (vars, functions calls, class invocations, includes, etc).

There are some random add_action() and add_filter() calls littered around some files in wp-includes/. These should be moved to wp-includes/default-filters.php with the rest of the registered hooks. It seems like this was the best practice for awhile and then we randomly stopped. This file loads way before any of the includes, so the hooks will be registered for any request that loads WordPress, even SHORTINIT - a lot of the hooks registered won't run anyways (that's already the case).

I made sure the hooks are in the same order - corresponding to the order their files load.

Almost all static analysis tools + hackificator (see Hack) complain about this. Also a requirement of PSR-2, which I know we don't care about, but doesn't hurt.

[rmccue]

+1, totally agree with this. I've had times where I'm loading in files (typically loading parts of wp-includes from a page that doesn't automatically include them) and the code in them makes it impossible to use.

I'd suggest the same for wp-admin/includes/ too if we can.

[wonderboymusic]

In 31168:

There are some random add_action() and add_filter() calls littered around some files in wp-includes/. These should be moved to wp-includes/default-filters.php with the rest of the registered hooks. It seems like this was the best practice for awhile and then we randomly stopped. This file loads way before any of the includes, so the hooks will be registered for any request that loads WordPress, even SHORTINIT - a lot of the hooks registered won't run anyways (that's already the case).

See #30947.

[wonderboymusic]

wp-admin/includes/ is way trickier - since the add_*() calls are really only meant to run when the "template part" is loaded, which is not universal for the whole admin.

Let's mark this as done and maybe spin up a new ticket if we want to clean up the admin calls.

[bobbingwide]

-1.

I think there may be a problem with the changes to update.php
since the code no longer takes into account this test that preceded the logic to add the actions.

if ( ( ! is_main_site() && ! is_network_admin() ) || ( defined( 'DOING_AJAX' ) && DOING_AJAX ) ) {
	return;
}

Question: What happens during AJAX processing for 'init'... will wp_schedule_update_checks() get invoked?

Answer: Yes, wp_schedule_update_checks will be run.
For instance. When you're editing a post and a heartbeat request is sent then this function is invoked.

Question: Will it work properly on multisite?
Answer: No.

Also... what's left at the bottom of the file is now rather unnecessary!

This is an unintended functional change.

I believe you'll need to revert the change to /wp-includes/update.php

[SergeyBiryukov]

30947.2.diff​ moves the reversed condition to default-filters.php.

[DrewAPicture]

This can ride to Beta 1, see [31168]. @wonderboymusic: Anything left here?

[ocean90]

30947.2.diff​ needs to be committed.

[SergeyBiryukov]

In 31701:

Don't run update checks for AJAX requests after [31168].

fixes #30947.

[rachelbaker]

On an multisite install after [31701] I see this:

Notice: Trying to get property of non-object in /srv/www/wp/wp-includes/functions.php on line 3834

the wp-includes/default-filters.php is loaded before the $current_site global is set.

[rachelbaker]

#31587 was marked as a duplicate.

[SergeyBiryukov]

I guess we should revert the part of [31168] that [31701] was trying to fix.

[SergeyBiryukov]

In 31708:

Revert the part of [31168] that [31701] was trying to fix.

default-filters.php is loaded before the $current_site global is set, so is_main_site() cannot be used there.

fixes #30947.

[nacin]

default-filters.php should definitely remain free of pretty much anything. Good call with [31708].