Allow for optional dependencies

I'm super +1 to this patch. But I'm nervous about this going in while Crell is away. Can we clean it up a bit and hope he's in a good mood when he returns?

----

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/OptionalDependencyPass.phpundefined
@@ -0,0 +1,36 @@
+ * Contains \Drupal\Core\DependencyInjection\Compiler\OptionalDependencyPass

Missing trailing .

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/OptionalDependencyPass.phpundefined
@@ -0,0 +1,36 @@
+ * Adds optional dependencies.
+ */
+class OptionalDependencyPass implements CompilerPassInterface {

I think this whole approach is awesome and will be very useful. But we should explain why this might be useful and how you might use it, maybe even add a @code block?

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/OptionalDependencyPass.phpundefined
@@ -0,0 +1,36 @@
+  public function process(ContainerBuilder $container) {

Missing {@inheritdoc}

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/OptionalDependencyPass.phpundefined
@@ -0,0 +1,36 @@
+        if ($reference_id[0] === '@') {
+          $reference_id = substr($reference_id, 1);
+        }
+        if ($container->has($reference_id)) {
+          $optional_services[$key] = new Reference($reference_id);
+        }
+      }
+      if ($optional_services) {
+        $container->getDefinition($id)->addArgument($optional_services);

It's reasonably clear what's going on, but this could use some inline documentation.

+++ b/core/modules/system/lib/Drupal/system/Tests/Bundle/BundleTest.phpundefined
@@ -54,5 +58,4 @@ function testBundleRegistrationDynamic() {
     $this->assertTrue(drupal_container()->has('bundle_test_class'), 'The bundle_test_class service exists in the DIC.');
   }
-
 }
diff --git a/core/modules/system/tests/modules/bundle_test/bundle_test.services.yml b/core/modules/system/tests/modules/bundle_test/bundle_test.services.yml

What did that blank line ever do to you? :)

+++ b/core/modules/system/tests/modules/bundle_test/lib/Drupal/bundle_test/TestClass.phpundefined
@@ -23,13 +23,23 @@ class TestClass implements EventSubscriberInterface, DestructableInterface {
    * @param \Drupal\Core\KeyValueStore\KeyValueStoreInterface $state
    *   The state key value store.
    */
-  public function __construct(KeyValueStoreInterface $state) {
+  public function __construct(KeyValueStoreInterface $state, array $optional_dependencies = array()) {

Need to add a @param for the new one

+++ b/core/modules/system/tests/modules/bundle_test/lib/Drupal/bundle_test/TestClass.phpundefined
@@ -56,4 +66,13 @@ static function getSubscribedEvents() {
+  /**
+   * @return bool

This is supposed to have a one liner above the @return (with a blank in the middle)

This is a very unintuitive feature. An optional dependency?

The example given should be an event.

Take for example menu_menu_delete():

function menu_menu_delete(Menu $menu) {
  menu_cache_clear_all();
  // Invalidate the block cache to update menu-based derivatives.
  if (module_exists('block')) {
    drupal_container()->get('plugin.manager.block')->clearCachedDefinitions();
  }
}

When a menu is deleted, the block plugin cache must be cleared to remove all menu-blocks.

If this were an event, who would define the event? Who would subscribe to it?

Block module shouldn't need to know that menus can be deleted.
Menu module (or the menu entity storage controller, that invokes this hook) shouldn't worry if the block plugin it provides is in use.

I started #1980212: Use cachetags to invalidate block definitions as an alternative solution to this issue.

I'm with Mark. This seems seriously weird. If you want to optionally integrate with another component, wouldn't either an event or just a direct observer object (that you or the other module can register) be cleaner?

There is <argument type="service" id="foo" on-invalid="null" /> .. IIRC for yaml the syntax is "?@foo" .. which just passes null if the service doesnt exist

#12 is what I'm doing for the url generator's optional dependency on the request over in #1888424-158: Make Drupal's URL generation logic available to HttpKernel, and minimize code repetition/divergence - see the first hunk there.

Between events and optional service dependencies (which we're already using anyway) I don't think this is necessary. Closing.