MenuChildrenNodeJoin::create not compatible with Drupal 11.3

Thanks @rjjakes - could you please provite this as MR instead?

The code inside the MenuChildrenNodeJoin Plugin looks very "unusual", let me take a look.

I don't quite get this plugin implementation conceptually.

For one, there are two instantiation mechanisms for the same plugin:
One, as a service ("views_menu_children_filter.join_handler", which is used by several other classes in this module) and the other through the views plugin system (via `@ViewsJoin("menu_children_node_join")`)

Second, Neither will actually use this "create" method:
The views plugin system instantiation would only use "create", if the class would implement "ContainerFactoryPluginInterface" but the class doesn't nor any parent nor any other class extending "JoinPluginBase" in core ever used dependency injection on a join plugin.
The service handler instantiation uses "_construct" by default.

Meaning, this method was never actually called.

Also, the create implementation doesn't actually inject any services. It is only used to set default settings which were never used to begin with.

I'd vote for entirely removing the method (and instantiating it only as a service? Unsure about that, not too deep into views implementations). It is not even manually executed where the class is instantiated as a service. Though, I wonder how the error:

Fatal error: Declaration of Drupal\views_menu_children_filter\Plugin\views\join\MenuChildrenNodeJoin::create(array $configuration, $plugin_id, $plugin_definition) must be compatible with Drupal\Core\Plugin\PluginBase::create(Symfony\Component\DependencyInjection\ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition)

came to be. Not even in 11.3.x "PluginBase" has a create method, see https://git.drupalcode.org/project/drupal/-/blob/11.3.x/core/lib/Drupal/...

@rjjakes: Could you link the Drupal 11.3 core change that caused this incompatibility?

I'm unsure if we may really remove the method, because I'm not sure about the details yet. Fixing the method parameters instead would be safe, if we understand the Core change.

Someone needs some more time on this, if the change is more complex. I don't have that time currently.

Would be great if we had some more details @rjjakes!

I simply wonder how it ever was called. No parent extends "ContainerInjectionInterface" and the service definition:

services:
  views_menu_children_filter.join_handler:
    class: Drupal\views_menu_children_filter\Plugin\views\join\MenuChildrenNodeJoin
    arguments:
      -
        type: INNER
        table: menu_link_content_data
        field: false
        left_table: false
        left_field: false
        operator: '='
      - menu_children_node_join
      - ''

generally uses "__construct()" directly. Nor have I seen this pattern in any other "ViewsJoin" Plugin.

Please test the new MR changes, if everything still works as expected. I don't have the time right now, to test the changes.

@rjjakes could you provide a backtrace maybe? That would answer it.

For me this happens with any views that has a relationship defined on a fresh cache.
The plugin manager tries to load the available ViewsJoin Plugins and fails because of the incorrect declaration.

( ! ) Fatal error: Declaration of Drupal\views_menu_children_filter\Plugin\views\join\MenuChildrenNodeJoin::create(...) must be compatible with Drupal\Core\Plugin\PluginBase::create(...) in /var/www/html/web/modules/contrib/views_menu_children_filter/src/Plugin/views/join/MenuChildrenNodeJoin.php on line 42

Call Stack

Time Memory Function Location
1 1.17s 6.1MB {fiber} .../Renderer.php:635
2 1.17s 6.2MB Drupal\Core\Render\Renderer::... .../Renderer.php:634
3 1.17s 6.2MB Drupal\Core\Render\MainContent\HtmlRenderer::... .../HtmlRenderer.php:242
4 1.17s 6.2MB Drupal\Core\Render\Renderer->render(...) .../HtmlRenderer.php:242
5 1.17s 6.2MB Drupal\Core\Render\Renderer->doRender(...) .../Renderer.php:230
6 1.18s 7.2MB Drupal\Core\Render\Renderer->doCallback(...) .../Renderer.php:441
7 1.18s 7.2MB Drupal\Core\Render\Renderer->doTrustedCallback(...) .../Renderer.php:910
8 1.18s 7.2MB call_user_func_array(...) .../DoTrustedCallbackTrait.php:107
9 1.18s 7.3MB Drupal\views\Element\View::preRenderViewElement(...) .../DoTrustedCallbackTrait.php:107
10 1.18s 7.3MB Drupal\views\ViewExecutable->executeDisplay(...) .../View.php:80
11 1.21s 11.7MB Drupal\views\Plugin\views\display\Page->execute() .../ViewExecutable.php:1686
12 1.21s 11.7MB Drupal\views\ViewExecutable->build(...) .../Page.php:198
13 1.21s 11.7MB Drupal\views\ViewExecutable->_build(...) .../ViewExecutable.php:1309
14 1.22s 12.2MB Drupal\views\Plugin\views\relationship\RelationshipPluginBase->query() .../ViewExecutable.php:1434
15 1.22s 12.2MB Drupal\views\Plugin\ViewsHandlerManager->createInstance(...) .../RelationshipPluginBase.php:160
16 1.22s 12.2MB Drupal\Component\Plugin\PluginManagerBase->createInstance(...) .../ViewsHandlerManager.php:148
17 1.22s 12.2MB Drupal\Core\Plugin\Factory\ContainerFactory->createInstance(...) .../PluginManagerBase.php:78
18 1.22s 12.2MB Drupal\Core\Plugin\DefaultPluginManager->getDefinition(...) .../ContainerFactory.php:16
19 1.22s 12.2MB Drupal\Core\Plugin\DefaultPluginManager->getDefinitions() .../DefaultPluginManager.php:216
20 1.22s 12.2MB Drupal\Core\Plugin\DerivativeDiscoveryDecorator->getDefinitions() .../DefaultPluginManager.php:340
21 1.22s 12.2MB Drupal\Core\Plugin\AttributeDiscoveryWithAnnotations->getDefinitions() .../DerivativeDiscoveryDecorator.php:86
22 1.22s 12.3MB Drupal\Component\Plugin\Discovery\AttributeClassDiscovery->getDefinitions() .../AttributeDiscoveryWithAnnotations.php:67
23 1.22s 12.3MB class_exists('Drupal\views_menu_children_filter\Plugin\views\join\MenuChildrenNodeJoin') .../AttributeClassDiscovery.php:162
24 1.22s 12.3MB Composer\Autoload\ClassLoader->loadClass(...) .../AttributeClassDiscovery.php:162
25 1.22s 12.3MB include('/var/www/html/web/modules/contrib/views_menu_children_filter/src/Plugin/views/join/MenuChildrenNodeJoin.php') .../ClassLoader.php:576

The provided MR solves the problem Thanks!
marking as RTBC.

I left a comment. I'm still afraid the MR might have unrelated side-effects!

Patch with an update for the signature is attached

@grevil let's keep it and add a comment linking this issue please, so it might be removed in the future! So we're double-safe!

Done. Please review!

Thanks, perfect!