DevinVinson / WordPress-Plugin-Boilerplate

[WordPress] A foundation for WordPress Plugin Development that aims to provide a clear and consistent guide for building your plugins.
http://wppb.io
7.62k stars 2.25k forks source link

Question about is_admin #585

Open xoex opened 1 year ago

xoex commented 1 year ago

I was looking at the code, asked myself, isn't it better to wrap admin hooks in is_admin() ? for example in this code :

private function define_admin_hooks() {

        $plugin_admin = new Ghorekeshi_Admin( $this->get_plugin_name(), $this->get_version() );

        $this->loader->add_action( 'admin_enqueue_scripts', $plugin_admin, 'enqueue_styles' );
        $this->loader->add_action( 'admin_enqueue_scripts', $plugin_admin, 'enqueue_scripts' );
    }

If wrap inside code in a if (is_admin()) {} so we don't run administrative hooks when non-admin pages and posts are loaded?

JadeResources commented 1 year ago

The hook admin_enqueue_scripts will only run on admin anyway so the is_admin() is a pointless check. The hook enqueue_scripts does the opposite and will only run on the front end.

xoex commented 1 year ago

yes it's true, but a lot of unused action and hooks can be added, what if we change this part of code :

    public function __construct() {
        if ( defined( 'PLUGIN_NAME_VERSION' ) ) {
            $this->version = PLUGIN_NAME_VERSION;
        } else {
            $this->version = '1.0.0';
        }
        $this->plugin_name = 'plugin-name';

        $this->load_dependencies();
        $this->set_locale();
        $this->define_admin_hooks();
        $this->define_public_hooks();
    }

to this :

    public function __construct() {
        if ( defined( 'PLUGIN_NAME_VERSION' ) ) {
            $this->version = PLUGIN_NAME_VERSION;
        } else {
            $this->version = '1.0.0';
        }
        $this->plugin_name = 'plugin-name';

        $this->load_dependencies();
        $this->set_locale();
        if (is_admin()) {
            $this->define_admin_hooks();
        }
        $this->define_public_hooks();
    }

So we don't run create a new instance of admin class and we don't add non necessary admin hooks and filters to public pages.

gerardreches commented 7 months ago

Most actions and filters that you will use inside define_admin_hooks() will or should only be executed when in the admin dashboard. There could even be some use cases in which you don't want to check is_admin(), such as a custom admin action that is triggered through a form on the frontend.