Allow to register pre-instantiated widgets

[Denis-de-Bernardy]

As things stand, the current API doesn't allow to instantiate a widget that relies on dependency injection because register_widget() and WP_Widget_Factory#register() expect a class name as a parameter.

It would be convenient if the factory would conditionally instantiate the widget classes instead:

	function register($widget_class) {
		$this->widgets[$widget_class] = is_object($widget_class) ? $widget_class : new $widget_class();
	}

[valendesigns]

@Denis-de-Bernardy could you please expand on why this functionality is needed from your perspective? I'm not sure I clearly see why Core should be supporting an alternative method for programmatically adding widgets. As well, if you could attach a code example. Any details you could provide here would be a huge help if you want to see any movement on this issue.

[Denis-de-Bernardy]

Sure. If you want your widgets to use a dependency injection, they need to be instantiated like so:

$my_widget = new MyWidget($dep1, $dep2);
wp_register_widget($my_widget); // or perhaps more appropriately, $my_widget->register();

... but as things stand, WP expects a call more like:

wp_register_widget('MyWidget'); // and now $dep1 and $dep2 dependencies are missing.

To work around the issue, you currently need to edit the widget factory's private variables.

[valendesigns]

Thank you for responding back. Could you please be more specific about what use case dependency injection has and why core should support it?

[Denis-de-Bernardy]

The use-case for dependency injection is probably too broad for a Trac answer. In a nutshell, though, it's about composition, loose coupling, and testability. A URL for further reading:

​http://stackoverflow.com/q/130794/417194

In the event it needs to be stressed, the purpose to be play better with PHP frameworks such as Symfony 2:

​http://symfony.com/doc/current/components/dependency_injection/index.html

I was looking into creating a sensible WP Widget wrapper that played well with a Symfony DIC when I opened this ticket.

[westonruter]

Replying to Denis-de-Bernardy:

The use-case for dependency injection is probably too broad for a Trac answer. In a nutshell, though, it's about composition, loose coupling, and testability. A URL for further reading:

​http://stackoverflow.com/q/130794/417194

In the event it needs to be stressed, the purpose to be play better with PHP frameworks such as Symfony 2:

​http://symfony.com/doc/current/components/dependency_injection/index.html

I was looking into creating a sensible WP Widget wrapper that played well with a Symfony DIC when I opened this ticket.

Why not use setter injection or property injection instead of constructor injection? For instance:

<?php
add_action( 'widgets_init', function () {
    /** @var \WP_Widget_Factory $wp_widget_factory */
    global $wp_widget_factory;
    register_widget( 'My_Widget' );
    $wp_widget_factory->widgets['My_Widget']->otherDependency = $foo;
} );

[Denis-de-Bernardy]

Replying to westonruter:

Why not use setter injection or property injection instead of constructor injection?

Because there's something wrong with engineering software like that in my opinion except in very special cases.

[westonruter]

Replying to Denis-de-Bernardy:

Because there's something wrong with engineering software like that in my opinion except in very special cases.

OK, I don't have any problem with the proposed change. Can you put together a patch, including unit tests?

[mdwheele]

Replying to westonruter:

OK, I don't have any problem with the proposed change. Can you put together a patch, including unit tests?

If it's not against contribution standards for me to do so, I can prepare a patch for this feature. I found this ticket in the process of looking to fix the same issue myself.

To expand upon the use-case for this being made-available to core, I have an example:

I maintain a set of microservices where I work that are responsible for serving HR and Student data as well as other use-cases. For this particular use-case, we have a service responsible for providing query capabilities for a University job-board. This is exposed as a JSON API and I have also written packages around that API for PHP-consumption. All this leads to... the implementation of a client that talks to that API conforms to a known interface and I want to make a WordPress widget that depends on that client interface to query the job board. I maintain a package written in PHP for querying this API to be used by web applications and other PHP clients. Taking advantage of this package in a WordPress widget or plugin should be a simple matter of wiring-only. Not a lot of custom logic and if there IS any, it's simple UI / WordPress-focused branching.

The biggest reason for wanting DI support for things like Widgets is that I can treat the widgets as simple-wiring of my own services into WordPress. I treat Plugins exactly the same; simple-wiring of a domain model into WordPress. Therefore, when I test, my focus is on a WordPress-agnostic domain model. When I write integration tests with WordPress and my custom model, I'm making sure that when a widget is "booted" it calls the right methods on my services. I do not necessarily care to invoke my services as part of that test... I just want to have spies tell me if things aren't called as expected.

This can certainly be achieved through setter-injection, but is more conventional to implement through constructor injection. More importantly, constructor injection FIRMLY declares that X widget wiring has a dependency on some service layer. Setting injection does not do so explicitly. In addition (and probably more importantly, as @Denis-de-Bernardy said), injecting as described above requires the client injecting to intimately understand not-only my Widget and its internal dependencies to be able to inject, but also must intimately be aware of WordPress internals, side-stepping the Core's framework for registration of Widgets. If that ever changed, client's would be broken. It is always preferable for clients to be able to rely on a stable abstraction.

Anywho... I typed all that out just to give a more in-depth use-case than we currently had. Also, I've made the changes to enable this and would GLADLY contribute upstream.. However, this would be my first contribution to core and I wanted to make sure it was cool for a sideline observer to send code for someone elses Trac ticket! :)

Thanks all!

[mdwheele]

Implementation that exits early.

[mdwheele]

Implementation that branches.

[mdwheele]

Updates register_widget and unregister_widget docs to reflect change in argument type expectation.

[mdwheele]

I've added two possible implementations that vary on whether branching or exiting-early is preferred in the widget factory register and unregister methods. This includes a test that has been tagged with this ticket number. Tests are passing. The third patch updates the register_widget and unregister_widget functions to reflect the change in argument type expectation.

I apologize in advance if this oversteps standards for contribution! I wanted to make sure I packaged this up before moving on to other tasks. Please let me know if there's anything you'd like changed!

[mdwheele]

Are there any updates on this?

[westonruter]

No update, other than let's look at this in 4.4

[mdwheele]

Sounds good. Was just curious and making sure nothing more was needed! Thanks!

[westonruter]

Related: #33473

[westonruter]

Aside: This is somewhat related to the recent WP-CLI changes to avoid having to register a command by providing a class name but instead to allow passing an object instance (here a callable): ​https://github.com/wp-cli/wp-cli/issues/2204

[PeterRKnight]

Replying to mdwheele:

I've added two possible implementations that vary on whether branching or exiting-early is preferred in the widget factory register and unregister methods. This includes a test that has been tagged with this ticket number. Tests are passing. The third patch updates the register_widget and unregister_widget functions to reflect the change in argument type expectation.

Your patch allows for pre-initialized widgets but if you try to instantiate multiple widgets of a certain class with arguments, they overwrite each other as far as I can see. So it doesn't cover the example given by @Denis-de-Bernardy here https://core.trac.wordpress.org/ticket/28216?replyto=11#comment:3

I would love to see core support for creating dynamic widgets make it into 4.6!

[PeterRKnight]

mdwheele's patch using spl_object_hash instead of get_class allowing for multiple instances of class

[westonruter]

@PeterRKnight How fortunate that spl_object_hash() is available in PHP 5.2! However, I see it is used in _wp_filter_build_unique_id() but has an alternate case for handling if the function is not defined. This worried me in that perhaps the function could be disabled in 5.2. But see #21267:

Since PHP 5.2 (the required version for WP is 5.2.4), spl_object_hash exists, which means the else statement and function_exists() check are completely useless in _wp_filter_build_unique_id()

So I think we're safe.

[westonruter]

I fleshed out the unit tests, testing the registration of a class name and a class instance together, ensuring both result in registered widgets. See 28216.5.diff​. I think this is good to go!

[ocean90]

@westonruter Inline @see tags ​should only be used for hooks.

[westonruter]

Fix docs

[westonruter]

Test registering multiple instances of the same class

[westonruter]

In 37329:

Widgets: Allow WP_Widget subclass instances (objects) to be registered/unregistered in addition to WP_Widget subclass names (strings).

Allows widgets to be registered which rely on dependency injection. Also will allow for new widget types to be created dynamically (e.g. a Recent Posts widget for each registered post type).

See #35990.
Props mdwheele, PeterRKnight, westonruter.
Fixes #28216.

[jdgrimes]

Replying to westonruter:

@PeterRKnight How fortunate that spl_object_hash() is available in PHP 5.2! However, I see it is used in _wp_filter_build_unique_id() but has an alternate case for handling if the function is not defined. This worried me in that perhaps the function could be disabled in 5.2. But see #21267:

Since PHP 5.2 (the required version for WP is 5.2.4), spl_object_hash exists, which means the else statement and function_exists() check are completely useless in _wp_filter_build_unique_id()

So I think we're safe.

The reason that the function_exists() check is needed is that the SPL extension can be disabled prior to PHP 5.3.0. So in 5.2.0 it is on by default but ​prior to 5.3.0 it can be disabled. In which case the functions that it provides wouldn't be available.

[westonruter]

Provide fallback if spl_object_hash() is not available

[westonruter]

@jdgrimes thank you for that information. How does 28216.8.fallback.diff​ look for providing a fallback in the case where spl_object_hash() is not available? It takes a somewhat similar approach as _wp_filter_build_unique_id().

[jdgrimes]

Replying to westonruter:

@jdgrimes thank you for that information. How does 28216.8.fallback.diff​ look for providing a fallback in the case where spl_object_hash() is not available? It takes a somewhat similar approach as _wp_filter_build_unique_id().

That looks OK to me. (Though admittedly I haven't been following this ticked closely, so don't take my word for it. :-)

[westonruter]

In 37333:

Widgets: Provide PHP 5.2 fallback for spl_object_hash() if disabled in logic for registering and unregistering pre-instantiated widgets.

Fixes #28216.